diff --git a/api/handler/get.go b/api/handler/get.go index 615f7f05..4c3bf38d 100644 --- a/api/handler/get.go +++ b/api/handler/get.go @@ -231,17 +231,24 @@ func (h *handler) GetObjectHandler(w http.ResponseWriter, r *http.Request) { func checkPreconditions(info *data.ObjectInfo, args *conditionalArgs) error { if len(args.IfMatch) > 0 && args.IfMatch != info.HashSum { - return errors.GetAPIError(errors.ErrPreconditionFailed) + return fmt.Errorf("%w: etag mismatched: '%s', '%s'", errors.GetAPIError(errors.ErrPreconditionFailed), args.IfMatch, info.HashSum) } if len(args.IfNoneMatch) > 0 && args.IfNoneMatch == info.HashSum { - return errors.GetAPIError(errors.ErrNotModified) + return fmt.Errorf("%w: etag matched: '%s', '%s'", errors.GetAPIError(errors.ErrNotModified), args.IfNoneMatch, info.HashSum) } if args.IfModifiedSince != nil && info.Created.Before(*args.IfModifiedSince) { - return errors.GetAPIError(errors.ErrNotModified) + return fmt.Errorf("%w: not modified since '%s', last modified '%s'", errors.GetAPIError(errors.ErrNotModified), + args.IfModifiedSince.Format(time.RFC3339), info.Created.Format(time.RFC3339)) } if args.IfUnmodifiedSince != nil && info.Created.After(*args.IfUnmodifiedSince) { + // https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html#API_GetObject_RequestSyntax + // If both of the If-Match and If-Unmodified-Since headers are present in the request as follows: + // If-Match condition evaluates to true, and; + // If-Unmodified-Since condition evaluates to false; + // then, S3 returns 200 OK and the data requested. if len(args.IfMatch) == 0 { - return errors.GetAPIError(errors.ErrPreconditionFailed) + return fmt.Errorf("%w: modified since '%s', last modified '%s'", errors.GetAPIError(errors.ErrPreconditionFailed), + args.IfUnmodifiedSince.Format(time.RFC3339), info.Created.Format(time.RFC3339)) } } @@ -251,8 +258,8 @@ func checkPreconditions(info *data.ObjectInfo, args *conditionalArgs) error { func parseConditionalHeaders(headers http.Header) (*conditionalArgs, error) { var err error args := &conditionalArgs{ - IfMatch: headers.Get(api.IfMatch), - IfNoneMatch: headers.Get(api.IfNoneMatch), + IfMatch: strings.Trim(headers.Get(api.IfMatch), "\""), + IfNoneMatch: strings.Trim(headers.Get(api.IfNoneMatch), "\""), } if args.IfModifiedSince, err = parseHTTPTime(headers.Get(api.IfModifiedSince)); err != nil { diff --git a/api/handler/get_test.go b/api/handler/get_test.go index 22c7784a..996dc2ee 100644 --- a/api/handler/get_test.go +++ b/api/handler/get_test.go @@ -2,6 +2,7 @@ package handler import ( "bytes" + stderrors "errors" "fmt" "io" "net/http" @@ -149,7 +150,11 @@ func TestPreconditions(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { actual := checkPreconditions(tc.info, tc.args) - require.Equal(t, tc.expected, actual) + if tc.expected == nil { + require.NoError(t, actual) + } else { + require.True(t, stderrors.Is(actual, tc.expected), tc.expected, actual) + } }) } } diff --git a/api/handler/head_test.go b/api/handler/head_test.go index d0c1225c..2936ceb4 100644 --- a/api/handler/head_test.go +++ b/api/handler/head_test.go @@ -27,10 +27,14 @@ func TestConditionalHead(t *testing.T) { tc.Handler().HeadObjectHandler(w, r) assertStatus(t, w, http.StatusOK) etag := w.Result().Header.Get(api.ETag) + etagQuoted := "\"" + etag + "\"" headers := map[string]string{api.IfMatch: etag} headObject(t, tc, bktName, objName, headers, http.StatusOK) + headers = map[string]string{api.IfMatch: etagQuoted} + headObject(t, tc, bktName, objName, headers, http.StatusOK) + headers = map[string]string{api.IfMatch: "etag"} headObject(t, tc, bktName, objName, headers, http.StatusPreconditionFailed) @@ -50,6 +54,9 @@ func TestConditionalHead(t *testing.T) { headers = map[string]string{api.IfNoneMatch: etag} headObject(t, tc, bktName, objName, headers, http.StatusNotModified) + headers = map[string]string{api.IfNoneMatch: etagQuoted} + headObject(t, tc, bktName, objName, headers, http.StatusNotModified) + headers = map[string]string{api.IfNoneMatch: "etag"} headObject(t, tc, bktName, objName, headers, http.StatusOK)