Skip to content

Commit

Permalink
Org-wide linter fixes (#1004)
Browse files Browse the repository at this point in the history
  • Loading branch information
roman-khimov authored Sep 17, 2024
2 parents 19810b2 + 367954b commit e528a00
Show file tree
Hide file tree
Showing 22 changed files with 126 additions and 83 deletions.
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

0 comments on commit e528a00

Please sign in to comment.