From: Jeff Halter Date: Sun, 15 Aug 2021 17:39:43 +0000 (-0700) Subject: Tests and minor fixes X-Git-Url: https://git.r.bdr.sh/rbdr/mobius/commitdiff_plain/00d1ef67636df59460bd4e060f6da4b0c9bcb24c Tests and minor fixes --- diff --git a/cmd/mobius-hotline-server/main.go b/cmd/mobius-hotline-server/main.go index 772625b..cf69ea8 100644 --- a/cmd/mobius-hotline-server/main.go +++ b/cmd/mobius-hotline-server/main.go @@ -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) diff --git a/hotline/client.go b/hotline/client.go index cf88d8a..33ec511 100644 --- a/hotline/client.go +++ b/hotline/client.go @@ -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() } diff --git a/hotline/file_path.go b/hotline/file_path.go index 733dfaa..d6c05bc 100644 --- a/hotline/file_path.go +++ b/hotline/file_path.go @@ -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 { diff --git a/hotline/file_path_test.go b/hotline/file_path_test.go index a5f5b60..a8ab2ce 100644 --- a/hotline/file_path_test.go +++ b/hotline/file_path_test.go @@ -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 index 0000000..81c1f05 --- /dev/null +++ b/hotline/file_store.go @@ -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) +} diff --git a/hotline/server_blackbox_test.go b/hotline/server_blackbox_test.go index 09687ad..64d56ab 100644 --- a/hotline/server_blackbox_test.go +++ b/hotline/server_blackbox_test.go @@ -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 diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go index d7a77e2..9e61427 100644 --- a/hotline/transaction_handlers.go +++ b/hotline/transaction_handlers.go @@ -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)) diff --git a/hotline/transaction_handlers_test.go b/hotline/transaction_handlers_test.go index e62cce2..ec947c9 100644 --- a/hotline/transaction_handlers_test.go +++ b/hotline/transaction_handlers_test.go @@ -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) + } + }) + } +} + diff --git a/hotline/version.go b/hotline/version.go index 6780c6d..db73b62 100644 --- a/hotline/version.go +++ b/hotline/version.go @@ -1,3 +1,3 @@ package hotline -const VERSION = "0.0.11" +const VERSION = "0.0.12"