From a75aa9bad59615b7c0843d8d69401f17a3f4c02f Mon Sep 17 00:00:00 2001 From: niksis02 Date: Fri, 2 Jan 2026 22:59:13 +0400 Subject: [PATCH] fix: fixes if-none-match precondition header logic in object write operations Fixes #1708 This PR focuses on evaluating the `x-amz-if-none-match` precondition header for object PUT operations. If any value other than `*` is provided, a `NotImplemented` error is returned. If `If-Match` is used together with `If-None-Match`, regardless of the value combination, a `NotImplemented` error is returned. When only `If-None-Match: *` is specified, a `PreconditionFailed` error is returned if the object already exists in `PutObject` or `CompleteMultipartUpload`; if the object does not exist, object creation is allowed. --- backend/azure/azure.go | 19 +++++---- backend/common.go | 32 +++++++++++++++ backend/posix/posix.go | 14 ++----- tests/integration/CompleteMultipartUpload.go | 42 +++++++++++--------- tests/integration/PutObject.go | 39 ++++++++++-------- 5 files changed, 93 insertions(+), 53 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index ac674a47..f6edd601 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -2085,17 +2085,20 @@ func (az *Azure) evaluateWritePreconditions(ctx context.Context, bucket, object, return nil } // call HeadObject to evaluate preconditions - _, err := az.HeadObject(ctx, &s3.HeadObjectInput{ - Bucket: bucket, - Key: object, - IfMatch: ifMatch, - IfNoneMatch: ifNoneMatch, + res, err := az.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: bucket, + Key: object, }) - if errors.Is(err, s3err.GetAPIError(s3err.ErrNotModified)) { - return s3err.GetAPIError(s3err.ErrPreconditionFailed) + if err != nil && !errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchKey)) { + return err } - return err + var etag string + if res != nil { + etag = backend.GetStringFromPtr(res.ETag) + } + + return backend.EvaluateObjectPutPreconditions(etag, ifMatch, ifNoneMatch, !errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchKey))) } func getAclFromMetadata(meta map[string]*string, key key) (*auth.ACL, error) { diff --git a/backend/common.go b/backend/common.go index c3c995c4..44bd0d99 100644 --- a/backend/common.go +++ b/backend/common.go @@ -594,6 +594,38 @@ func EvaluateMatchPreconditions(etag string, ifMatch, ifNoneMatch *string) error return nil } +// EvaluateObjectPutPreconditions evaluates if-match and if-none-match preconditions +// for object PUT(PutObject, CompleteMultipartUpload) actions +func EvaluateObjectPutPreconditions(etag string, ifMatch, ifNoneMatch *string, objExists bool) error { + if ifMatch == nil && ifNoneMatch == nil { + return nil + } + + if ifNoneMatch != nil && *ifNoneMatch != "*" { + return s3err.GetAPIError(s3err.ErrNotImplemented) + } + + if ifNoneMatch != nil && ifMatch != nil { + return s3err.GetAPIError(s3err.ErrNotImplemented) + } + + if ifNoneMatch != nil && objExists { + return s3err.GetAPIError(s3err.ErrPreconditionFailed) + } + + if ifMatch != nil && !objExists { + return s3err.GetAPIError(s3err.ErrNoSuchKey) + } + + etag = strings.Trim(etag, `"`) + + if ifMatch != nil && *ifMatch != etag { + return s3err.GetAPIError(s3err.ErrPreconditionFailed) + } + + return nil +} + type ObjectDeletePreconditions struct { IfMatch *string IfMatchLastModTime *time.Time diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 54cce0d8..8af37c37 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1473,11 +1473,8 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C } b, err := p.meta.RetrieveAttribute(nil, bucket, object, etagkey) - if errors.Is(err, fs.ErrNotExist) && (input.IfMatch != nil || input.IfNoneMatch != nil) { - return s3response.CompleteMultipartUploadResult{}, "", s3err.GetAPIError(s3err.ErrNoSuchKey) - } - if err == nil { - err = backend.EvaluateMatchPreconditions(string(b), input.IfMatch, input.IfNoneMatch) + if err == nil || errors.Is(err, fs.ErrNotExist) || errors.Is(err, meta.ErrNoSuchKey) { + err = backend.EvaluateObjectPutPreconditions(string(b), input.IfMatch, input.IfNoneMatch, err == nil) if err != nil { return res, "", err } @@ -2916,11 +2913,8 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje // evaluate preconditions etagBytes, err := p.meta.RetrieveAttribute(nil, *po.Bucket, *po.Key, etagkey) - if errors.Is(err, fs.ErrNotExist) && (po.IfMatch != nil || po.IfNoneMatch != nil) { - return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchKey) - } - if err == nil { - err := backend.EvaluateMatchPreconditions(string(etagBytes), po.IfMatch, po.IfNoneMatch) + if err == nil || errors.Is(err, fs.ErrNotExist) || errors.Is(err, meta.ErrNoSuchKey) { + err = backend.EvaluateObjectPutPreconditions(string(etagBytes), po.IfMatch, po.IfNoneMatch, err == nil) if err != nil { return s3response.PutObjectOutput{}, err } diff --git a/tests/integration/CompleteMultipartUpload.go b/tests/integration/CompleteMultipartUpload.go index eb3501bf..e0b1380a 100644 --- a/tests/integration/CompleteMultipartUpload.go +++ b/tests/integration/CompleteMultipartUpload.go @@ -1378,6 +1378,7 @@ func CompleteMultipartUpload_conditional_writes(s *S3Conf) error { incorrectEtag := getPtr("incorrect_etag") errPrecond := s3err.GetAPIError(s3err.ErrPreconditionFailed) errNoSuchKey := s3err.GetAPIError(s3err.ErrNoSuchKey) + errNotImplemented := s3err.GetAPIError(s3err.ErrNotImplemented) for i, test := range []struct { obj string @@ -1386,28 +1387,33 @@ func CompleteMultipartUpload_conditional_writes(s *S3Conf) error { err error }{ {obj, etag, nil, nil}, - {obj, etag, etag, errPrecond}, - {obj, etag, incorrectEtag, nil}, - {obj, incorrectEtag, incorrectEtag, errPrecond}, - {obj, incorrectEtag, etag, errPrecond}, + {obj, etag, etag, errNotImplemented}, + {obj, etag, incorrectEtag, errNotImplemented}, + {obj, incorrectEtag, incorrectEtag, errNotImplemented}, + {obj, incorrectEtag, etag, errNotImplemented}, {obj, incorrectEtag, nil, errPrecond}, - {obj, nil, incorrectEtag, nil}, - {obj, nil, etag, errPrecond}, + {obj, nil, incorrectEtag, errNotImplemented}, + {obj, nil, etag, errNotImplemented}, + {obj, nil, getPtr("*"), errPrecond}, + {obj, etag, getPtr("*"), errNotImplemented}, {obj, nil, nil, nil}, - // should return NoSuchKey error, if any precondition - // header is present, but object doesn't exist - {"obj-1", incorrectEtag, etag, errNoSuchKey}, - {"obj-2", etag, etag, errNoSuchKey}, - {"obj-3", etag, incorrectEtag, errNoSuchKey}, - {"obj-4", incorrectEtag, nil, errNoSuchKey}, - {"obj-5", nil, etag, errNoSuchKey}, - // precondtion headers without quotes + // precondition headers without quotes {obj, &etagTrimmed, nil, nil}, - {obj, &etagTrimmed, &etagTrimmed, errPrecond}, - {obj, &etagTrimmed, incorrectEtag, nil}, - {obj, incorrectEtag, &etagTrimmed, errPrecond}, - {obj, nil, &etagTrimmed, errPrecond}, + {obj, &etagTrimmed, &etagTrimmed, errNotImplemented}, + {obj, &etagTrimmed, incorrectEtag, errNotImplemented}, + {obj, incorrectEtag, &etagTrimmed, errNotImplemented}, + {obj, nil, &etagTrimmed, errNotImplemented}, + + // object deson't exist tests + {"obj-1", incorrectEtag, etag, errNotImplemented}, + {"obj-2", etag, etag, errNotImplemented}, + {"obj-3", etag, nil, errNoSuchKey}, + {"obj-4", etag, incorrectEtag, errNotImplemented}, + {"obj-5", incorrectEtag, nil, errNoSuchKey}, + {"obj-6", nil, etag, errNotImplemented}, + {"obj-7", nil, getPtr("*"), nil}, + {"obj-8", etag, getPtr("*"), errNotImplemented}, } { res, err := putObjectWithData(0, &s3.PutObjectInput{ Bucket: &bucket, diff --git a/tests/integration/PutObject.go b/tests/integration/PutObject.go index 516e0852..0026b5cd 100644 --- a/tests/integration/PutObject.go +++ b/tests/integration/PutObject.go @@ -317,6 +317,7 @@ func PutObject_conditional_writes(s *S3Conf) error { incorrectEtag := getPtr("incorrect_etag") errPrecond := s3err.GetAPIError(s3err.ErrPreconditionFailed) errNoSuchKey := s3err.GetAPIError(s3err.ErrNoSuchKey) + errNotImplemented := s3err.GetAPIError(s3err.ErrNotImplemented) for i, test := range []struct { obj string @@ -325,29 +326,33 @@ func PutObject_conditional_writes(s *S3Conf) error { err error }{ {obj, etag, nil, nil}, - {obj, etag, etag, errPrecond}, - {obj, etag, incorrectEtag, nil}, - {obj, incorrectEtag, incorrectEtag, errPrecond}, - {obj, incorrectEtag, etag, errPrecond}, + {obj, etag, etag, errNotImplemented}, + {obj, etag, incorrectEtag, errNotImplemented}, + {obj, incorrectEtag, incorrectEtag, errNotImplemented}, + {obj, incorrectEtag, etag, errNotImplemented}, {obj, incorrectEtag, nil, errPrecond}, - {obj, nil, incorrectEtag, nil}, - {obj, nil, etag, errPrecond}, + {obj, nil, incorrectEtag, errNotImplemented}, + {obj, nil, etag, errNotImplemented}, + {obj, nil, getPtr("*"), errPrecond}, + {obj, etag, getPtr("*"), errNotImplemented}, {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}, + {obj, &etagTrimmed, &etagTrimmed, errNotImplemented}, + {obj, &etagTrimmed, incorrectEtag, errNotImplemented}, + {obj, incorrectEtag, &etagTrimmed, errNotImplemented}, + {obj, nil, &etagTrimmed, errNotImplemented}, - // should return NoSuchKey error, if any precondition - // header is present, but object doesn't exist - {"obj-1", incorrectEtag, etag, errNoSuchKey}, - {"obj-2", etag, etag, errNoSuchKey}, - {"obj-3", etag, incorrectEtag, errNoSuchKey}, - {"obj-4", incorrectEtag, nil, errNoSuchKey}, - {"obj-5", nil, etag, errNoSuchKey}, + // object deson't exist tests + {"obj-1", incorrectEtag, etag, errNotImplemented}, + {"obj-2", etag, etag, errNotImplemented}, + {"obj-3", etag, nil, errNoSuchKey}, + {"obj-4", etag, incorrectEtag, errNotImplemented}, + {"obj-5", incorrectEtag, nil, errNoSuchKey}, + {"obj-6", nil, etag, errNotImplemented}, + {"obj-7", nil, getPtr("*"), nil}, + {"obj-8", etag, getPtr("*"), errNotImplemented}, } { res, err := putObjectWithData(0, &s3.PutObjectInput{ Bucket: &bucket,