]> git.r.bdr.sh - rbdr/mobius/commitdiff
Tests and minor fixes
authorJeff Halter <redacted>
Sun, 15 Aug 2021 17:39:43 +0000 (10:39 -0700)
committerJeff Halter <redacted>
Sun, 15 Aug 2021 17:39:43 +0000 (10:39 -0700)
cmd/mobius-hotline-server/main.go
hotline/client.go
hotline/file_path.go
hotline/file_path_test.go
hotline/file_store.go [new file with mode: 0644]
hotline/server_blackbox_test.go
hotline/transaction_handlers.go
hotline/transaction_handlers_test.go
hotline/version.go

index 772625b6a19b897d74a1ed95b77f135f5a9239e1..cf69ea8ae1f5f2e35b431c5c78a745b38cad4e44 100644 (file)
@@ -44,6 +44,8 @@ func main() {
                logger.Fatalw("Configuration directory not found", "path", configDir)
        }
 
+       hotline.FS = hotline.OSFileStore{}
+
        srv, err := hotline.NewServer(*configDir, "", *basePort, logger)
        if err != nil {
                logger.Fatal(err)
index cf88d8a8a9fd6d9b8b3053acb76342bf83b050fa..33ec511341878cd7d98c97a64567f48c05f40766 100644 (file)
@@ -200,7 +200,7 @@ var clientHandlers = map[uint16]clientTHandler{
                Handler: handleTranServerMsg,
        },
        tranKeepAlive: clientTransaction{
-               Name:    "tranKeepAlive",
+               Name: "tranKeepAlive",
                Handler: func(client *Client, transaction *Transaction) (t []Transaction, err error) {
                        return t, err
                },
@@ -699,9 +699,5 @@ func (c *Client) HandleTransaction(t *Transaction) error {
 }
 
 func (c *Client) Disconnect() error {
-       err := c.Connection.Close()
-       if err != nil {
-               return err
-       }
-       return nil
+       return c.Connection.Close()
 }
index 733dfaa64ceafce0bc6bd0f03e8c2c91a43789f2..d6c05bcbcfb581d4df6d7f0dd89e2eb0670cea81 100644 (file)
@@ -1,8 +1,10 @@
 package hotline
 
 import (
+       "bytes"
        "encoding/binary"
-       "strings"
+       "errors"
+       "path"
 )
 
 const pathSeparator = "/" // File path separator TODO: make configurable to support Windows
@@ -24,12 +26,19 @@ func NewFilePathItem(b []byte) FilePathItem {
 }
 
 type FilePath struct {
-       ItemCount []byte
+       ItemCount [2]byte
        Items     []FilePathItem
 }
 
+const minFilePathLen = 2
 func (fp *FilePath) UnmarshalBinary(b []byte) error {
-       fp.ItemCount = b[0:2]
+       if len(b) < minFilePathLen {
+               return errors.New("insufficient bytes")
+       }
+       err := binary.Read(bytes.NewReader(b[0:2]), binary.BigEndian, &fp.ItemCount)
+       if err != nil {
+               return err
+       }
 
        pathData := b[2:]
        for i := uint16(0); i < fp.Len(); i++ {
@@ -42,15 +51,16 @@ func (fp *FilePath) UnmarshalBinary(b []byte) error {
 }
 
 func (fp *FilePath) Len() uint16 {
-       return binary.BigEndian.Uint16(fp.ItemCount)
+       return binary.BigEndian.Uint16(fp.ItemCount[:])
 }
 
 func (fp *FilePath) String() string {
-       var out []string
+       out := []string{"/"}
        for _, i := range fp.Items {
                out = append(out, string(i.Name))
        }
-       return strings.Join(out, pathSeparator)
+
+       return path.Join(out...)
 }
 
 func ReadFilePath(filePathFieldData []byte) string {
index a5f5b60326579610bcacc03f845aae7098a0acf9..a8ab2ce29c3bdee557f54389e2f985958ed2ad8c 100644 (file)
@@ -31,7 +31,7 @@ func TestFilePath_UnmarshalBinary(t *testing.T) {
                                0x41, 0x20, 0x53, 0x75, 0x62, 0x44, 0x69, 0x72,
                        }},
                        want: FilePath{
-                               ItemCount: []byte{0x00, 0x02},
+                               ItemCount: [2]byte{0x00, 0x02},
                                Items: []FilePathItem{
                                        {
                                                Len:  0x0f,
diff --git a/hotline/file_store.go b/hotline/file_store.go
new file mode 100644 (file)
index 0000000..81c1f05
--- /dev/null
@@ -0,0 +1,46 @@
+package hotline
+
+import (
+       "github.com/stretchr/testify/mock"
+       "os"
+)
+
+var FS FileStore
+
+type FileStore interface {
+       Mkdir(name string, perm os.FileMode) error
+
+       Stat(name string) (os.FileInfo, error)
+       // TODO: implement
+
+       //Rename(oldpath string, newpath string) error
+       //RemoveAll(path string) error
+}
+
+type OSFileStore struct{}
+
+func (fs OSFileStore) Mkdir(name string, perm os.FileMode) error {
+       return os.Mkdir(name, perm)
+}
+
+func (fs OSFileStore) Stat(name string) (os.FileInfo, error) {
+       return os.Stat(name)
+}
+
+type MockFileStore struct {
+       mock.Mock
+}
+
+func (mfs MockFileStore) Mkdir(name string, perm os.FileMode) error {
+       args := mfs.Called(name, perm)
+       return args.Error(0)
+}
+
+func (mfs MockFileStore) Stat(name string) (os.FileInfo, error) {
+       args := mfs.Called(name)
+       if  args.Get(0) == nil {
+               return nil, args.Error(1)
+
+       }
+       return args.Get(0).(os.FileInfo), args.Error(1)
+}
index 09687add34f869f5f5a347946a3c8dd6161c9ba5..64d56ab97391341a9023e1a22ed0ecaac30cfc8b 100644 (file)
@@ -4,11 +4,12 @@ import (
        "bytes"
        "context"
        "fmt"
+       "github.com/davecgh/go-spew/spew"
+       "github.com/stretchr/testify/assert"
        "go.uber.org/zap"
        "go.uber.org/zap/zapcore"
        "net"
        "os"
-       "reflect"
        "testing"
 )
 
@@ -309,22 +310,21 @@ func TestNewUser(t *testing.T) {
        }
 }
 
-// equal is a utility function used only in tests that determines if transactions are equal enough
-func (t Transaction) equal(otherT Transaction) bool {
-       t.ID = []byte{0, 0, 0, 0}
-       otherT.ID = []byte{0, 0, 0, 0}
-
-       t.TotalSize = []byte{0, 0, 0, 0}
-       otherT.TotalSize = []byte{0, 0, 0, 0}
+func tranAssertEqual(t *testing.T, tran1, tran2 []Transaction) bool {
+       var newT1 []Transaction
+       var newT2 []Transaction
+       for _, trans := range tran1{
+               trans.ID = []byte{0,0,0,0}
+               newT1 = append(newT1, trans)
+       }
 
-       t.DataSize = []byte{0, 0, 0, 0}
-       otherT.DataSize = []byte{0, 0, 0, 0}
+       for _, trans := range tran2{
+               trans.ID = []byte{0,0,0,0}
+               newT2 = append(newT2, trans)
 
-       t.ParamCount = []byte{0, 0}
-       otherT.ParamCount = []byte{0, 0}
+       }
 
-       //spew.Dump(t)
-       //spew.Dump(otherT)
+       spew.Dump(newT1, newT2)
 
-       return reflect.DeepEqual(t, otherT)
-}
+       return assert.Equal(t, newT1, newT2)
+}
\ No newline at end of file
index d7a77e2f58a12aaa19594b3b7bf75ae752fd50f1..9e61427d4198f5099a379ee3e2ac42732452e30d 100644 (file)
@@ -9,6 +9,7 @@ import (
        "io/ioutil"
        "math/big"
        "os"
+       "path"
        "sort"
        "strings"
        "time"
@@ -503,8 +504,8 @@ func HandleDeleteFile(cc *ClientConn, t *Transaction) (res []Transaction, err er
 // HandleMoveFile moves files or folders. Note: seemingly not documented
 func HandleMoveFile(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        fileName := string(t.GetField(fieldFileName).Data)
-       filePath :=  cc.Server.Config.FileRoot + ReadFilePath(t.GetField(fieldFilePath).Data)
-       fileNewPath :=  cc.Server.Config.FileRoot + ReadFilePath(t.GetField(fieldFileNewPath).Data)
+       filePath := cc.Server.Config.FileRoot + ReadFilePath(t.GetField(fieldFilePath).Data)
+       fileNewPath := cc.Server.Config.FileRoot + ReadFilePath(t.GetField(fieldFileNewPath).Data)
 
        cc.Server.Logger.Debugw("Move file", "src", filePath+"/"+fileName, "dst", fileNewPath+"/"+fileName)
 
@@ -542,18 +543,33 @@ func HandleMoveFile(cc *ClientConn, t *Transaction) (res []Transaction, err erro
 
 func HandleNewFolder(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
        newFolderPath := cc.Server.Config.FileRoot
+       folderName := string(t.GetField(fieldFileName).Data)
+
+       folderName = path.Join("/", folderName)
 
        // fieldFilePath is only present for nested paths
        if t.GetField(fieldFilePath).Data != nil {
                var newFp FilePath
-               newFp.UnmarshalBinary(t.GetField(fieldFilePath).Data)
+               err := newFp.UnmarshalBinary(t.GetField(fieldFilePath).Data)
+               if err != nil {
+                       return nil, err
+               }
                newFolderPath += newFp.String()
        }
-       newFolderPath += "/" + string(t.GetField(fieldFileName).Data)
+       newFolderPath = path.Join(newFolderPath, folderName)
 
-       if err := os.Mkdir(newFolderPath, 0777); err != nil {
-               // TODO: Send error response to client
-               return []Transaction{}, err
+       // TODO: check path and folder name lengths
+
+       if _, err := FS.Stat(newFolderPath); !os.IsNotExist(err) {
+               msg := fmt.Sprintf("Cannot create folder \"%s\" because there is already a file or folder with that name.", folderName)
+               return []Transaction{cc.NewErrReply(t, msg)}, nil
+       }
+
+       // TODO: check for disallowed characters to maintain compatibility for original client
+
+       if err := FS.Mkdir(newFolderPath, 0777); err != nil {
+               msg := fmt.Sprintf("Cannot create folder \"%s\" because an error occurred.", folderName)
+               return []Transaction{cc.NewErrReply(t, msg)}, nil
        }
 
        res = append(res, cc.NewReply(t))
index e62cce20176ae808bdf6f04ac8eaa659bd7f8414..ec947c9feeb2ded12de6fb71c4309b2febdaf93d 100644 (file)
@@ -2,7 +2,9 @@ package hotline
 
 import (
        "github.com/stretchr/testify/assert"
+       "io/fs"
        "math/rand"
+       "os"
        "reflect"
        "testing"
 )
@@ -522,9 +524,217 @@ func TestHandleGetFileInfo(t *testing.T) {
                                t.Errorf("HandleGetFileInfo() error = %v, wantErr %v", err, tt.wantErr)
                                return
                        }
-                       if !assert.Equal(t, tt.wantRes,  gotRes) {
+                       if !assert.Equal(t, tt.wantRes, gotRes) {
                                t.Errorf("HandleGetFileInfo() gotRes = %v, want %v", gotRes, tt.wantRes)
                        }
                })
        }
 }
+
+func TestHandleNewFolder(t *testing.T) {
+       type args struct {
+               cc *ClientConn
+               t  *Transaction
+       }
+       tests := []struct {
+               setup   func()
+               name    string
+               args    args
+               wantRes []Transaction
+               wantErr bool
+       }{
+               {
+                       name: "when path is nested",
+                       args: args{
+                               cc: &ClientConn{
+                                       ID: &[]byte{0, 1},
+                                       Server: &Server{
+                                               Config: &Config{
+                                                       FileRoot: "/Files/",
+                                               },
+                                       },
+                               },
+                               t: NewTransaction(
+                                       tranNewFolder, &[]byte{0, 1},
+                                       NewField(fieldFileName, []byte("testFolder")),
+                                       NewField(fieldFilePath, []byte{
+                                               0x00, 0x01,
+                                               0x00, 0x00,
+                                               0x03,
+                                               0x61, 0x61, 0x61,
+                                       }),
+                               ),
+                       },
+                       setup: func() {
+                               mfs := MockFileStore{}
+                               mfs.On("Mkdir", "/Files/aaa/testFolder", fs.FileMode(0777)).Return(nil)
+                               mfs.On("Stat", "/Files/aaa/testFolder").Return(nil, os.ErrNotExist)
+                               FS = mfs
+                       },
+                       wantRes: []Transaction{
+                               {
+                                       clientID:  &[]byte{0, 1},
+                                       Flags:     0x00,
+                                       IsReply:   0x01,
+                                       Type:      []byte{0, 0xcd},
+                                       ID:        []byte{0x9a, 0xcb, 0x04, 0x42}, // Random ID from rand.Seed(1)
+                                       ErrorCode: []byte{0, 0, 0, 0},
+                               },
+                       },
+                       wantErr: false,
+               },
+               {
+                       name: "when path is not nested",
+                       args: args{
+                               cc: &ClientConn{
+                                       ID: &[]byte{0, 1},
+                                       Server: &Server{
+                                               Config: &Config{
+                                                       FileRoot: "/Files",
+                                               },
+                                       },
+                               },
+                               t: NewTransaction(
+                                       tranNewFolder, &[]byte{0, 1},
+                                       NewField(fieldFileName, []byte("testFolder")),
+                               ),
+                       },
+                       setup: func() {
+                               mfs := MockFileStore{}
+                               mfs.On("Mkdir", "/Files/testFolder", fs.FileMode(0777)).Return(nil)
+                               mfs.On("Stat", "/Files/testFolder").Return(nil, os.ErrNotExist)
+                               FS = mfs
+                       },
+                       wantRes: []Transaction{
+                               {
+                                       clientID:  &[]byte{0, 1},
+                                       Flags:     0x00,
+                                       IsReply:   0x01,
+                                       Type:      []byte{0, 0xcd},
+                                       ID:        []byte{0x9a, 0xcb, 0x04, 0x42}, // Random ID from rand.Seed(1)
+                                       ErrorCode: []byte{0, 0, 0, 0},
+                               },
+                       },
+                       wantErr: false,
+               },
+               {
+                       name: "when UnmarshalBinary returns an err",
+                       args: args{
+                               cc: &ClientConn{
+                                       ID: &[]byte{0, 1},
+                                       Server: &Server{
+                                               Config: &Config{
+                                                       FileRoot: "/Files/",
+                                               },
+                                       },
+                               },
+                               t: NewTransaction(
+                                       tranNewFolder, &[]byte{0, 1},
+                                       NewField(fieldFileName, []byte("testFolder")),
+                                       NewField(fieldFilePath, []byte{
+                                               0x00,
+                                       }),
+                               ),
+                       },
+                       setup: func() {
+                               mfs := MockFileStore{}
+                               mfs.On("Mkdir", "/Files/aaa/testFolder", fs.FileMode(0777)).Return(nil)
+                               mfs.On("Stat", "/Files/aaa/testFolder").Return(nil, os.ErrNotExist)
+                               FS = mfs
+                       },
+                       wantRes: []Transaction{},
+                       wantErr: true,
+               },
+               {
+                       name: "fieldFileName does not allow directory traversal",
+                       args: args{
+                               cc: &ClientConn{
+                                       ID: &[]byte{0, 1},
+                                       Server: &Server{
+                                               Config: &Config{
+                                                       FileRoot: "/Files/",
+                                               },
+                                       },
+                               },
+                               t: NewTransaction(
+                                       tranNewFolder, &[]byte{0, 1},
+                                       NewField(fieldFileName, []byte("../../testFolder")),
+
+                               ),
+                       },
+                       setup: func() {
+                               mfs := MockFileStore{}
+                               mfs.On("Mkdir", "/Files/testFolder", fs.FileMode(0777)).Return(nil)
+                               mfs.On("Stat", "/Files/testFolder").Return(nil, os.ErrNotExist)
+                               FS = mfs
+                       },
+                       wantRes: []Transaction{
+                               {
+                                       clientID:  &[]byte{0, 1},
+                                       Flags:     0x00,
+                                       IsReply:   0x01,
+                                       Type:      []byte{0, 0xcd},
+                                       ID:        []byte{0x9a, 0xcb, 0x04, 0x42}, // Random ID from rand.Seed(1)
+                                       ErrorCode: []byte{0, 0, 0, 0},
+                               },
+                       },                      wantErr: false,
+               },
+               {
+                       name: "fieldFilePath does not allow directory traversal",
+                       args: args{
+                               cc: &ClientConn{
+                                       ID: &[]byte{0, 1},
+                                       Server: &Server{
+                                               Config: &Config{
+                                                       FileRoot: "/Files/",
+                                               },
+                                       },
+                               },
+                               t: NewTransaction(
+                                       tranNewFolder, &[]byte{0, 1},
+                                       NewField(fieldFileName, []byte("testFolder")),
+                                       NewField(fieldFilePath, []byte{
+                                               0x00, 0x02,
+                                               0x00, 0x00,
+                                               0x03,
+                                               0x2e, 0x2e, 0x2f,
+                                               0x00, 0x00,
+                                               0x03,
+                                               0x66, 0x6f, 0x6f,
+                                       }),
+                               ),
+                       },
+                       setup: func() {
+                               mfs := MockFileStore{}
+                               mfs.On("Mkdir", "/Files/foo/testFolder", fs.FileMode(0777)).Return(nil)
+                               mfs.On("Stat", "/Files/foo/testFolder").Return(nil, os.ErrNotExist)
+                               FS = mfs
+                       },
+                       wantRes: []Transaction{
+                               {
+                                       clientID:  &[]byte{0, 1},
+                                       Flags:     0x00,
+                                       IsReply:   0x01,
+                                       Type:      []byte{0, 0xcd},
+                                       ID:        []byte{0x9a, 0xcb, 0x04, 0x42}, // Random ID from rand.Seed(1)
+                                       ErrorCode: []byte{0, 0, 0, 0},
+                               },
+                       },                      wantErr: false,
+               },
+       }
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       tt.setup()
+
+                       gotRes, err := HandleNewFolder(tt.args.cc, tt.args.t)
+                       if (err != nil) != tt.wantErr {
+                               t.Errorf("HandleNewFolder() error = %v, wantErr %v", err, tt.wantErr)
+                               return
+                       }
+                       if !tranAssertEqual(t, tt.wantRes, gotRes) {
+                               t.Errorf("HandleNewFolder() gotRes = %v, want %v", gotRes, tt.wantRes)
+                       }
+               })
+       }
+}
+
index 6780c6db0aad7e1099db9b214c768816a57ff056..db73b62137c818841f8f29260fce7eed0215e05b 100644 (file)
@@ -1,3 +1,3 @@
 package hotline
 
-const VERSION = "0.0.11"
+const VERSION = "0.0.12"