From 8a3dbcf97d996ea4955426e6b8a76eb96d2cd92a Mon Sep 17 00:00:00 2001 From: hgichon Date: Wed, 15 Jul 2026 11:21:20 +0900 Subject: [PATCH] fix: omit null version id for suspended buckets Keep null as the stored object version ID when bucket versioning is suspended, but omit x-amz-version-id from the PutObject response to match Amazon S3. Update the existing versioning integration coverage to distinguish the PutObject response from the null version returned by ListObjectVersions, and verify empty response headers are not rendered. Fixes #2164 Co-authored-by: Claude Opus 4.8 (1M context) --- backend/posix/posix.go | 6 ++++++ s3api/controllers/base_test.go | 13 +++++++++---- tests/integration/ListObjectVersions.go | 6 +++--- tests/integration/versioning.go | 12 ++++++------ 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index bf77d14e..8933de2d 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -3938,6 +3938,12 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje } } + // "null" identifies the current object internally while versioning is + // suspended, but S3 omits the version ID from the PutObject response. + if versionID == nullVersionId { + versionID = "" + } + err = postprocess(f.File()) if err != nil { return s3response.PutObjectOutput{}, diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index 00644a8f..fb403825 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -198,9 +198,10 @@ func TestSetResponseHeaders(t *testing.T) { headers map[string]*string } tests := []struct { - name string - args args - expected map[string]string + name string + args args + expected map[string]string + unexpected []string }{ { name: "should not set if map is nil", @@ -215,7 +216,7 @@ func TestSetResponseHeaders(t *testing.T) { headers: map[string]*string{ "x-amz-checksum-algorithm": utils.GetStringPtr("crc32"), "x-amz-meta-key": utils.GetStringPtr("meta_key"), - "x-amz-mp-size": utils.GetStringPtr(""), + "x-amz-version-id": utils.GetStringPtr(""), "something": nil, }, }, @@ -223,6 +224,7 @@ func TestSetResponseHeaders(t *testing.T) { "x-amz-checksum-algorithm": "crc32", "x-amz-meta-key": "meta_key", }, + unexpected: []string{"x-amz-version-id", "something"}, }, } for _, tt := range tests { @@ -236,6 +238,9 @@ func TestSetResponseHeaders(t *testing.T) { assert.Equal(t, val, string(v)) } } + for _, key := range tt.unexpected { + assert.Empty(t, ctx.Response().Header.Peek(key)) + } }) } } diff --git a/tests/integration/ListObjectVersions.go b/tests/integration/ListObjectVersions.go index bca81d66..27a3932e 100644 --- a/tests/integration/ListObjectVersions.go +++ b/tests/integration/ListObjectVersions.go @@ -336,9 +336,9 @@ func ListObjectVersions_containing_null_versionId_obj(s *S3Conf) error { return err } - if getString(out.res.VersionId) != nullVersionId { - return fmt.Errorf("expected the uploaded object versionId to be %v, instead got %v", - nullVersionId, getString(out.res.VersionId)) + if out.res.VersionId != nil { + return fmt.Errorf("expected PutObject response to omit versionId, instead got %v", + getString(out.res.VersionId)) } versions[0].IsLatest = getBoolPtr(false) diff --git a/tests/integration/versioning.go b/tests/integration/versioning.go index 274d5376..2662a45a 100644 --- a/tests/integration/versioning.go +++ b/tests/integration/versioning.go @@ -64,9 +64,9 @@ func Versioning_PutObject_suspended_null_versionId_obj(s *S3Conf) error { return err } - if getString(out.res.VersionId) != nullVersionId { - return fmt.Errorf("expected the uploaded object versionId to be %v, instead got %v", - nullVersionId, getString(out.res.VersionId)) + if out.res.VersionId != nil { + return fmt.Errorf("expected PutObject response to omit versionId, instead got %v", + getString(out.res.VersionId)) } return nil @@ -161,9 +161,9 @@ func Versioning_PutObject_overwrite_null_versionId_obj(s *S3Conf) error { return err } - if getString(out.res.VersionId) != nullVersionId { - return fmt.Errorf("expected the uploaded object versionId to be %v, insted got %v", - nullVersionId, getString(out.res.VersionId)) + if out.res.VersionId != nil { + return fmt.Errorf("expected PutObject response to omit versionId, instead got %v", + getString(out.res.VersionId)) } versions[0].IsLatest = getBoolPtr(false)