From 3c009271f82b401a7aa670d6987a76193c7b0a37 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 2 Sep 2022 12:21:24 +0300 Subject: [PATCH] golangci: enable bodyclose checker and fix related code It has found an issue in the oracle code, so I think it's worth doing. --- .golangci.yml | 1 + pkg/rpcclient/wsclient.go | 5 ++++- pkg/services/oracle/network_test.go | 2 +- pkg/services/oracle/request.go | 1 + pkg/services/oracle/response.go | 2 -- pkg/services/rpcsrv/server_test.go | 4 +++- pkg/services/rpcsrv/subscription_test.go | 8 ++++++-- 7 files changed, 16 insertions(+), 7 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5c3ad60e1..2fed511a0 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -42,6 +42,7 @@ linters: # extra linters # - exhaustive + - bodyclose - gofmt - whitespace - goimports diff --git a/pkg/rpcclient/wsclient.go b/pkg/rpcclient/wsclient.go index ab77f141d..d1284716b 100644 --- a/pkg/rpcclient/wsclient.go +++ b/pkg/rpcclient/wsclient.go @@ -90,7 +90,10 @@ var errConnClosedByUser = errors.New("connection closed by user") // operating on. func NewWS(ctx context.Context, endpoint string, opts Options) (*WSClient, error) { dialer := websocket.Dialer{HandshakeTimeout: opts.DialTimeout} - ws, _, err := dialer.Dial(endpoint, nil) + ws, resp, err := dialer.Dial(endpoint, nil) + if resp != nil && resp.Body != nil { // Can be non-nil even with error returned. + defer resp.Body.Close() // Not exactly required by websocket, but let's do this for bodyclose checker. + } if err != nil { return nil, err } diff --git a/pkg/services/oracle/network_test.go b/pkg/services/oracle/network_test.go index 37d623939..de132508a 100644 --- a/pkg/services/oracle/network_test.go +++ b/pkg/services/oracle/network_test.go @@ -40,7 +40,7 @@ func TestDefaultClient_RestrictedRedirectErr(t *testing.T) { } for _, c := range testCases { t.Run(c, func(t *testing.T) { - _, err := cl.Get(c) + _, err := cl.Get(c) //nolint:bodyclose // It errors out and it's a test. require.Error(t, err) require.True(t, errors.Is(err, ErrRestrictedRedirect), err) require.True(t, strings.Contains(err.Error(), "IP is not global unicast"), err) diff --git a/pkg/services/oracle/request.go b/pkg/services/oracle/request.go index 1b6ed6197..fddc9d771 100644 --- a/pkg/services/oracle/request.go +++ b/pkg/services/oracle/request.go @@ -138,6 +138,7 @@ func (o *Oracle) processRequest(priv *keys.PrivateKey, req request) error { o.Log.Warn("oracle request failed", zap.String("url", req.Req.URL), zap.Error(err), zap.Stringer("code", resp.Code)) break } + defer r.Body.Close() switch r.StatusCode { case http.StatusOK: if !checkMediaType(r.Header.Get("Content-Type"), o.MainCfg.AllowedContentTypes) { diff --git a/pkg/services/oracle/response.go b/pkg/services/oracle/response.go index b57be2f3b..7d37036e0 100644 --- a/pkg/services/oracle/response.go +++ b/pkg/services/oracle/response.go @@ -67,8 +67,6 @@ func (o *Oracle) AddResponse(pub *keys.PublicKey, reqID uint64, txSig []byte) { var ErrResponseTooLarge = errors.New("too big response") func readResponse(rc gio.ReadCloser, limit int) ([]byte, error) { - defer rc.Close() - buf := make([]byte, limit+1) n, err := gio.ReadFull(rc, buf) if err == gio.ErrUnexpectedEOF && n <= limit { diff --git a/pkg/services/rpcsrv/server_test.go b/pkg/services/rpcsrv/server_test.go index d5a9fce4a..e0e2ee86a 100644 --- a/pkg/services/rpcsrv/server_test.go +++ b/pkg/services/rpcsrv/server_test.go @@ -2633,8 +2633,9 @@ func checkErrGetBatchResult(t *testing.T, body []byte, expectingFail bool) json. func doRPCCallOverWS(rpcCall string, url string, t *testing.T) []byte { dialer := websocket.Dialer{HandshakeTimeout: time.Second} url = "ws" + strings.TrimPrefix(url, "http") - c, _, err := dialer.Dial(url+"/ws", nil) + c, r, err := dialer.Dial(url+"/ws", nil) require.NoError(t, err) + defer r.Body.Close() err = c.SetWriteDeadline(time.Now().Add(time.Second)) require.NoError(t, err) require.NoError(t, c.WriteMessage(1, []byte(rpcCall))) @@ -2651,6 +2652,7 @@ func doRPCCallOverHTTP(rpcCall string, url string, t *testing.T) []byte { resp, err := cl.Post(url, "application/json", strings.NewReader(rpcCall)) require.NoErrorf(t, err, "could not make a POST request") body, err := gio.ReadAll(resp.Body) + resp.Body.Close() assert.NoErrorf(t, err, "could not read response from the request: %s", rpcCall) return bytes.TrimSpace(body) } diff --git a/pkg/services/rpcsrv/subscription_test.go b/pkg/services/rpcsrv/subscription_test.go index 1a5e5a957..41a8191dc 100644 --- a/pkg/services/rpcsrv/subscription_test.go +++ b/pkg/services/rpcsrv/subscription_test.go @@ -59,8 +59,9 @@ func initCleanServerAndWSClient(t *testing.T) (*core.Blockchain, *Server, *webso dialer := websocket.Dialer{HandshakeTimeout: time.Second} url := "ws" + strings.TrimPrefix(httpSrv.URL, "http") + "/ws" - ws, _, err := dialer.Dial(url, nil) + ws, r, err := dialer.Dial(url, nil) require.NoError(t, err) + defer r.Body.Close() // Use buffered channel to read server's messages and then read expected // responses from it. @@ -520,7 +521,10 @@ func TestWSClientsLimit(t *testing.T) { wss := make([]*websocket.Conn, maxSubscribers) for i := 0; i < len(wss)+1; i++ { - ws, _, err := dialer.Dial(url, nil) + ws, r, err := dialer.Dial(url, nil) + if r != nil && r.Body != nil { + defer r.Body.Close() + } if i < maxSubscribers { require.NoError(t, err) wss[i] = ws