From e3141fc8e3fce6afcc330fca5f6826c26e89e6bc Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Tue, 26 Nov 2024 13:32:00 +0300 Subject: [PATCH] [#563] Ignore precondition headers with invalid date format Signed-off-by: Marina Biryukova --- api/handler/attributes.go | 8 ++++---- api/handler/get.go | 24 ++++++++++++------------ api/handler/head.go | 6 +----- api/handler/head_test.go | 10 ++++++++++ api/handler/patch.go | 18 ++++++++---------- api/handler/patch_test.go | 7 +++++++ internal/logs/logs.go | 1 + 7 files changed, 43 insertions(+), 31 deletions(-) diff --git a/api/handler/attributes.go b/api/handler/attributes.go index da4f8d4..22ceffb 100644 --- a/api/handler/attributes.go +++ b/api/handler/attributes.go @@ -73,7 +73,7 @@ func (h *handler) GetObjectAttributesHandler(w http.ResponseWriter, r *http.Requ ctx := r.Context() reqInfo := middleware.GetReqInfo(ctx) - params, err := parseGetObjectAttributeArgs(r) + params, err := parseGetObjectAttributeArgs(r, h.reqLogger(ctx)) if err != nil { h.logAndSendError(ctx, w, "invalid request", reqInfo, err) return @@ -145,7 +145,7 @@ func writeAttributesHeaders(h http.Header, info *data.ExtendedObjectInfo, isBuck // x-amz-request-charged } -func parseGetObjectAttributeArgs(r *http.Request) (*GetObjectAttributesArgs, error) { +func parseGetObjectAttributeArgs(r *http.Request, log *zap.Logger) (*GetObjectAttributesArgs, error) { res := &GetObjectAttributesArgs{ VersionID: r.URL.Query().Get(api.QueryVersionID), } @@ -178,8 +178,8 @@ func parseGetObjectAttributeArgs(r *http.Request) (*GetObjectAttributesArgs, err } } - res.Conditional, err = parseConditionalHeaders(r.Header) - return res, err + res.Conditional = parseConditionalHeaders(r.Header, log) + return res, nil } func encodeToObjectAttributesResponse(info *data.ObjectInfo, p *GetObjectAttributesArgs, md5Enabled bool) (*GetObjectAttributesResponse, error) { diff --git a/api/handler/get.go b/api/handler/get.go index c524dfc..11b8770 100644 --- a/api/handler/get.go +++ b/api/handler/get.go @@ -13,6 +13,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" "go.uber.org/zap" ) @@ -154,11 +155,7 @@ func (h *handler) GetObjectHandler(w http.ResponseWriter, r *http.Request) { reqInfo = middleware.GetReqInfo(ctx) ) - conditional, err := parseConditionalHeaders(r.Header) - if err != nil { - h.logAndSendError(ctx, w, "could not parse request params", reqInfo, err) - return - } + conditional := parseConditionalHeaders(r.Header, h.reqLogger(ctx)) bktInfo, err := h.getBucketAndCheckOwner(r, reqInfo.BucketName) if err != nil { @@ -290,21 +287,24 @@ func checkPreconditions(info *data.ObjectInfo, args *conditionalArgs, md5Enabled return nil } -func parseConditionalHeaders(headers http.Header) (*conditionalArgs, error) { - var err error +func parseConditionalHeaders(headers http.Header, log *zap.Logger) *conditionalArgs { args := &conditionalArgs{ IfMatch: data.UnQuote(headers.Get(api.IfMatch)), IfNoneMatch: data.UnQuote(headers.Get(api.IfNoneMatch)), } - if args.IfModifiedSince, err = parseHTTPTime(headers.Get(api.IfModifiedSince)); err != nil { - return nil, err + if httpTime, err := parseHTTPTime(headers.Get(api.IfModifiedSince)); err == nil { + args.IfModifiedSince = httpTime + } else { + log.Warn(logs.FailedToParseHTTPTime, zap.String(api.IfModifiedSince, headers.Get(api.IfModifiedSince)), zap.Error(err)) } - if args.IfUnmodifiedSince, err = parseHTTPTime(headers.Get(api.IfUnmodifiedSince)); err != nil { - return nil, err + if httpTime, err := parseHTTPTime(headers.Get(api.IfUnmodifiedSince)); err == nil { + args.IfUnmodifiedSince = httpTime + } else { + log.Warn(logs.FailedToParseHTTPTime, zap.String(api.IfUnmodifiedSince, headers.Get(api.IfUnmodifiedSince)), zap.Error(err)) } - return args, nil + return args } func parseHTTPTime(data string) (*time.Time, error) { diff --git a/api/handler/head.go b/api/handler/head.go index ba183d3..314adfe 100644 --- a/api/handler/head.go +++ b/api/handler/head.go @@ -36,11 +36,7 @@ func (h *handler) HeadObjectHandler(w http.ResponseWriter, r *http.Request) { return } - conditional, err := parseConditionalHeaders(r.Header) - if err != nil { - h.logAndSendError(ctx, w, "could not parse request params", reqInfo, err) - return - } + conditional := parseConditionalHeaders(r.Header, h.reqLogger(ctx)) p := &layer.HeadObjectParams{ BktInfo: bktInfo, diff --git a/api/handler/head_test.go b/api/handler/head_test.go index 4eebc8c..b6aec29 100644 --- a/api/handler/head_test.go +++ b/api/handler/head_test.go @@ -68,6 +68,16 @@ func TestConditionalHead(t *testing.T) { api.IfModifiedSince: zeroTime.UTC().Format(http.TimeFormat), } headObject(t, tc, bktName, objName, headers, http.StatusNotModified) + + headers = map[string]string{ + api.IfUnmodifiedSince: zeroTime.UTC().Format(time.RFC3339), // invalid format, header is ignored + } + headObject(t, tc, bktName, objName, headers, http.StatusOK) + + headers = map[string]string{ + api.IfModifiedSince: objInfo.Created.Add(time.Minute).Format(time.RFC3339), // invalid format, header is ignored + } + headObject(t, tc, bktName, objName, headers, http.StatusOK) } func headObject(t *testing.T, tc *handlerContext, bktName, objName string, headers map[string]string, status int) { diff --git a/api/handler/patch.go b/api/handler/patch.go index a9e79bc..60efe3d 100644 --- a/api/handler/patch.go +++ b/api/handler/patch.go @@ -12,6 +12,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" "go.uber.org/zap" ) @@ -31,11 +32,7 @@ func (h *handler) PatchObjectHandler(w http.ResponseWriter, r *http.Request) { return } - conditional, err := parsePatchConditionalHeaders(r.Header) - if err != nil { - h.logAndSendError(ctx, w, "could not parse conditional headers", reqInfo, err) - return - } + conditional := parsePatchConditionalHeaders(r.Header, h.reqLogger(ctx)) bktInfo, err := h.getBucketAndCheckOwner(r, reqInfo.BucketName) if err != nil { @@ -137,17 +134,18 @@ func (h *handler) PatchObjectHandler(w http.ResponseWriter, r *http.Request) { } } -func parsePatchConditionalHeaders(headers http.Header) (*conditionalArgs, error) { - var err error +func parsePatchConditionalHeaders(headers http.Header, log *zap.Logger) *conditionalArgs { args := &conditionalArgs{ IfMatch: data.UnQuote(headers.Get(api.IfMatch)), } - if args.IfUnmodifiedSince, err = parseHTTPTime(headers.Get(api.IfUnmodifiedSince)); err != nil { - return nil, err + if httpTime, err := parseHTTPTime(headers.Get(api.IfUnmodifiedSince)); err == nil { + args.IfUnmodifiedSince = httpTime + } else { + log.Warn(logs.FailedToParseHTTPTime, zap.String(api.IfUnmodifiedSince, headers.Get(api.IfUnmodifiedSince)), zap.Error(err)) } - return args, nil + return args } func parsePatchByteRange(rangeStr string, objSize uint64) (*layer.RangeParams, error) { diff --git a/api/handler/patch_test.go b/api/handler/patch_test.go index aa29423..33aa339 100644 --- a/api/handler/patch_test.go +++ b/api/handler/patch_test.go @@ -60,6 +60,13 @@ func TestPatch(t *testing.T) { api.IfMatch: etag, }, }, + { + name: "If-Unmodified-Since invalid format, header is ignored", + rng: "bytes 0-2/*", + headers: map[string]string{ + api.IfUnmodifiedSince: created.Add(-24 * time.Hour).Format(time.RFC3339), + }, + }, { name: "invalid range syntax", rng: "bytes 0-2", diff --git a/internal/logs/logs.go b/internal/logs/logs.go index 3da1255..f8047a2 100644 --- a/internal/logs/logs.go +++ b/internal/logs/logs.go @@ -177,4 +177,5 @@ const ( MultinetConfigWontBeUpdated = "multinet config won't be updated" MultinetDialSuccess = "multinet dial successful" MultinetDialFail = "multinet dial failed" + FailedToParseHTTPTime = "failed to parse http time, header is ignored" )