From 352d5345fca804e99f62d33313e8f1c65d815a38 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Wed, 14 Jul 2021 15:51:45 +0300 Subject: [PATCH] [#158] Handled s3 errors on conditional headers Signed-off-by: Denis Kirillov --- api/errors.go | 6 ++++++ api/handler/copy.go | 5 ++--- api/handler/get.go | 17 ++++++++--------- api/handler/get_test.go | 29 +++++++++++++++-------------- 4 files changed, 31 insertions(+), 26 deletions(-) diff --git a/api/errors.go b/api/errors.go index a619193..3f4b622 100644 --- a/api/errors.go +++ b/api/errors.go @@ -62,6 +62,7 @@ const ( ErrNoSuchVersion ErrNotImplemented ErrPreconditionFailed + ErrNotModified ErrRequestTimeTooSkewed ErrSignatureDoesNotMatch ErrMethodNotAllowed @@ -494,6 +495,11 @@ var errorCodes = errorCodeMap{ Description: "At least one of the pre-conditions you specified did not hold", HTTPStatusCode: http.StatusPreconditionFailed, }, + ErrNotModified: { + Code: "NotModified", + Description: "The resource was not changed.", + HTTPStatusCode: http.StatusNotModified, + }, ErrRequestTimeTooSkewed: { Code: "RequestTimeTooSkewed", Description: "The difference between the request time and the server's time is too large.", diff --git a/api/handler/copy.go b/api/handler/copy.go index cb47763..c9fea7e 100644 --- a/api/handler/copy.go +++ b/api/handler/copy.go @@ -73,9 +73,8 @@ func (h *handler) CopyObjectHandler(w http.ResponseWriter, r *http.Request) { return } - status := checkPreconditions(inf, args.Conditional) - if status != http.StatusOK { - w.WriteHeader(status) + if err = checkPreconditions(inf, args.Conditional); err != nil { + api.WriteErrorResponse(r.Context(), w, api.GetAPIError(api.ErrPreconditionFailed), r.URL) return } diff --git a/api/handler/get.go b/api/handler/get.go index eaef90a..41b4b96 100644 --- a/api/handler/get.go +++ b/api/handler/get.go @@ -96,9 +96,8 @@ func (h *handler) GetObjectHandler(w http.ResponseWriter, r *http.Request) { return } - status := checkPreconditions(inf, args.Conditional) - if status != http.StatusOK { - w.WriteHeader(status) + if err = checkPreconditions(inf, args.Conditional); err != nil { + api.WriteErrorResponse(r.Context(), w, err, r.URL) return } @@ -122,23 +121,23 @@ func (h *handler) GetObjectHandler(w http.ResponseWriter, r *http.Request) { } } -func checkPreconditions(inf *layer.ObjectInfo, args *conditionalArgs) int { +func checkPreconditions(inf *layer.ObjectInfo, args *conditionalArgs) error { if len(args.IfMatch) > 0 && args.IfMatch != inf.HashSum { - return http.StatusPreconditionFailed + return api.GetAPIError(api.ErrPreconditionFailed) } if len(args.IfNoneMatch) > 0 && args.IfNoneMatch == inf.HashSum { - return http.StatusNotModified + return api.GetAPIError(api.ErrNotModified) } if args.IfModifiedSince != nil && inf.Created.Before(*args.IfModifiedSince) { - return http.StatusNotModified + return api.GetAPIError(api.ErrNotModified) } if args.IfUnmodifiedSince != nil && inf.Created.After(*args.IfUnmodifiedSince) { if len(args.IfMatch) == 0 { - return http.StatusPreconditionFailed + return api.GetAPIError(api.ErrPreconditionFailed) } } - return http.StatusOK + return nil } func parseGetObjectArgs(headers http.Header) (*getObjectArgs, error) { diff --git a/api/handler/get_test.go b/api/handler/get_test.go index 9ebc51d..94a5d31 100644 --- a/api/handler/get_test.go +++ b/api/handler/get_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/nspcc-dev/neofs-s3-gw/api" "github.com/nspcc-dev/neofs-s3-gw/api/layer" "github.com/stretchr/testify/require" ) @@ -58,79 +59,79 @@ func TestPreconditions(t *testing.T) { name string info *layer.ObjectInfo args *conditionalArgs - expected int + expected error }{ { name: "no conditions", info: new(layer.ObjectInfo), args: new(conditionalArgs), - expected: http.StatusOK, + expected: nil, }, { name: "IfMatch true", info: newInfo(etag, today), args: &conditionalArgs{IfMatch: etag}, - expected: http.StatusOK, + expected: nil, }, { name: "IfMatch false", info: newInfo(etag, today), args: &conditionalArgs{IfMatch: etag2}, - expected: http.StatusPreconditionFailed}, + expected: api.GetAPIError(api.ErrPreconditionFailed)}, { name: "IfNoneMatch true", info: newInfo(etag, today), args: &conditionalArgs{IfNoneMatch: etag2}, - expected: http.StatusOK}, + expected: nil}, { name: "IfNoneMatch false", info: newInfo(etag, today), args: &conditionalArgs{IfNoneMatch: etag}, - expected: http.StatusNotModified}, + expected: api.GetAPIError(api.ErrNotModified)}, { name: "IfModifiedSince true", info: newInfo(etag, today), args: &conditionalArgs{IfModifiedSince: &yesterday}, - expected: http.StatusOK}, + expected: nil}, { name: "IfModifiedSince false", info: newInfo(etag, yesterday), args: &conditionalArgs{IfModifiedSince: &today}, - expected: http.StatusNotModified}, + expected: api.GetAPIError(api.ErrNotModified)}, { name: "IfUnmodifiedSince true", info: newInfo(etag, yesterday), args: &conditionalArgs{IfUnmodifiedSince: &today}, - expected: http.StatusOK}, + expected: nil}, { name: "IfUnmodifiedSince false", info: newInfo(etag, today), args: &conditionalArgs{IfUnmodifiedSince: &yesterday}, - expected: http.StatusPreconditionFailed}, + expected: api.GetAPIError(api.ErrPreconditionFailed)}, { name: "IfMatch true, IfUnmodifiedSince false", info: newInfo(etag, today), args: &conditionalArgs{IfMatch: etag, IfUnmodifiedSince: &yesterday}, - expected: http.StatusOK, + expected: nil, }, { name: "IfMatch false, IfUnmodifiedSince true", info: newInfo(etag, yesterday), args: &conditionalArgs{IfMatch: etag2, IfUnmodifiedSince: &today}, - expected: http.StatusPreconditionFailed, + expected: api.GetAPIError(api.ErrPreconditionFailed), }, { name: "IfNoneMatch false, IfModifiedSince true", info: newInfo(etag, today), args: &conditionalArgs{IfNoneMatch: etag, IfModifiedSince: &yesterday}, - expected: http.StatusNotModified, + expected: api.GetAPIError(api.ErrNotModified), }, { name: "IfNoneMatch true, IfModifiedSince false", info: newInfo(etag, yesterday), args: &conditionalArgs{IfNoneMatch: etag2, IfModifiedSince: &today}, - expected: http.StatusNotModified, + expected: api.GetAPIError(api.ErrNotModified), }, } { t.Run(tc.name, func(t *testing.T) {