diff --git a/.github/workflows/static-check.yml b/.github/workflows/static-check.yml new file mode 100644 index 00000000..795f9ef2 --- /dev/null +++ b/.github/workflows/static-check.yml @@ -0,0 +1,26 @@ +name: static-check + +on: + pull_request: + push: + tags: + - 'v*' + branches: + - main + +jobs: + staticcheck: + name: staticcheck (project) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + with: + fetch-depth: 1 + - id: install_go + uses: WillAbides/setup-go-faster@v1.7.0 + with: + go-version: "1.20.x" + - uses: dominikh/staticcheck-action@v1.2.0 + with: + install-go: false + version: "2023.1.7" diff --git a/client/httpclient_test.go b/client/httpclient_test.go index 53664240..6e9cdd31 100644 --- a/client/httpclient_test.go +++ b/client/httpclient_test.go @@ -3,7 +3,7 @@ package client import ( "compress/gzip" "context" - "io/ioutil" + "io" "net/http" "sync/atomic" "testing" @@ -22,10 +22,12 @@ func TestHTTPPolling(t *testing.T) { srv := internal.StartMockServer(t) var rcvCounter int64 srv.OnMessage = func(msg *protobufs.AgentToServer) *protobufs.ServerToAgent { - assert.EqualValues(t, rcvCounter, msg.SequenceNum) - if msg != nil { - atomic.AddInt64(&rcvCounter, 1) + if msg == nil { + t.Error("unexpected nil msg") + return nil } + assert.EqualValues(t, rcvCounter, msg.SequenceNum) + atomic.AddInt64(&rcvCounter, 1) return nil } @@ -63,7 +65,7 @@ func TestHTTPClientCompression(t *testing.T) { assert.Equal(t, "gzip", r.Header.Get("Content-Encoding")) reader, err := gzip.NewReader(r.Body) assert.NoError(t, err) - body, err := ioutil.ReadAll(reader) + body, err := io.ReadAll(reader) assert.NoError(t, err) _ = r.Body.Close() var response protobufs.AgentToServer @@ -112,10 +114,12 @@ func TestHTTPClientSetPollingInterval(t *testing.T) { srv := internal.StartMockServer(t) var rcvCounter int64 srv.OnMessage = func(msg *protobufs.AgentToServer) *protobufs.ServerToAgent { - assert.EqualValues(t, rcvCounter, msg.SequenceNum) - if msg != nil { - atomic.AddInt64(&rcvCounter, 1) + if msg == nil { + t.Error("unexpected nil msg") + return nil } + assert.EqualValues(t, rcvCounter, msg.SequenceNum) + atomic.AddInt64(&rcvCounter, 1) return nil } diff --git a/client/internal/httpsender.go b/client/internal/httpsender.go index 00516423..f3626e4b 100644 --- a/client/internal/httpsender.go +++ b/client/internal/httpsender.go @@ -110,7 +110,6 @@ func (h *HTTPSender) Run( // This will make hasPendingMessage channel readable, so we will enter // the case above on the next iteration of the loop. h.ScheduleSend() - break case <-ctx.Done(): return diff --git a/client/internal/mockserver.go b/client/internal/mockserver.go index 6a85ec64..2d88ac9d 100644 --- a/client/internal/mockserver.go +++ b/client/internal/mockserver.go @@ -40,8 +40,8 @@ const contentTypeProtobuf = "application/x-protobuf" func newMockServer(t *testing.T) (*MockServer, *http.ServeMux) { srv := &MockServer{ t: t, - expectedHandlers: make(chan receivedMessageHandler, 0), - expectedComplete: make(chan struct{}, 0), + expectedHandlers: make(chan receivedMessageHandler), + expectedComplete: make(chan struct{}), } m := http.NewServeMux() @@ -112,6 +112,11 @@ func (m *MockServer) EnableExpectMode() { func (m *MockServer) handlePlainHttp(w http.ResponseWriter, r *http.Request) { msgBytes, err := io.ReadAll(r.Body) + if err != nil { + // request could not be read for some reason + http.Error(w, err.Error(), 500) + return + } // We use alwaysRespond=true here because plain HTTP requests must always have // a response. diff --git a/client/internal/packagessyncer.go b/client/internal/packagessyncer.go index f6814401..bdb52fde 100644 --- a/client/internal/packagessyncer.go +++ b/client/internal/packagessyncer.go @@ -104,7 +104,7 @@ func (s *packagesSyncer) doSync(ctx context.Context) { s.logger.Errorf(ctx, "Package syncing failed: %V", err) return } - if bytes.Compare(hash, s.available.AllPackagesHash) == 0 { + if bytes.Equal(hash, s.available.AllPackagesHash) { s.logger.Debugf(ctx, "All packages are already up to date.") return } @@ -243,7 +243,7 @@ func (s *packagesSyncer) shouldDownloadFile(ctx context.Context, packageName str } else { // Compare the checksum of the file we have with what // we are offered by the server. - if bytes.Compare(fileContentHash, file.ContentHash) != 0 { + if !bytes.Equal(fileContentHash, file.ContentHash) { s.logger.Debugf(ctx, "Package %s: file hash mismatch, will download.", packageName) return true, nil } diff --git a/client/wsclient.go b/client/wsclient.go index db179997..2b5ab5ad 100644 --- a/client/wsclient.go +++ b/client/wsclient.go @@ -128,7 +128,7 @@ func (c *wsClient) SetPackageStatuses(statuses *protobufs.PackageStatuses) error // Try to connect once. Returns an error if connection fails and optional retryAfter // duration to indicate to the caller to retry after the specified time as instructed // by the Server. -func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sharedinternal.OptionalDuration) { +func (c *wsClient) tryConnectOnce(ctx context.Context) (retryAfter sharedinternal.OptionalDuration, err error) { var resp *http.Response conn, resp, err := c.dialer.DialContext(ctx, c.url.String(), c.requestHeader) if err != nil { @@ -142,7 +142,7 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh redirect, err := resp.Location() if err != nil { c.common.Logger.Errorf(ctx, "%d redirect, but no valid location: %s", resp.StatusCode, err) - return err, duration + return duration, err } // rewrite the scheme for the sake of tolerance if redirect.Scheme == "http" { @@ -157,9 +157,9 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh } else { c.common.Logger.Errorf(ctx, "Server responded with status=%v", resp.Status) } - return err, duration + return duration, err } - return err, sharedinternal.OptionalDuration{Defined: false} + return sharedinternal.OptionalDuration{Defined: false}, err } // Successfully connected. @@ -170,7 +170,7 @@ func (c *wsClient) tryConnectOnce(ctx context.Context) (err error, retryAfter sh c.common.Callbacks.OnConnect(ctx) } - return nil, sharedinternal.OptionalDuration{Defined: false} + return sharedinternal.OptionalDuration{Defined: false}, nil } // Continuously try until connected. Will return nil when successfully @@ -190,7 +190,7 @@ func (c *wsClient) ensureConnected(ctx context.Context) error { select { case <-timer.C: { - if err, retryAfter := c.tryConnectOnce(ctx); err != nil { + if retryAfter, err := c.tryConnectOnce(ctx); err != nil { c.lastInternalErr.Store(&err) if errors.Is(err, context.Canceled) { c.common.Logger.Debugf(ctx, "Client is stopped, will not try anymore.") diff --git a/internal/certs.go b/internal/certs.go index 7b86e078..75cbbf04 100644 --- a/internal/certs.go +++ b/internal/certs.go @@ -15,7 +15,6 @@ import ( "encoding/pem" "errors" "fmt" - "io/ioutil" "math/big" "net" "os" @@ -79,19 +78,18 @@ func CreateServerTLSConfig(caCertPath, serverCertPath, serverKeyPath string) (*t InsecureSkipVerify: true, ClientCAs: caCertPool, } - tlsConfig.BuildNameToCertificate() return tlsConfig, nil } func CreateTLSCert(caCertPath, caKeyPath string) (*protobufs.TLSCertificate, error) { // Load CA Cert. - caCertBytes, err := ioutil.ReadFile(caCertPath) + caCertBytes, err := os.ReadFile(caCertPath) if err != nil { return nil, fmt.Errorf("cannot read CA cert: %v", err) } - caKeyBytes, err := ioutil.ReadFile(caKeyPath) + caKeyBytes, err := os.ReadFile(caKeyPath) if err != nil { return nil, fmt.Errorf("cannot read CA key: %v", err) } diff --git a/internal/retryafter_test.go b/internal/retryafter_test.go index 20bc1997..f2e36e9f 100644 --- a/internal/retryafter_test.go +++ b/internal/retryafter_test.go @@ -1,12 +1,13 @@ package internal import ( - "github.com/stretchr/testify/assert" "math/rand" "net/http" "strconv" "testing" "time" + + "github.com/stretchr/testify/assert" ) func response503() *http.Response { @@ -32,7 +33,6 @@ func assertDuration(t *testing.T, duration OptionalDuration, expected time.Durat func TestExtractRetryAfterHeaderDelaySeconds(t *testing.T) { // Generate random n > 0 int - rand.Seed(time.Now().UnixNano()) retryIntervalSec := rand.Intn(9999) // Generate a 503 status code response with Retry-After = n header @@ -58,7 +58,6 @@ func TestExtractRetryAfterHeaderDelaySeconds(t *testing.T) { func TestExtractRetryAfterHeaderHttpDate(t *testing.T) { // Generate a random n > 0 second duration now := time.Now() - rand.Seed(now.UnixNano()) retryIntervalSec := rand.Intn(9999) expectedDuration := time.Second * time.Duration(retryIntervalSec) diff --git a/internal/wsmessage.go b/internal/wsmessage.go index 541d60b7..8a1a0cc8 100644 --- a/internal/wsmessage.go +++ b/internal/wsmessage.go @@ -11,6 +11,7 @@ import ( // Message header is currently uint64 zero value. const wsMsgHeader = uint64(0) +// DecodeWSMessage decodes a websocket message as bytes into a proto.Message. func DecodeWSMessage(bytes []byte, msg proto.Message) error { // Message header is optional until the end of grace period that ends Feb 1, 2023. // Check if the header is present. @@ -23,9 +24,9 @@ func DecodeWSMessage(bytes []byte, msg proto.Message) error { } // Skip the header. It really is just a single zero byte for now. bytes = bytes[n:] - } else { - // Old message format. No header present. } + // If no header was present (the "if" check above), then this is the old + // message format. No header is present. // Decode WebSocket message as a Protobuf message. err := proto.Unmarshal(bytes, msg) diff --git a/protobufshelpers/anyvaluehelpers.go b/protobufshelpers/anyvaluehelpers.go index 898fd994..1da9e639 100644 --- a/protobufshelpers/anyvaluehelpers.go +++ b/protobufshelpers/anyvaluehelpers.go @@ -39,7 +39,7 @@ func IsEqualAnyValue(v1, v2 *protobufs.AnyValue) bool { case *protobufs.AnyValue_BytesValue: v2, ok := v2.Value.(*protobufs.AnyValue_BytesValue) - return ok && bytes.Compare(v1.BytesValue, v2.BytesValue) == 0 + return ok && bytes.Equal(v1.BytesValue, v2.BytesValue) case *protobufs.AnyValue_ArrayValue: v2, ok := v2.Value.(*protobufs.AnyValue_ArrayValue) diff --git a/server/serverimpl_test.go b/server/serverimpl_test.go index bc04a6d9..8f473de7 100644 --- a/server/serverimpl_test.go +++ b/server/serverimpl_test.go @@ -628,6 +628,7 @@ func TestServerHonoursClientRequestContentEncoding(t *testing.T) { require.NoError(t, err) req, err := http.NewRequest("POST", "http://"+settings.ListenEndpoint+settings.ListenPath, bytes.NewReader(b)) + require.NoError(t, err) req.Header.Set(headerContentType, contentTypeProtobuf) req.Header.Set(headerContentEncoding, contentEncodingGzip) resp, err := hc.Do(req) @@ -700,6 +701,7 @@ func TestServerHonoursAcceptEncoding(t *testing.T) { b, err := proto.Marshal(&sendMsg) require.NoError(t, err) req, err := http.NewRequest("POST", "http://"+settings.ListenEndpoint+settings.ListenPath, bytes.NewReader(b)) + require.NoError(t, err) req.Header.Set(headerContentType, contentTypeProtobuf) req.Header.Set(headerAcceptEncoding, contentEncodingGzip) resp, err := hc.Do(req)