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.
This commit is contained in:
Harshavardhana 2024-05-05 09:56:21 -07:00 committed by GitHub
parent 8ff70ea5a9
commit 523bd769f1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 61 additions and 29 deletions

View File

@ -56,19 +56,23 @@ type APIError struct {
Code string Code string
Description string Description string
HTTPStatusCode int HTTPStatusCode int
ObjectSize string
RangeRequested string
} }
// APIErrorResponse - error response format // APIErrorResponse - error response format
type APIErrorResponse struct { type APIErrorResponse struct {
XMLName xml.Name `xml:"Error" json:"-"` XMLName xml.Name `xml:"Error" json:"-"`
Code string Code string
Message string Message string
Key string `xml:"Key,omitempty" json:"Key,omitempty"` Key string `xml:"Key,omitempty" json:"Key,omitempty"`
BucketName string `xml:"BucketName,omitempty" json:"BucketName,omitempty"` BucketName string `xml:"BucketName,omitempty" json:"BucketName,omitempty"`
Resource string Resource string
Region string `xml:"Region,omitempty" json:"Region,omitempty"` Region string `xml:"Region,omitempty" json:"Region,omitempty"`
RequestID string `xml:"RequestId" json:"RequestId"` RequestID string `xml:"RequestId" json:"RequestId"`
HostID string `xml:"HostId" json:"HostId"` 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. // APIErrorCode type of error status.
@ -2412,10 +2416,9 @@ func toAPIError(ctx context.Context, err error) APIError {
apiErr := errorCodes.ToAPIErr(toAPIErrorCode(ctx, err)) apiErr := errorCodes.ToAPIErr(toAPIErrorCode(ctx, err))
switch apiErr.Code { switch apiErr.Code {
case "NotImplemented": case "NotImplemented":
desc := fmt.Sprintf("%s (%v)", apiErr.Description, err)
apiErr = APIError{ apiErr = APIError{
Code: apiErr.Code, Code: apiErr.Code,
Description: desc, Description: fmt.Sprintf("%s (%v)", apiErr.Description, err),
HTTPStatusCode: apiErr.HTTPStatusCode, HTTPStatusCode: apiErr.HTTPStatusCode,
} }
case "XMinioBackendDown": case "XMinioBackendDown":
@ -2432,7 +2435,19 @@ func toAPIError(ctx context.Context, err error) APIError {
HTTPStatusCode: e.HTTPStatusCode, HTTPStatusCode: e.HTTPStatusCode,
} }
case batchReplicationJobError: 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: case InvalidArgument:
apiErr = APIError{ apiErr = APIError{
Code: "InvalidArgument", Code: "InvalidArgument",
@ -2559,13 +2574,15 @@ func getAPIError(code APIErrorCode) APIError {
func getAPIErrorResponse(ctx context.Context, err APIError, resource, requestID, hostID string) APIErrorResponse { func getAPIErrorResponse(ctx context.Context, err APIError, resource, requestID, hostID string) APIErrorResponse {
reqInfo := logger.GetReqInfo(ctx) reqInfo := logger.GetReqInfo(ctx)
return APIErrorResponse{ return APIErrorResponse{
Code: err.Code, Code: err.Code,
Message: err.Description, Message: err.Description,
BucketName: reqInfo.BucketName, BucketName: reqInfo.BucketName,
Key: reqInfo.ObjectName, Key: reqInfo.ObjectName,
Resource: resource, Resource: resource,
Region: globalSite.Region, Region: globalSite.Region,
RequestID: requestID, RequestID: requestID,
HostID: hostID, HostID: hostID,
ActualObjectSize: err.ObjectSize,
RangeRequested: err.RangeRequested,
} }
} }

View File

@ -1231,6 +1231,7 @@ type batchReplicationJobError struct {
Code string Code string
Description string Description string
HTTPStatusCode int HTTPStatusCode int
ObjectSize int64
} }
func (e batchReplicationJobError) Error() string { func (e batchReplicationJobError) Error() string {

View File

@ -251,6 +251,18 @@ func (er erasureObjects) GetObjectNInfo(ctx context.Context, bucket, object stri
opts.NoDecryption = true 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() { if objInfo.IsRemote() {
gr, err := getTransitionedObjectReader(ctx, bucket, object, rs, h, objInfo, opts) gr, err := getTransitionedObjectReader(ctx, bucket, object, rs, h, objInfo, opts)
if err != nil { if err != nil {
@ -260,11 +272,6 @@ func (er erasureObjects) GetObjectNInfo(ctx context.Context, bucket, object stri
return gr.WithCleanupFuncs(nsUnlocker), nil 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) fn, off, length, err := NewGetObjectReader(rs, objInfo, opts)
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -60,7 +60,11 @@ func (h *HTTPRangeSpec) GetLength(resourceSize int64) (rangeLength int64, err er
} }
case h.Start >= resourceSize: case h.Start >= resourceSize:
return 0, errInvalidRange return 0, InvalidRange{
OffsetBegin: h.Start,
OffsetEnd: h.End,
ResourceSize: resourceSize,
}
case h.End > -1: case h.End > -1:
end := h.End end := h.End

View File

@ -72,7 +72,7 @@ func TestHTTPRequestRangeSpec(t *testing.T) {
if err == nil { if err == nil {
t.Errorf("Case %d: Did not get an expected error - got %v", i, rs) 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) t.Errorf("Case %d: Got invalid range error instead of a parse error", i)
} }
if rs != nil { if rs != nil {
@ -95,7 +95,7 @@ func TestHTTPRequestRangeSpec(t *testing.T) {
if err1 == nil { if err1 == nil {
o, l, err2 = rs.GetOffsetLength(resourceSize) o, l, err2 = rs.GetOffsetLength(resourceSize)
} }
if err1 == errInvalidRange || (err1 == nil && err2 == errInvalidRange) { if isErrInvalidRange(err1) || (err1 == nil && isErrInvalidRange(err2)) {
continue continue
} }
t.Errorf("Case %d: Expected errInvalidRange but: %v %v %d %d %v", i, rs, err1, o, l, err2) t.Errorf("Case %d: Expected errInvalidRange but: %v %v %d %d %v", i, rs, err1, o, l, err2)

View File

@ -595,7 +595,7 @@ type InvalidRange struct {
} }
func (e InvalidRange) Error() string { 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. // 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 { func isErrInvalidRange(err error) bool {
if errors.Is(err, errInvalidRange) {
return true
}
_, ok := err.(InvalidRange) _, ok := err.(InvalidRange)
return ok return ok
} }