Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Org-wide linter fixes #1004

Merged
merged 11 commits into from
Sep 17, 2024
3 changes: 1 addition & 2 deletions api/auth/center.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package auth

import (
"context"
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion api/auth/center_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions api/auth/signer/v4/chunked_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion api/handler/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion api/handler/copy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
20 changes: 14 additions & 6 deletions api/handler/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package handler

import (
"encoding/xml"
"errors"
"net/http"
"strconv"
"strings"
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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{
Expand Down
8 changes: 6 additions & 2 deletions api/handler/encryption_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion api/handler/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
12 changes: 9 additions & 3 deletions api/handler/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -236,14 +238,18 @@ 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))
}
}

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)
}
4 changes: 3 additions & 1 deletion api/handler/head_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 16 additions & 6 deletions api/handler/locking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions api/handler/put.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions api/layer/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion api/reqinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
27 changes: 17 additions & 10 deletions api/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package api
import (
"bytes"
"encoding/xml"
"errors"
"fmt"
"net/http"
"strconv"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
22 changes: 10 additions & 12 deletions api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand Down
Loading
Loading