From 5aa2a822e8c2a84ae334122caa6165aefc9e04d7 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Sun, 28 Dec 2025 13:29:48 +0400 Subject: [PATCH] fix: Makes precondition headers insensitive to whether the value is quoted Fixes #1710 The `If-Match` and `If-None-Match` precondition header values represent object ETags. ETags are generally quoted; however, S3 evaluates precondition headers equivalently whether the ETag is quoted or not, comparing only the underlying value and ignoring the quotes if present. The new implementation trims quotes from the ETag in both the input precondition header and the object metadata, ensuring that comparisons are performed purely on the ETag value and are insensitive to quoting. --- backend/common.go | 3 ++ s3api/utils/precondition.go | 17 +++++++- tests/integration/CompleteMultipartUpload.go | 9 ++++ tests/integration/CopyObject.go | 42 +++++++++++++++++++ tests/integration/GetObject.go | 42 +++++++++++++++++++ tests/integration/HeadObject.go | 43 ++++++++++++++++++++ tests/integration/PutObject.go | 10 +++++ tests/integration/UploadPartCopy.go | 43 ++++++++++++++++++++ 8 files changed, 208 insertions(+), 1 deletion(-) diff --git a/backend/common.go b/backend/common.go index ccd1146c..c3c995c4 100644 --- a/backend/common.go +++ b/backend/common.go @@ -495,6 +495,8 @@ func EvaluatePreconditions(etag string, modTime time.Time, preconditions PreCond return nil } + etag = strings.Trim(etag, `"`) + // convert all conditions to *bool to evaluate the conditions var ifMatch, ifNoneMatch, ifModSince, ifUnmodeSince *bool if preconditions.IfMatch != nil { @@ -581,6 +583,7 @@ func EvaluatePreconditions(etag string, modTime time.Time, preconditions PreCond // EvaluateMatchPreconditions evaluates if-match and if-none-match preconditions func EvaluateMatchPreconditions(etag string, ifMatch, ifNoneMatch *string) error { + etag = strings.Trim(etag, `"`) if ifMatch != nil && *ifMatch != etag { return errPreconditionFailed } diff --git a/s3api/utils/precondition.go b/s3api/utils/precondition.go index 9f3e17f4..f8538686 100644 --- a/s3api/utils/precondition.go +++ b/s3api/utils/precondition.go @@ -67,7 +67,10 @@ func ParsePreconditionMatchHeaders(ctx *fiber.Ctx, opts ...preconditionOpt) (*st if cfg.withCopySource { prefix = "X-Amz-Copy-Source-" } - return GetStringPtr(ctx.Get(prefix + "If-Match")), GetStringPtr(ctx.Get(prefix + "If-None-Match")) + + ifMatch := trimQuotes(ctx.Get(prefix + "If-Match")) + ifNoneMatch := trimQuotes(ctx.Get(prefix + "If-None-Match")) + return GetStringPtr(ifMatch), GetStringPtr(ifNoneMatch) } // ParsePreconditionDateHeaders parses the "If-Modified-Since" and "If-Unmodified-Since" @@ -136,3 +139,15 @@ func ParseIfMatchSize(ctx *fiber.Ctx) *int64 { return &ifMatchSize } + +func trimQuotes(str string) string { + if len(str) < 2 { + return str + } + + if str[0] == str[len(str)-1] && str[0] == '"' { + return str[1 : len(str)-1] + } + + return str +} diff --git a/tests/integration/CompleteMultipartUpload.go b/tests/integration/CompleteMultipartUpload.go index 36e7e686..119e993d 100644 --- a/tests/integration/CompleteMultipartUpload.go +++ b/tests/integration/CompleteMultipartUpload.go @@ -1374,6 +1374,7 @@ func CompleteMultipartUpload_conditional_writes(s *S3Conf) error { obj := "my-obj" etag := getPtr("") + var etagTrimmed string incorrectEtag := getPtr("incorrect_etag") errPrecond := s3err.GetAPIError(s3err.ErrPreconditionFailed) @@ -1399,6 +1400,13 @@ func CompleteMultipartUpload_conditional_writes(s *S3Conf) error { {"obj-3", etag, incorrectEtag, nil}, {"obj-4", incorrectEtag, nil, nil}, {"obj-5", nil, etag, nil}, + + // precondtion headers without quotes + {obj, &etagTrimmed, nil, nil}, + {obj, &etagTrimmed, &etagTrimmed, errPrecond}, + {obj, &etagTrimmed, incorrectEtag, nil}, + {obj, incorrectEtag, &etagTrimmed, errPrecond}, + {obj, nil, &etagTrimmed, errPrecond}, } { res, err := putObjectWithData(0, &s3.PutObjectInput{ Bucket: &bucket, @@ -1412,6 +1420,7 @@ func CompleteMultipartUpload_conditional_writes(s *S3Conf) error { // the exact same data. // to avoid ETag collision reassign the etag value *etag = *res.res.ETag + etagTrimmed = strings.Trim(*etag, `"`) mp, err := createMp(s3client, bucket, test.obj) if err != nil { diff --git a/tests/integration/CopyObject.go b/tests/integration/CopyObject.go index 34a000cb..858f25cb 100644 --- a/tests/integration/CopyObject.go +++ b/tests/integration/CopyObject.go @@ -910,6 +910,7 @@ func CopyObject_conditional_reads(s *S3Conf) error { before := time.Now().AddDate(0, 0, -3) after := time.Now() etag := obj.res.ETag + etagTrimmed := strings.Trim(*etag, `"`) for i, test := range []struct { ifmatch *string @@ -1008,6 +1009,47 @@ func CopyObject_conditional_reads(s *S3Conf) error { {nil, nil, nil, &before, errCond}, {nil, nil, nil, &after, nil}, {nil, nil, nil, nil, nil}, + + // if-match, if-non-match without quotes + {&etagTrimmed, getPtr("invalid_etag"), &before, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, nil, nil}, + + {&etagTrimmed, &etagTrimmed, &before, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &before, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &before, nil, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &after, nil, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &before, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &after, errMod}, + {&etagTrimmed, &etagTrimmed, nil, nil, errMod}, + + {&etagTrimmed, nil, &before, &before, nil}, + {&etagTrimmed, nil, &before, &after, nil}, + {&etagTrimmed, nil, &before, nil, nil}, + {&etagTrimmed, nil, &after, &before, errMod}, + {&etagTrimmed, nil, &after, &after, errMod}, + {&etagTrimmed, nil, &after, nil, errMod}, + {&etagTrimmed, nil, nil, &before, nil}, + {&etagTrimmed, nil, nil, &after, nil}, + {&etagTrimmed, nil, nil, nil, nil}, + + {nil, &etagTrimmed, &before, &before, errCond}, + {nil, &etagTrimmed, &before, &after, errMod}, + {nil, &etagTrimmed, &before, nil, errMod}, + {nil, &etagTrimmed, &after, &before, errCond}, + {nil, &etagTrimmed, &after, &after, errMod}, + {nil, &etagTrimmed, &after, nil, errMod}, + {nil, &etagTrimmed, nil, &before, errCond}, + {nil, &etagTrimmed, nil, &after, errMod}, + {nil, &etagTrimmed, nil, nil, errMod}, } { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.CopyObject(ctx, &s3.CopyObjectInput{ diff --git a/tests/integration/GetObject.go b/tests/integration/GetObject.go index 97640ee0..67a0017e 100644 --- a/tests/integration/GetObject.go +++ b/tests/integration/GetObject.go @@ -379,6 +379,7 @@ func GetObject_conditional_reads(s *S3Conf) error { before := time.Now().AddDate(0, 0, -3) after := time.Now() etag := obj.res.ETag + etagTrimmed := strings.Trim(*etag, `"`) for i, test := range []struct { ifmatch *string @@ -477,6 +478,47 @@ func GetObject_conditional_reads(s *S3Conf) error { {nil, nil, nil, &before, errCond}, {nil, nil, nil, &after, nil}, {nil, nil, nil, nil, nil}, + + // if-match, if-non-match without quotes + {&etagTrimmed, getPtr("invalid_etag"), &before, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, nil, nil}, + + {&etagTrimmed, &etagTrimmed, &before, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &before, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &before, nil, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &after, nil, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &before, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &after, errMod}, + {&etagTrimmed, &etagTrimmed, nil, nil, errMod}, + + {&etagTrimmed, nil, &before, &before, nil}, + {&etagTrimmed, nil, &before, &after, nil}, + {&etagTrimmed, nil, &before, nil, nil}, + {&etagTrimmed, nil, &after, &before, errMod}, + {&etagTrimmed, nil, &after, &after, errMod}, + {&etagTrimmed, nil, &after, nil, errMod}, + {&etagTrimmed, nil, nil, &before, nil}, + {&etagTrimmed, nil, nil, &after, nil}, + {&etagTrimmed, nil, nil, nil, nil}, + + {nil, &etagTrimmed, &before, &before, errCond}, + {nil, &etagTrimmed, &before, &after, errMod}, + {nil, &etagTrimmed, &before, nil, errMod}, + {nil, &etagTrimmed, &after, &before, errCond}, + {nil, &etagTrimmed, &after, &after, errMod}, + {nil, &etagTrimmed, &after, nil, errMod}, + {nil, &etagTrimmed, nil, &before, errCond}, + {nil, &etagTrimmed, nil, &after, errMod}, + {nil, &etagTrimmed, nil, nil, errMod}, } { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.GetObject(ctx, &s3.GetObjectInput{ diff --git a/tests/integration/HeadObject.go b/tests/integration/HeadObject.go index 988c800e..c9928f78 100644 --- a/tests/integration/HeadObject.go +++ b/tests/integration/HeadObject.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -422,6 +423,7 @@ func HeadObject_conditional_reads(s *S3Conf) error { before := time.Now().AddDate(0, 0, -3) after := time.Now() etag := obj.res.ETag + etagTrimmed := strings.Trim(*etag, `"`) for i, test := range []struct { ifmatch *string @@ -520,6 +522,47 @@ func HeadObject_conditional_reads(s *S3Conf) error { {nil, nil, nil, &before, errCond}, {nil, nil, nil, &after, nil}, {nil, nil, nil, nil, nil}, + + // if-match, if-non-match without quotes + {&etagTrimmed, getPtr("invalid_etag"), &before, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, nil, nil}, + + {&etagTrimmed, &etagTrimmed, &before, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &before, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &before, nil, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &after, nil, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &before, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &after, errMod}, + {&etagTrimmed, &etagTrimmed, nil, nil, errMod}, + + {&etagTrimmed, nil, &before, &before, nil}, + {&etagTrimmed, nil, &before, &after, nil}, + {&etagTrimmed, nil, &before, nil, nil}, + {&etagTrimmed, nil, &after, &before, errMod}, + {&etagTrimmed, nil, &after, &after, errMod}, + {&etagTrimmed, nil, &after, nil, errMod}, + {&etagTrimmed, nil, nil, &before, nil}, + {&etagTrimmed, nil, nil, &after, nil}, + {&etagTrimmed, nil, nil, nil, nil}, + + {nil, &etagTrimmed, &before, &before, errCond}, + {nil, &etagTrimmed, &before, &after, errMod}, + {nil, &etagTrimmed, &before, nil, errMod}, + {nil, &etagTrimmed, &after, &before, errCond}, + {nil, &etagTrimmed, &after, &after, errMod}, + {nil, &etagTrimmed, &after, nil, errMod}, + {nil, &etagTrimmed, nil, &before, errCond}, + {nil, &etagTrimmed, nil, &after, errMod}, + {nil, &etagTrimmed, nil, nil, errMod}, } { ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) _, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ diff --git a/tests/integration/PutObject.go b/tests/integration/PutObject.go index 2c6b4784..68ce81d0 100644 --- a/tests/integration/PutObject.go +++ b/tests/integration/PutObject.go @@ -313,6 +313,7 @@ func PutObject_conditional_writes(s *S3Conf) error { } etag := res.res.ETag + etagTrimmed := strings.Trim(*etag, `"`) incorrectEtag := getPtr("incorrect_etag") errPrecond := s3err.GetAPIError(s3err.ErrPreconditionFailed) @@ -331,6 +332,14 @@ func PutObject_conditional_writes(s *S3Conf) error { {obj, nil, incorrectEtag, nil}, {obj, nil, etag, errPrecond}, {obj, nil, nil, nil}, + + // precondition headers without quotes + {obj, &etagTrimmed, nil, nil}, + {obj, &etagTrimmed, &etagTrimmed, errPrecond}, + {obj, &etagTrimmed, incorrectEtag, nil}, + {obj, incorrectEtag, &etagTrimmed, errPrecond}, + {obj, nil, &etagTrimmed, errPrecond}, + // should ignore the precondition headers if // an object with the given name doesn't exist {"obj-1", incorrectEtag, etag, nil}, @@ -351,6 +360,7 @@ func PutObject_conditional_writes(s *S3Conf) error { // the exact same data. // to avoid ETag collision reassign the etag value *etag = *res.res.ETag + etagTrimmed = strings.Trim(*res.res.ETag, `"`) } if test.err == nil && err != nil { return fmt.Errorf("test case %v: expected no error, instead got %w", i, err) diff --git a/tests/integration/UploadPartCopy.go b/tests/integration/UploadPartCopy.go index 49c36f3b..54ababd5 100644 --- a/tests/integration/UploadPartCopy.go +++ b/tests/integration/UploadPartCopy.go @@ -17,6 +17,7 @@ package integration import ( "context" "fmt" + "strings" "time" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -663,6 +664,7 @@ func UploadPartCopy_conditional_reads(s *S3Conf) error { before := time.Now().AddDate(0, 0, -3) after := time.Now() etag := obj.res.ETag + etagTrimmed := strings.Trim(*etag, `"`) for i, test := range []struct { ifmatch *string @@ -761,6 +763,47 @@ func UploadPartCopy_conditional_reads(s *S3Conf) error { {nil, nil, nil, &before, errCond}, {nil, nil, nil, &after, nil}, {nil, nil, nil, nil, nil}, + + // if-match, if-non-match without quotes + {&etagTrimmed, getPtr("invalid_etag"), &before, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &before, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), &after, nil, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &before, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, &after, nil}, + {&etagTrimmed, getPtr("invalid_etag"), nil, nil, nil}, + + {&etagTrimmed, &etagTrimmed, &before, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &before, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &before, nil, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &before, errMod}, + {&etagTrimmed, &etagTrimmed, &after, &after, errMod}, + {&etagTrimmed, &etagTrimmed, &after, nil, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &before, errMod}, + {&etagTrimmed, &etagTrimmed, nil, &after, errMod}, + {&etagTrimmed, &etagTrimmed, nil, nil, errMod}, + + {&etagTrimmed, nil, &before, &before, nil}, + {&etagTrimmed, nil, &before, &after, nil}, + {&etagTrimmed, nil, &before, nil, nil}, + {&etagTrimmed, nil, &after, &before, errMod}, + {&etagTrimmed, nil, &after, &after, errMod}, + {&etagTrimmed, nil, &after, nil, errMod}, + {&etagTrimmed, nil, nil, &before, nil}, + {&etagTrimmed, nil, nil, &after, nil}, + {&etagTrimmed, nil, nil, nil, nil}, + + {nil, &etagTrimmed, &before, &before, errCond}, + {nil, &etagTrimmed, &before, &after, errMod}, + {nil, &etagTrimmed, &before, nil, errMod}, + {nil, &etagTrimmed, &after, &before, errCond}, + {nil, &etagTrimmed, &after, &after, errMod}, + {nil, &etagTrimmed, &after, nil, errMod}, + {nil, &etagTrimmed, nil, &before, errCond}, + {nil, &etagTrimmed, nil, &after, errMod}, + {nil, &etagTrimmed, nil, nil, errMod}, } { mpKey := "mp-key" mp, err := createMp(s3client, bucket, mpKey)