From: Jeff Halter Date: Tue, 18 Jun 2024 02:51:37 +0000 (-0700) Subject: Clean up various linter warnings X-Git-Url: https://git.r.bdr.sh/rbdr/mobius/commitdiff_plain/f8e4cd540b87de3e308ec18a2b040b284a741522?ds=sidebyside Clean up various linter warnings --- diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 0000000..6f2d802 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,25 @@ +name: golangci-lint +on: + pull_request: + types: [opened, reopened] + +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + pull-requests: read + # Optional: allow write access to checks to allow the action to annotate code in the PR. + checks: write + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: stable + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v1.58 \ No newline at end of file diff --git a/cmd/mobius-hotline-server/main.go b/cmd/mobius-hotline-server/main.go index 66566cd..c34dc71 100644 --- a/cmd/mobius-hotline-server/main.go +++ b/cmd/mobius-hotline-server/main.go @@ -99,7 +99,7 @@ func main() { } if _, err := os.Stat(*configDir); os.IsNotExist(err) { - slogger.Error(fmt.Sprintf("Configuration directory not found. Correct the path or re-run with -init to generate initial config.")) + slogger.Error("Configuration directory not found. Correct the path or re-run with -init to generate initial config.") os.Exit(1) } @@ -145,7 +145,9 @@ func (sh *statHandler) RenderStats(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, string(u)) } -func defaultConfigPath() (cfgPath string) { +func defaultConfigPath() string { + var cfgPath string + switch runtime.GOOS { case "windows": cfgPath = "config/" diff --git a/go.mod b/go.mod index 5fe9eaf..24c803e 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/jhalter/mobius go 1.22 require ( - github.com/davecgh/go-spew v1.1.1 github.com/go-playground/validator/v10 v10.19.0 github.com/stretchr/testify v1.9.0 golang.org/x/crypto v0.22.0 @@ -13,6 +12,7 @@ require ( ) require ( + github.com/davecgh/go-spew v1.1.1 // indirect github.com/gabriel-vasile/mimetype v1.4.3 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index 7c24109..194e8b9 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -1,10 +1,10 @@ package hotline import ( + "crypto/rand" "encoding/binary" "fmt" "math" - "math/rand" "path/filepath" "sync" ) @@ -51,7 +51,7 @@ func (wc *WriteCounter) Write(p []byte) (int, error) { func (cc *ClientConn) newFileTransfer(transferType int, fileName, filePath, size []byte) *FileTransfer { var transactionRef [4]byte - rand.Read(transactionRef[:]) + _, _ = rand.Read(transactionRef[:]) ft := &FileTransfer{ FileName: fileName, diff --git a/hotline/files.go b/hotline/files.go index 901b98c..6753cce 100644 --- a/hotline/files.go +++ b/hotline/files.go @@ -40,8 +40,7 @@ func getFileNameList(path string, ignoreList []string) (fields []Field, err erro return fields, fmt.Errorf("error reading path: %s: %w", path, err) } - for i, _ := range files { - file := files[i] + for _, file := range files { var fnwi FileNameWithInfo if ignoreFile(file.Name(), ignoreList) { diff --git a/hotline/flattened_file_object.go b/hotline/flattened_file_object.go index 6419a1f..b178c6c 100644 --- a/hotline/flattened_file_object.go +++ b/hotline/flattened_file_object.go @@ -189,7 +189,7 @@ func (ffif *FlatFileInformationFork) Write(p []byte) (int, error) { ffif.Comment = p[commentStartPos:commentEndPos] - total = uint16(commentEndPos) + //total = uint16(commentEndPos) } return len(p), nil diff --git a/hotline/server.go b/hotline/server.go index 448aab1..5245c7b 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "context" + "crypto/rand" "encoding/binary" "errors" "fmt" @@ -15,7 +16,6 @@ import ( "log" "log/slog" "math/big" - "math/rand" "net" "os" "path" @@ -760,7 +760,8 @@ func (s *Server) NewPrivateChat(cc *ClientConn) []byte { defer s.PrivateChatsMu.Unlock() randID := make([]byte, 4) - rand.Read(randID) + _, _ = rand.Read(randID) + data := binary.BigEndian.Uint32(randID) s.PrivateChats[data] = &PrivateChat{ diff --git a/hotline/server_blackbox_test.go b/hotline/server_blackbox_test.go index 059fe5a..b45e279 100644 --- a/hotline/server_blackbox_test.go +++ b/hotline/server_blackbox_test.go @@ -30,7 +30,7 @@ func assertTransferBytesEqual(t *testing.T, wantHexDump string, got []byte) bool return assert.Equal(t, wantHexDump, hex.Dump(clean)) } -// tranAssertEqual compares equality of transactions slices after stripping out the random ID +// tranAssertEqual compares equality of transactions slices after stripping out the random transaction ID func tranAssertEqual(t *testing.T, tran1, tran2 []Transaction) bool { var newT1 []Transaction var newT2 []Transaction @@ -39,7 +39,10 @@ func tranAssertEqual(t *testing.T, tran1, tran2 []Transaction) bool { trans.ID = []byte{0, 0, 0, 0} var fs []Field for _, field := range trans.Fields { - if field.ID == [2]byte{0x00, 0x6b} { + if field.ID == [2]byte{0x00, 0x6b} { // FieldRefNum + continue + } + if field.ID == [2]byte{0x00, 0x72} { // FieldChatID continue } fs = append(fs, field) @@ -52,7 +55,10 @@ func tranAssertEqual(t *testing.T, tran1, tran2 []Transaction) bool { trans.ID = []byte{0, 0, 0, 0} var fs []Field for _, field := range trans.Fields { - if field.ID == [2]byte{0x00, 0x6b} { + if field.ID == [2]byte{0x00, 0x6b} { // FieldRefNum + continue + } + if field.ID == [2]byte{0x00, 0x72} { // FieldChatID continue } fs = append(fs, field) diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go index 20b87e2..0fb250f 100644 --- a/hotline/transaction_handlers.go +++ b/hotline/transaction_handlers.go @@ -1256,8 +1256,9 @@ func HandleNewNewsFldr(cc *ClientConn, t *Transaction) (res []Transaction, err e func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction, err error) { if !cc.Authorize(accessNewsReadArt) { res = append(res, cc.NewErrReply(t, "You are not allowed to read news.")) - return res, err + return res, nil } + pathStrs := ReadNewsPath(t.GetField(FieldNewsPath).Data) var cat NewsCategoryListData15 @@ -1272,11 +1273,11 @@ func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction b, err := io.ReadAll(&nald) if err != nil { - + return res, fmt.Errorf("error loading news articles: %w", err) } res = append(res, cc.NewReply(t, NewField(FieldNewsArtListData, b))) - return res, err + return res, nil } // HandleGetNewsArtData requests information about the specific news article. diff --git a/hotline/transaction_handlers_test.go b/hotline/transaction_handlers_test.go index 9c0359f..09cb1cb 100644 --- a/hotline/transaction_handlers_test.go +++ b/hotline/transaction_handlers_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/mock" "io" "io/fs" - "math/rand" "os" "path/filepath" "strings" @@ -109,15 +108,13 @@ func TestHandleSetChatSubject(t *testing.T) { }, } for _, tt := range tests { - rand.Seed(1) // reset seed between tests to make transaction IDs predictable - t.Run(tt.name, func(t *testing.T) { got, err := HandleSetChatSubject(tt.args.cc, tt.args.t) if (err != nil) != tt.wantErr { t.Errorf("HandleSetChatSubject() error = %v, wantErr %v", err, tt.wantErr) return } - if !assert.Equal(t, tt.want, got) { + if !tranAssertEqual(t, tt.want, got) { t.Errorf("HandleSetChatSubject() got = %v, want %v", got, tt.want) } }) @@ -195,14 +192,13 @@ func TestHandleLeaveChat(t *testing.T) { }, } for _, tt := range tests { - rand.Seed(1) t.Run(tt.name, func(t *testing.T) { got, err := HandleLeaveChat(tt.args.cc, tt.args.t) if (err != nil) != tt.wantErr { t.Errorf("HandleLeaveChat() error = %v, wantErr %v", err, tt.wantErr) return } - if !assert.Equal(t, tt.want, got) { + if !tranAssertEqual(t, tt.want, got) { t.Errorf("HandleLeaveChat() got = %v, want %v", got, tt.want) } }) @@ -724,8 +720,6 @@ func TestHandleChatSend(t *testing.T) { } func TestHandleGetFileInfo(t *testing.T) { - rand.Seed(1) // reset seed between tests to make transaction IDs predictable - type args struct { cc *ClientConn t *Transaction @@ -781,8 +775,6 @@ func TestHandleGetFileInfo(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rand.Seed(1) // reset seed between tests to make transaction IDs predictable - gotRes, err := HandleGetFileInfo(tt.args.cc, tt.args.t) if (err != nil) != tt.wantErr { t.Errorf("HandleGetFileInfo() error = %v, wantErr %v", err, tt.wantErr) @@ -793,7 +785,8 @@ func TestHandleGetFileInfo(t *testing.T) { // TODO: revisit how to test this by mocking the stat calls gotRes[0].Fields[4].Data = make([]byte, 8) gotRes[0].Fields[5].Data = make([]byte, 8) - if !assert.Equal(t, tt.wantRes, gotRes) { + + if !tranAssertEqual(t, tt.wantRes, gotRes) { t.Errorf("HandleGetFileInfo() gotRes = %v, want %v", gotRes, tt.wantRes) } }) @@ -1166,7 +1159,6 @@ func TestHandleUploadFile(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rand.Seed(1) gotRes, err := HandleUploadFile(tt.args.cc, tt.args.t) if (err != nil) != tt.wantErr { t.Errorf("HandleUploadFile() error = %v, wantErr %v", err, tt.wantErr) @@ -3592,7 +3584,6 @@ func TestHandleInviteNewChat(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rand.Seed(1) gotRes, err := HandleInviteNewChat(tt.args.cc, tt.args.t) if !tt.wantErr(t, err, fmt.Sprintf("HandleInviteNewChat(%v, %v)", tt.args.cc, tt.args.t)) { return @@ -3699,6 +3690,78 @@ func TestHandleGetNewsArtNameList(t *testing.T) { }, wantErr: assert.NoError, }, + { + name: "when user has required access", + args: args{ + cc: &ClientConn{ + Account: &Account{ + Access: func() accessBitmap { + var bits accessBitmap + bits.Set(accessNewsReadArt) + return bits + }(), + }, + Server: &Server{ + ThreadedNews: &ThreadedNews{ + Categories: map[string]NewsCategoryListData15{ + "Example Category": { + Type: [2]byte{0, 2}, + Name: "", + Articles: map[uint32]*NewsArtData{ + uint32(1): { + Title: "testTitle", + Poster: "testPoster", + Data: "testBody", + }, + }, + SubCats: nil, + GUID: [16]byte{}, + AddSN: [4]byte{}, + DeleteSN: [4]byte{}, + }, + }, + }, + + //Accounts: map[string]*Account{ + // "guest": { + // Name: "guest", + // Login: "guest", + // Password: "zz", + // Access: accessBitmap{255, 255, 255, 255, 255, 255, 255, 255}, + // }, + //}, + }, + }, + t: NewTransaction( + TranGetNewsArtNameList, + &[]byte{0, 1}, + // 00000000 00 01 00 00 10 45 78 61 6d 70 6c 65 20 43 61 74 |.....Example Cat| + // 00000010 65 67 6f 72 79 |egory| + NewField(FieldNewsPath, []byte{ + 0x00, 0x01, 0x00, 0x00, 0x10, 0x45, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x20, 0x43, 0x61, 0x74, 0x65, 0x67, 0x6f, 0x72, 0x79, + }), + ), + }, + wantRes: []Transaction{ + { + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0, 0}, + ErrorCode: []byte{0, 0, 0, 0}, + Fields: []Field{ + NewField(FieldNewsArtListData, []byte{ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x09, 0x74, 0x65, 0x73, 0x74, 0x54, 0x69, 0x74, 0x6c, 0x65, 0x0a, 0x74, 0x65, 0x73, 0x74, 0x50, + 0x6f, 0x73, 0x74, 0x65, 0x72, 0x0a, 0x74, 0x65, 0x78, 0x74, 0x2f, 0x70, 0x6c, 0x61, 0x69, 0x6e, + 0x00, 0x08, + }, + ), + }, + }, + }, + wantErr: assert.NoError, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {