From 523bd769f1ce39dfdff892d90d85d1ed3221c40d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 5 May 2024 09:56:21 -0700 Subject: [PATCH] add support for specific error response for InvalidRange (#19668) fixes #19648 AWS S3 returns the actual object size as part of XML response for InvalidRange error, this is used apparently by SDKs to retry the request without the range. --- cmd/api-errors.go | 57 ++++++++++++++++++++++++++-------------- cmd/batch-handlers.go | 1 + cmd/erasure-object.go | 17 ++++++++---- cmd/httprange.go | 6 ++++- cmd/httprange_test.go | 4 +-- cmd/object-api-errors.go | 5 +++- 6 files changed, 61 insertions(+), 29 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index c4aeb9a6a..a35df607f 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -56,19 +56,23 @@ type APIError struct { Code string Description string HTTPStatusCode int + ObjectSize string + RangeRequested string } // APIErrorResponse - error response format type APIErrorResponse struct { - XMLName xml.Name `xml:"Error" json:"-"` - Code string - Message string - Key string `xml:"Key,omitempty" json:"Key,omitempty"` - BucketName string `xml:"BucketName,omitempty" json:"BucketName,omitempty"` - Resource string - Region string `xml:"Region,omitempty" json:"Region,omitempty"` - RequestID string `xml:"RequestId" json:"RequestId"` - HostID string `xml:"HostId" json:"HostId"` + XMLName xml.Name `xml:"Error" json:"-"` + Code string + Message string + Key string `xml:"Key,omitempty" json:"Key,omitempty"` + BucketName string `xml:"BucketName,omitempty" json:"BucketName,omitempty"` + Resource string + Region string `xml:"Region,omitempty" json:"Region,omitempty"` + RequestID string `xml:"RequestId" json:"RequestId"` + HostID string `xml:"HostId" json:"HostId"` + ActualObjectSize string `xml:"ActualObjectSize,omitempty" json:"ActualObjectSize,omitempty"` + RangeRequested string `xml:"RangeRequested,omitempty" json:"RangeRequested,omitempty"` } // APIErrorCode type of error status. @@ -2412,10 +2416,9 @@ func toAPIError(ctx context.Context, err error) APIError { apiErr := errorCodes.ToAPIErr(toAPIErrorCode(ctx, err)) switch apiErr.Code { case "NotImplemented": - desc := fmt.Sprintf("%s (%v)", apiErr.Description, err) apiErr = APIError{ Code: apiErr.Code, - Description: desc, + Description: fmt.Sprintf("%s (%v)", apiErr.Description, err), HTTPStatusCode: apiErr.HTTPStatusCode, } case "XMinioBackendDown": @@ -2432,7 +2435,19 @@ func toAPIError(ctx context.Context, err error) APIError { HTTPStatusCode: e.HTTPStatusCode, } case batchReplicationJobError: - apiErr = APIError(e) + apiErr = APIError{ + Description: e.Description, + Code: e.Code, + HTTPStatusCode: e.HTTPStatusCode, + } + case InvalidRange: + apiErr = APIError{ + Code: "InvalidRange", + Description: e.Error(), + HTTPStatusCode: errorCodes[ErrInvalidRange].HTTPStatusCode, + ObjectSize: strconv.FormatInt(e.ResourceSize, 10), + RangeRequested: fmt.Sprintf("%d-%d", e.OffsetBegin, e.OffsetEnd), + } case InvalidArgument: apiErr = APIError{ Code: "InvalidArgument", @@ -2559,13 +2574,15 @@ func getAPIError(code APIErrorCode) APIError { func getAPIErrorResponse(ctx context.Context, err APIError, resource, requestID, hostID string) APIErrorResponse { reqInfo := logger.GetReqInfo(ctx) return APIErrorResponse{ - Code: err.Code, - Message: err.Description, - BucketName: reqInfo.BucketName, - Key: reqInfo.ObjectName, - Resource: resource, - Region: globalSite.Region, - RequestID: requestID, - HostID: hostID, + Code: err.Code, + Message: err.Description, + BucketName: reqInfo.BucketName, + Key: reqInfo.ObjectName, + Resource: resource, + Region: globalSite.Region, + RequestID: requestID, + HostID: hostID, + ActualObjectSize: err.ObjectSize, + RangeRequested: err.RangeRequested, } } diff --git a/cmd/batch-handlers.go b/cmd/batch-handlers.go index bd65fa9c7..12bf67ac6 100644 --- a/cmd/batch-handlers.go +++ b/cmd/batch-handlers.go @@ -1231,6 +1231,7 @@ type batchReplicationJobError struct { Code string Description string HTTPStatusCode int + ObjectSize int64 } func (e batchReplicationJobError) Error() string { diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index b819d5ad3..30181d6a4 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -251,6 +251,18 @@ func (er erasureObjects) GetObjectNInfo(ctx context.Context, bucket, object stri opts.NoDecryption = true } + if objInfo.Size == 0 { + if _, _, err := rs.GetOffsetLength(objInfo.Size); err != nil { + // Make sure to return object info to provide extra information. + return &GetObjectReader{ + ObjInfo: objInfo, + }, err + } + + // Zero byte objects don't even need to further initialize pipes etc. + return NewGetObjectReaderFromReader(bytes.NewReader(nil), objInfo, opts) + } + if objInfo.IsRemote() { gr, err := getTransitionedObjectReader(ctx, bucket, object, rs, h, objInfo, opts) if err != nil { @@ -260,11 +272,6 @@ func (er erasureObjects) GetObjectNInfo(ctx context.Context, bucket, object stri return gr.WithCleanupFuncs(nsUnlocker), nil } - if objInfo.Size == 0 { - // Zero byte objects don't even need to further initialize pipes etc. - return NewGetObjectReaderFromReader(bytes.NewReader(nil), objInfo, opts) - } - fn, off, length, err := NewGetObjectReader(rs, objInfo, opts) if err != nil { return nil, err diff --git a/cmd/httprange.go b/cmd/httprange.go index db2229905..2e198d474 100644 --- a/cmd/httprange.go +++ b/cmd/httprange.go @@ -60,7 +60,11 @@ func (h *HTTPRangeSpec) GetLength(resourceSize int64) (rangeLength int64, err er } case h.Start >= resourceSize: - return 0, errInvalidRange + return 0, InvalidRange{ + OffsetBegin: h.Start, + OffsetEnd: h.End, + ResourceSize: resourceSize, + } case h.End > -1: end := h.End diff --git a/cmd/httprange_test.go b/cmd/httprange_test.go index 2ce9c4e9f..ea13a3800 100644 --- a/cmd/httprange_test.go +++ b/cmd/httprange_test.go @@ -72,7 +72,7 @@ func TestHTTPRequestRangeSpec(t *testing.T) { if err == nil { t.Errorf("Case %d: Did not get an expected error - got %v", i, rs) } - if err == errInvalidRange { + if isErrInvalidRange(err) { t.Errorf("Case %d: Got invalid range error instead of a parse error", i) } if rs != nil { @@ -95,7 +95,7 @@ func TestHTTPRequestRangeSpec(t *testing.T) { if err1 == nil { o, l, err2 = rs.GetOffsetLength(resourceSize) } - if err1 == errInvalidRange || (err1 == nil && err2 == errInvalidRange) { + if isErrInvalidRange(err1) || (err1 == nil && isErrInvalidRange(err2)) { continue } t.Errorf("Case %d: Expected errInvalidRange but: %v %v %d %d %v", i, rs, err1, o, l, err2) diff --git a/cmd/object-api-errors.go b/cmd/object-api-errors.go index 10c34f415..e04cb2dd0 100644 --- a/cmd/object-api-errors.go +++ b/cmd/object-api-errors.go @@ -595,7 +595,7 @@ type InvalidRange struct { } func (e InvalidRange) Error() string { - return fmt.Sprintf("The requested range \"bytes %d -> %d of %d\" is not satisfiable.", e.OffsetBegin, e.OffsetEnd, e.ResourceSize) + return fmt.Sprintf("The requested range 'bytes=%d-%d' is not satisfiable", e.OffsetBegin, e.OffsetEnd) } // ObjectTooLarge error returned when the size of the object > max object size allowed (5G) per request. @@ -758,6 +758,9 @@ func isErrMethodNotAllowed(err error) bool { } func isErrInvalidRange(err error) bool { + if errors.Is(err, errInvalidRange) { + return true + } _, ok := err.(InvalidRange) return ok }