From 707b20e1a59c6e34a2644a54805476ebcade7897 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 12 Sep 2024 23:31:26 +0300 Subject: [PATCH 01/11] handler: don't use min/max names for variables api/handler/put.go:132:4 predeclared variable min has same name as predeclared identifier api/handler/put.go:133:4 predeclared variable max has same name as predeclared identifier Signed-off-by: Roman Khimov --- api/handler/put.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/handler/put.go b/api/handler/put.go index 7c65a047..484845fd 100644 --- a/api/handler/put.go +++ b/api/handler/put.go @@ -129,13 +129,13 @@ func (p *policyCondition) UnmarshalJSON(data []byte) error { } if p.Matching == "content-length-range" { - min, ok := v[1].(float64) - max, ok2 := v[2].(float64) + minV, ok := v[1].(float64) + maxV, ok2 := v[2].(float64) if !ok || !ok2 { return errInvalidCondition } - p.Key = strconv.FormatFloat(min, 'f', 0, 32) - p.Value = strconv.FormatFloat(max, 'f', 0, 32) + p.Key = strconv.FormatFloat(minV, 'f', 0, 32) + p.Value = strconv.FormatFloat(maxV, 'f', 0, 32) } else { key, ok2 := v[1].(string) p.Value, ok = v[2].(string) From 05f32246cfac73bf1b83b713e3c9dd1ca91a6a5a Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 12 Sep 2024 23:37:51 +0300 Subject: [PATCH 02/11] *: fix type assertions and switches for wrapped errors api/response.go:116:14 errorlint type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors api/response.go:234:14 errorlint type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors api/s3errors/errors.go:1689:11 errorlint type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors api/handler/delete.go:145:9 errorlint type switch on error will fail on wrapped errors. Use errors.As to check for specific errors api/handler/delete.go:230:20 errorlint type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors internal/neofs/neofs.go:593:16 errorlint type switch on error will fail on wrapped errors. Use errors.As to check for specific errors Signed-off-by: Roman Khimov --- api/handler/delete.go | 20 ++++++++++++++------ api/response.go | 27 +++++++++++++++++---------- api/s3errors/errors.go | 8 ++++++-- internal/neofs/neofs.go | 20 +++++++++----------- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/api/handler/delete.go b/api/handler/delete.go index a6d1aefc..a2e93254 100644 --- a/api/handler/delete.go +++ b/api/handler/delete.go @@ -2,6 +2,7 @@ package handler import ( "encoding/xml" + "errors" "net/http" "strconv" "strings" @@ -142,12 +143,15 @@ func (h *handler) DeleteObjectHandler(w http.ResponseWriter, r *http.Request) { } func isErrObjectLocked(err error) bool { - switch err.(type) { + var ( + ol apistatus.ObjectLocked + olp *apistatus.ObjectLocked + ) + switch { + case errors.As(err, &ol), errors.As(err, &olp): + return true default: return strings.Contains(err.Error(), "object is locked") - case apistatus.ObjectLocked, - *apistatus.ObjectLocked: - return true } } @@ -226,8 +230,12 @@ func (h *handler) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *http.Re var errs []error for _, obj := range deletedObjects { if obj.Error != nil { - code := "BadRequest" - if s3err, ok := obj.Error.(s3errors.Error); ok { + var ( + code = "BadRequest" + s3err s3errors.Error + ) + + if errors.As(obj.Error, &s3err) { code = s3err.Code } response.Errors = append(response.Errors, DeleteError{ diff --git a/api/response.go b/api/response.go index f5226dcc..fbed8115 100644 --- a/api/response.go +++ b/api/response.go @@ -3,6 +3,7 @@ package api import ( "bytes" "encoding/xml" + "errors" "fmt" "net/http" "strconv" @@ -111,12 +112,15 @@ var s3ErrorResponseMap = map[string]string{ // WriteErrorResponse writes error headers. func WriteErrorResponse(w http.ResponseWriter, reqInfo *ReqInfo, err error) int { - code := http.StatusInternalServerError + var ( + code = http.StatusInternalServerError + s3err s3errors.Error + ) - if e, ok := err.(s3errors.Error); ok { - code = e.HTTPStatusCode + if errors.As(err, &s3err) { + code = s3err.HTTPStatusCode - switch e.Code { + switch s3err.Code { case "SlowDown", "XNeoFSServerNotInitialized", "XNeoFSReadQuorum", "XNeoFSWriteQuorum": // Set retry-after header to indicate user-agents to retry request after 120secs. // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After @@ -228,12 +232,15 @@ func (e ErrorResponse) Error() string { // getErrorResponse gets in standard error and resource value and // provides an encodable populated response values. func getAPIErrorResponse(info *ReqInfo, err error) ErrorResponse { - code := "InternalError" - desc := err.Error() - - if e, ok := err.(s3errors.Error); ok { - code = e.Code - desc = e.Description + var ( + code = "InternalError" + desc = err.Error() + s3err s3errors.Error + ) + + if errors.As(err, &s3err) { + code = s3err.Code + desc = s3err.Description } var resource string diff --git a/api/s3errors/errors.go b/api/s3errors/errors.go index 7700de75..36421cea 100644 --- a/api/s3errors/errors.go +++ b/api/s3errors/errors.go @@ -1,6 +1,7 @@ package s3errors import ( + "errors" "fmt" "net/http" ) @@ -1686,8 +1687,11 @@ var errorCodes = errorCodeMap{ // IsS3Error checks if the provided error is a specific s3 error. func IsS3Error(err error, code ErrorCode) bool { - e, ok := err.(Error) - return ok && e.ErrCode == code + var e Error + if errors.As(err, &e) { + return e.ErrCode == code + } + return false } func (e errorCodeMap) toAPIErrWithErr(errCode ErrorCode, err error) Error { diff --git a/internal/neofs/neofs.go b/internal/neofs/neofs.go index 660ca5a6..1c797e38 100644 --- a/internal/neofs/neofs.go +++ b/internal/neofs/neofs.go @@ -584,19 +584,17 @@ func (x *NeoFS) CurrentEpoch() uint64 { } func isErrAccessDenied(err error) (string, bool) { - unwrappedErr := errors.Unwrap(err) - for unwrappedErr != nil { - err = unwrappedErr - unwrappedErr = errors.Unwrap(err) - } - - switch err := err.(type) { + var ( + oad apistatus.ObjectAccessDenied + oadp *apistatus.ObjectAccessDenied + ) + switch { + case errors.As(err, &oad): + return oad.Reason(), true + case errors.As(err, &oadp): + return oadp.Reason(), true default: return "", false - case apistatus.ObjectAccessDenied: - return err.Reason(), true - case *apistatus.ObjectAccessDenied: - return err.Reason(), true } } From 7177803b62c3d051f07be3893ab3d517e07ba20f Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 15:16:53 +0300 Subject: [PATCH 03/11] neofs: fix error wrapping internal/neofs/tree.go:103:49 errorlint non-wrapping format verb for fmt.Errorf. Use `%w` to format errors Signed-off-by: Roman Khimov --- internal/neofs/tree.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/neofs/tree.go b/internal/neofs/tree.go index 7da5f395..4844734b 100644 --- a/internal/neofs/tree.go +++ b/internal/neofs/tree.go @@ -100,7 +100,7 @@ const ( func NewTreeClient(ctx context.Context, addr string, key *keys.PrivateKey) (*TreeClient, error) { conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - return nil, fmt.Errorf("did not connect: %v", err) + return nil, fmt.Errorf("did not connect: %w", err) } c := tree.NewTreeServiceClient(conn) From 96c6c292d2db6c1bcf854309f58685a7167307f3 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 15:18:50 +0300 Subject: [PATCH 04/11] *: fix error comparisons api/auth/signer/v4/chunked_reader.go:140:8 errorlint comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error api/auth/signer/v4/chunked_reader.go:179:13 errorlint comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error api/auth/signer/v4/chunked_reader.go:195:6 errorlint comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error cmd/s3-gw/service.go:23:20 errorlint comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error Signed-off-by: Roman Khimov --- api/auth/signer/v4/chunked_reader.go | 6 +++--- cmd/s3-gw/service.go | 3 ++- internal/neofs/tree.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/api/auth/signer/v4/chunked_reader.go b/api/auth/signer/v4/chunked_reader.go index 5308d301..ce1c1acb 100644 --- a/api/auth/signer/v4/chunked_reader.go +++ b/api/auth/signer/v4/chunked_reader.go @@ -137,7 +137,7 @@ func (cr *chunkedReader) Read(b []uint8) (n int, err error) { break } } else { - if cr.err == io.EOF { + if errors.Is(cr.err, io.EOF) { cr.err = io.ErrUnexpectedEOF } break @@ -176,7 +176,7 @@ func (cr *chunkedReader) Read(b []uint8) (n int, err error) { // bytes to verify they are "\r\n". if cr.n == 0 && cr.err == nil { cr.checkEnd = true - } else if cr.err == io.EOF { + } else if errors.Is(cr.err, io.EOF) { cr.err = io.ErrUnexpectedEOF } } @@ -192,7 +192,7 @@ func readChunkLine(b *bufio.Reader) ([]byte, []byte, error) { if err != nil { // We always know when EOF is coming. // If the caller asked for a line, there should be a line. - if err == io.EOF { + if errors.Is(err, io.EOF) { err = io.ErrUnexpectedEOF } else if errors.Is(err, bufio.ErrBufferFull) { err = ErrLineTooLong diff --git a/cmd/s3-gw/service.go b/cmd/s3-gw/service.go index c3f91a04..0154492d 100644 --- a/cmd/s3-gw/service.go +++ b/cmd/s3-gw/service.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "net/http" "go.uber.org/zap" @@ -20,7 +21,7 @@ func (ms *Service) Start() { if ms.enabled { ms.log.Info("service is running", zap.String("endpoint", ms.Addr)) err := ms.ListenAndServe() - if err != nil && err != http.ErrServerClosed { + if err != nil && !errors.Is(err, http.ErrServerClosed) { ms.log.Warn("service couldn't start on configured port") } } else { diff --git a/internal/neofs/tree.go b/internal/neofs/tree.go index 4844734b..1310c321 100644 --- a/internal/neofs/tree.go +++ b/internal/neofs/tree.go @@ -1309,7 +1309,7 @@ func (c *TreeClient) getSubTree(ctx context.Context, bktInfo *data.BucketInfo, t var subtree []*tree.GetSubTreeResponse_Body for { resp, err := cli.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } else if err != nil { return nil, handleError("failed to get sub tree", err) From 9a1bc54bacaaff18400f9e007a92681023bc0c1a Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 15:42:50 +0300 Subject: [PATCH 05/11] *: fix bodyclose warnings in tests api/handler/locking_test.go:650:32 bodyclose response body must be closed ... Signed-off-by: Roman Khimov --- api/auth/center_test.go | 3 ++- api/handler/acl_test.go | 4 +++- api/handler/copy_test.go | 4 +++- api/handler/encryption_test.go | 8 ++++++-- api/handler/get_test.go | 4 +++- api/handler/handlers_test.go | 12 +++++++++--- api/handler/head_test.go | 4 +++- api/handler/locking_test.go | 20 +++++++++++++++----- 8 files changed, 44 insertions(+), 15 deletions(-) diff --git a/api/auth/center_test.go b/api/auth/center_test.go index c6b745e5..3bb1f21c 100644 --- a/api/auth/center_test.go +++ b/api/auth/center_test.go @@ -426,7 +426,8 @@ func TestAwsEncodedWithRequest(t *testing.T) { req.Body = io.NopCloser(buff) req.Header.Set("content-length", strconv.Itoa(buff.Len())) - _, err = http.DefaultClient.Do(req) + res, err := http.DefaultClient.Do(req) + res.Body.Close() require.NoError(t, err) } diff --git a/api/handler/acl_test.go b/api/handler/acl_test.go index ef9a2f23..6a76ec0c 100644 --- a/api/handler/acl_test.go +++ b/api/handler/acl_test.go @@ -1411,7 +1411,9 @@ func getBucketPolicy(hc *handlerContext, bktName string) *bucketPolicy { assertStatus(hc.t, w, http.StatusOK) policy := &bucketPolicy{} - err := json.NewDecoder(w.Result().Body).Decode(policy) + body := w.Result().Body + err := json.NewDecoder(body).Decode(policy) + body.Close() require.NoError(hc.t, err) return policy } diff --git a/api/handler/copy_test.go b/api/handler/copy_test.go index 1d5cba71..06cd7ae2 100644 --- a/api/handler/copy_test.go +++ b/api/handler/copy_test.go @@ -109,7 +109,9 @@ func getObjectTagging(t *testing.T, tc *handlerContext, bktName, objName, versio assertStatus(t, w, http.StatusOK) tagging := &Tagging{} - err := xml.NewDecoder(w.Result().Body).Decode(tagging) + body := w.Result().Body + err := xml.NewDecoder(body).Decode(tagging) + body.Close() require.NoError(t, err) return tagging } diff --git a/api/handler/encryption_test.go b/api/handler/encryption_test.go index df1663d1..363e13b5 100644 --- a/api/handler/encryption_test.go +++ b/api/handler/encryption_test.go @@ -269,7 +269,9 @@ func getEncryptedObject(t *testing.T, tc *handlerContext, bktName, objName strin setEncryptHeaders(r) tc.Handler().GetObjectHandler(w, r) assertStatus(t, w, http.StatusOK) - content, err := io.ReadAll(w.Result().Body) + body := w.Result().Body + content, err := io.ReadAll(body) + body.Close() require.NoError(t, err) return content, w.Header() } @@ -280,7 +282,9 @@ func getEncryptedObjectRange(t *testing.T, tc *handlerContext, bktName, objName r.Header.Set("Range", fmt.Sprintf("bytes=%d-%d", start, end)) tc.Handler().GetObjectHandler(w, r) assertStatus(t, w, http.StatusPartialContent) - content, err := io.ReadAll(w.Result().Body) + body := w.Result().Body + content, err := io.ReadAll(body) + body.Close() require.NoError(t, err) return content } diff --git a/api/handler/get_test.go b/api/handler/get_test.go index f2a39710..a40d3ed9 100644 --- a/api/handler/get_test.go +++ b/api/handler/get_test.go @@ -182,7 +182,9 @@ func getObjectRange(t *testing.T, tc *handlerContext, bktName, objName string, s r.Header.Set("Range", fmt.Sprintf("bytes=%d-%d", start, end)) tc.Handler().GetObjectHandler(w, r) assertStatus(t, w, http.StatusPartialContent) - content, err := io.ReadAll(w.Result().Body) + body := w.Result().Body + content, err := io.ReadAll(body) + body.Close() require.NoError(t, err) return content } diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index 31b52998..115a31f2 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -213,7 +213,9 @@ func prepareTestPayloadRequest(hc *handlerContext, bktName, objName string, payl func parseTestResponse(t *testing.T, response *httptest.ResponseRecorder, body any) { assertStatus(t, response, http.StatusOK) - err := xml.NewDecoder(response.Result().Body).Decode(body) + b := response.Result().Body + err := xml.NewDecoder(b).Decode(body) + b.Close() require.NoError(t, err) } @@ -236,7 +238,9 @@ func listOIDsFromMockedNeoFS(t *testing.T, tc *handlerContext, bktName string) [ func assertStatus(t *testing.T, w *httptest.ResponseRecorder, status int) { if w.Code != status { - resp, err := io.ReadAll(w.Result().Body) + body := w.Result().Body + resp, err := io.ReadAll(body) + body.Close() require.NoError(t, err) require.Failf(t, "unexpected status", "expected: %d, actual: %d, resp: '%s'", status, w.Code, string(resp)) } @@ -244,6 +248,8 @@ func assertStatus(t *testing.T, w *httptest.ResponseRecorder, status int) { func readResponse(t *testing.T, w *httptest.ResponseRecorder, status int, model any) { assertStatus(t, w, status) - err := xml.NewDecoder(w.Result().Body).Decode(model) + body := w.Result().Body + err := xml.NewDecoder(body).Decode(model) + body.Close() require.NoError(t, err) } diff --git a/api/handler/head_test.go b/api/handler/head_test.go index e12b1eea..e630869c 100644 --- a/api/handler/head_test.go +++ b/api/handler/head_test.go @@ -24,7 +24,9 @@ func TestConditionalHead(t *testing.T) { w, r := prepareTestRequest(tc, bktName, objName, nil) tc.Handler().HeadObjectHandler(w, r) assertStatus(t, w, http.StatusOK) - etag := w.Result().Header.Get(api.ETag) + res := w.Result() + etag := res.Header.Get(api.ETag) + res.Body.Close() headers := map[string]string{api.IfMatch: etag} headObject(t, tc, bktName, objName, headers, http.StatusOK) diff --git a/api/handler/locking_test.go b/api/handler/locking_test.go index 89fd3ee1..0f28ea37 100644 --- a/api/handler/locking_test.go +++ b/api/handler/locking_test.go @@ -415,7 +415,9 @@ func TestGetBucketLockConfigurationHandler(t *testing.T) { } actualConf := &data.ObjectLockConfiguration{} - err := xml.NewDecoder(w.Result().Body).Decode(actualConf) + body := w.Result().Body + err := xml.NewDecoder(body).Decode(actualConf) + body.Close() require.NoError(t, err) require.Equal(t, tc.expectedConf.ObjectLockEnabled, actualConf.ObjectLockEnabled) @@ -426,7 +428,9 @@ func TestGetBucketLockConfigurationHandler(t *testing.T) { func assertS3Error(t *testing.T, w *httptest.ResponseRecorder, expectedError s3errors.Error) { actualErrorResponse := &api.ErrorResponse{} - err := xml.NewDecoder(w.Result().Body).Decode(actualErrorResponse) + body := w.Result().Body + err := xml.NewDecoder(body).Decode(actualErrorResponse) + body.Close() require.NoError(t, err) require.Equal(t, expectedError.HTTPStatusCode, w.Code) @@ -476,7 +480,9 @@ func putObjectLegalHold(hc *handlerContext, bktName, objName, status string) { func assertLegalHold(t *testing.T, w *httptest.ResponseRecorder, status string) { actualHold := &data.LegalHold{} - err := xml.NewDecoder(w.Result().Body).Decode(actualHold) + body := w.Result().Body + err := xml.NewDecoder(body).Decode(actualHold) + body.Close() require.NoError(t, err) require.Equal(t, status, actualHold.Status) require.Equal(t, http.StatusOK, w.Code) @@ -532,7 +538,9 @@ func putObjectRetention(hc *handlerContext, bktName, objName string, retention * func assertRetention(t *testing.T, w *httptest.ResponseRecorder, retention *data.Retention) { actualRetention := &data.Retention{} - err := xml.NewDecoder(w.Result().Body).Decode(actualRetention) + body := w.Result().Body + err := xml.NewDecoder(body).Decode(actualRetention) + body.Close() require.NoError(t, err) require.Equal(t, retention.Mode, actualRetention.Mode) require.Equal(t, retention.RetainUntilDate, actualRetention.RetainUntilDate) @@ -639,7 +647,9 @@ func putObjectRetentionFailed(t *testing.T, hc *handlerContext, bktName, objName func assertRetentionApproximate(t *testing.T, w *httptest.ResponseRecorder, retention *data.Retention, delta float64) { actualRetention := &data.Retention{} - err := xml.NewDecoder(w.Result().Body).Decode(actualRetention) + body := w.Result().Body + err := xml.NewDecoder(body).Decode(actualRetention) + body.Close() require.NoError(t, err) require.Equal(t, retention.Mode, actualRetention.Mode) require.Equal(t, http.StatusOK, w.Code) From 9cad5ba0c7ffcfcfbf2fd9c41642fd8f34c30e87 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 15:57:57 +0300 Subject: [PATCH 06/11] api: fix contextcheck errors related to GetContextID Interface-based function is just misleading here: api/router.go:185:42 contextcheck Function `GetRequestID` should pass the context parameter api/layer/container.go:41:25 contextcheck Function `GetRequestID` should pass the context parameter api/layer/container.go:100:25 contextcheck Function `GetRequestID` should pass the context parameter api/handler/locking_test.go:335:64 contextcheck Function `NewReqInfo->GetRequestID` should pass the context parameter Signed-off-by: Roman Khimov --- api/layer/container.go | 4 ++-- api/reqinfo.go | 2 +- api/router.go | 20 +++++++++----------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/api/layer/container.go b/api/layer/container.go index 9b4903e5..f70ac043 100644 --- a/api/layer/container.go +++ b/api/layer/container.go @@ -38,7 +38,7 @@ func (n *layer) containerInfo(ctx context.Context, idCnr cid.ID) (*data.BucketIn var ( err error res *container.Container - rid = api.GetRequestID(ctx) + rid = api.GetContextRequestID(ctx) log = n.log.With(zap.Stringer("cid", idCnr), zap.String("request_id", rid)) info = &data.BucketInfo{ @@ -97,7 +97,7 @@ func (n *layer) containerList(ctx context.Context) ([]*data.BucketInfo, error) { err error own = n.Owner(ctx) res []cid.ID - rid = api.GetRequestID(ctx) + rid = api.GetContextRequestID(ctx) ) res, err = n.neoFS.UserContainers(ctx, own) if err != nil { diff --git a/api/reqinfo.go b/api/reqinfo.go index 3f96b925..15ab1aaa 100644 --- a/api/reqinfo.go +++ b/api/reqinfo.go @@ -136,7 +136,7 @@ func NewReqInfo(w http.ResponseWriter, r *http.Request, req ObjectRequest) *ReqI ObjectName: req.Object, UserAgent: r.UserAgent(), RemoteHost: GetSourceIP(r), - RequestID: GetRequestID(w), + RequestID: GetWriterRequestID(w), DeploymentID: deploymentID.String(), URL: r.URL, } diff --git a/api/router.go b/api/router.go index 1659f2a0..60ebbba9 100644 --- a/api/router.go +++ b/api/router.go @@ -182,7 +182,7 @@ func logErrorResponse(l *zap.Logger) mux.MiddlewareFunc { l.Info("call method", zap.Int("status", lw.statusCode), zap.String("host", r.Host), - zap.String("request_id", GetRequestID(r.Context())), + zap.String("request_id", reqInfo.RequestID), zap.String("method", mux.CurrentRoute(r).GetName()), zap.String("bucket", reqInfo.BucketName), zap.String("object", reqInfo.ObjectName), @@ -191,16 +191,14 @@ func logErrorResponse(l *zap.Logger) mux.MiddlewareFunc { } } -// GetRequestID returns the request ID from the response writer or the context. -func GetRequestID(v any) string { - switch t := v.(type) { - case context.Context: - return GetReqInfo(t).RequestID - case http.ResponseWriter: - return t.Header().Get(hdrAmzRequestID) - default: - panic("unknown type") - } +// GetContextRequestID returns the request ID from the context. +func GetContextRequestID(ctx context.Context) string { + return GetReqInfo(ctx).RequestID +} + +// GetWriterRequestID extracts request ID from the response writer. +func GetWriterRequestID(w http.ResponseWriter) string { + return w.Header().Get(hdrAmzRequestID) } // Attach adds S3 API handlers from h to r for domains with m client limit using From cf1bd793ec26f89a20647cdadb5889dc53bf9b72 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 16:01:41 +0300 Subject: [PATCH 07/11] auth: properly pass context into the new request api/auth/center.go:208:31 contextcheck Function `cloneRequest` should pass the context parameter Signed-off-by: Roman Khimov --- api/auth/center.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/auth/center.go b/api/auth/center.go index e7e25fd2..406898ae 100644 --- a/api/auth/center.go +++ b/api/auth/center.go @@ -1,7 +1,6 @@ package auth import ( - "context" "crypto/hmac" "crypto/sha256" "encoding/hex" @@ -289,7 +288,7 @@ func (c *center) checkFormData(r *http.Request) (*Box, error) { } func cloneRequest(r *http.Request, authHeader *authHeader) *http.Request { - otherRequest := r.Clone(context.TODO()) + otherRequest := r.Clone(r.Context()) otherRequest.Header = make(http.Header) for key, val := range r.Header { From f2c4a6fd6afa7877435d0acbaeff9550b3ea521e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 16:04:58 +0300 Subject: [PATCH 08/11] s3-gw: fix shutdown context management cmd/s3-gw/app.go:478:16 contextcheck Function `stopServices` should pass the context parameter cmd/s3-gw/app.go:507:16 contextcheck Function `stopServices` should pass the context parameter Signed-off-by: Roman Khimov --- cmd/s3-gw/app.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index 8072deac..4d952f66 100644 --- a/cmd/s3-gw/app.go +++ b/cmd/s3-gw/app.go @@ -469,19 +469,19 @@ LOOP: } } - ctx, cancel := shutdownContext() + ctx, cancel := shutdownContext(ctx) defer cancel() a.log.Info("stopping server", zap.Error(srv.Shutdown(ctx))) a.metrics.Shutdown() - a.stopServices() + a.stopServices(ctx) close(a.webDone) } -func shutdownContext() (context.Context, context.CancelFunc) { - return context.WithTimeout(context.Background(), defaultShutdownTimeout) +func shutdownContext(ctx context.Context) (context.Context, context.CancelFunc) { + return context.WithTimeout(ctx, defaultShutdownTimeout) } func (a *App) configReload(ctx context.Context) { @@ -504,7 +504,9 @@ func (a *App) configReload(ctx context.Context) { a.log.Warn("failed to reload server parameters", zap.Error(err)) } - a.stopServices() + ctx, cancel := shutdownContext(ctx) + defer cancel() + a.stopServices(ctx) a.startServices() a.updateSettings() @@ -573,10 +575,7 @@ func (a *App) updateServers() error { return nil } -func (a *App) stopServices() { - ctx, cancel := shutdownContext() - defer cancel() - +func (a *App) stopServices(ctx context.Context) { for _, svc := range a.services { svc.ShutDown(ctx) } From 5c33b2aedee9e43bb07a61fee66f3dd28e14efd6 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 16:06:40 +0300 Subject: [PATCH 09/11] neofs: mute contextcheck warnings internal/neofs/neofs_test.go:225:33 contextcheck Non-inherited new context, use function like `context.WithXXX` instead internal/neofs/neofs_test.go:232:28 contextcheck Non-inherited new context, use function like `context.WithXXX` instead Signed-off-by: Roman Khimov --- internal/neofs/neofs_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/neofs/neofs_test.go b/internal/neofs/neofs_test.go index 387dbfe9..c1976a96 100644 --- a/internal/neofs/neofs_test.go +++ b/internal/neofs/neofs_test.go @@ -222,14 +222,14 @@ func uploadDownload(ctx context.Context, t *testing.T, neo *NeoFS, p *pool.Pool, createParams.Payload = bytes.NewReader(payload) createParams.CreationTime = time.Now() - objID, err := neo.CreateObject(opContext, createParams) + objID, err := neo.CreateObject(opContext, createParams) //nolint:contextcheck // It's intentional, see above require.NoError(t, err) var objReadPrm layer.PrmObjectRead objReadPrm.Object = objID objReadPrm.Container = id - op, err := neo.ReadObject(opContext, objReadPrm) + op, err := neo.ReadObject(opContext, objReadPrm) //nolint:contextcheck // It's intentional, see above require.NoError(t, err) pl, err := io.ReadAll(op.Payload) From 6cdd12b942273a722c6c49dc5d2c064b31d055c8 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 16:09:21 +0300 Subject: [PATCH 10/11] api: mute bogus contextcheck warning api/router.go:137:20 contextcheck Non-inherited new context, use function like `context.WithXXX` or `r.Context` instead Signed-off-by: Roman Khimov --- api/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/router.go b/api/router.go index 60ebbba9..75174a9c 100644 --- a/api/router.go +++ b/api/router.go @@ -134,7 +134,7 @@ func setRequestID(h http.Handler) http.Handler { )) // set request info into context - r = r.WithContext(prepareContext(w, r)) + r = r.WithContext(prepareContext(w, r)) //nolint:contextcheck // In fact, r.Context() is reused internally // continue execution h.ServeHTTP(w, r) From 367954b7609d5c12850eb12af13e2bb2373d502e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 13 Sep 2024 16:11:25 +0300 Subject: [PATCH 11/11] handler: mute contextcheck api/handler/locking_test.go:335:21 contextcheck Non-inherited new context, use function like `context.WithXXX` instead Signed-off-by: Roman Khimov --- api/handler/locking_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/handler/locking_test.go b/api/handler/locking_test.go index 0f28ea37..5dfcf630 100644 --- a/api/handler/locking_test.go +++ b/api/handler/locking_test.go @@ -332,7 +332,7 @@ func TestPutBucketLockConfigurationHandler(t *testing.T) { w := httptest.NewRecorder() r := httptest.NewRequest(http.MethodPut, defaultURL, bytes.NewReader(body)) - r = r.WithContext(api.SetReqInfo(r.Context(), api.NewReqInfo(w, r, api.ObjectRequest{Bucket: tc.bucket}))) + r = r.WithContext(api.SetReqInfo(r.Context(), api.NewReqInfo(w, r, api.ObjectRequest{Bucket: tc.bucket}))) //nolint:contextcheck hc.Handler().PutBucketObjectLockConfigHandler(w, r)