From 10a30a83e1bdfca25c19506ef4d0d39d22e17628 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 4 Mar 2026 12:52:09 -0800 Subject: [PATCH] s3api: add GetObjectAttributes API support (#8504) * s3api: add error code and header constants for GetObjectAttributes Add ErrInvalidAttributeName error code and header constants (X-Amz-Object-Attributes, X-Amz-Max-Parts, X-Amz-Part-Number-Marker, X-Amz-Delete-Marker) needed by the S3 GetObjectAttributes API. Co-Authored-By: Claude Opus 4.6 * s3api: implement GetObjectAttributes handler Add GetObjectAttributesHandler that returns selected object metadata (ETag, Checksum, StorageClass, ObjectSize, ObjectParts) without returning the object body. Follows the same versioning and conditional header patterns as HeadObjectHandler. The handler parses the X-Amz-Object-Attributes header to determine which attributes to include in the XML response, and supports ObjectParts pagination via X-Amz-Max-Parts and X-Amz-Part-Number-Marker. Ref: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObjectAttributes.html Co-Authored-By: Claude Opus 4.6 * s3api: register GetObjectAttributes route Register the GET /{object}?attributes route for the GetObjectAttributes API, placed before other object query routes to ensure proper matching. Co-Authored-By: Claude Opus 4.6 * s3api: add integration tests for GetObjectAttributes Test coverage: - Basic: simple object with all attribute types - MultipartObject: multipart upload with parts pagination - SelectiveAttributes: requesting only specific attributes - InvalidAttribute: server rejects invalid attribute names - NonExistentObject: returns NoSuchKey for missing objects Co-Authored-By: Claude Opus 4.6 * s3api: add versioned object test for GetObjectAttributes Test puts two versions of the same object and verifies that: - GetObjectAttributes returns the latest version by default - GetObjectAttributes with versionId returns the specific version - ObjectSize and VersionId are correct for each version Co-Authored-By: Claude Opus 4.6 * s3api: fix combined conditional header evaluation per RFC 7232 Per RFC 7232: - Section 3.4: If-Unmodified-Since MUST be ignored when If-Match is present (If-Match is the more accurate replacement) - Section 3.3: If-Modified-Since MUST be ignored when If-None-Match is present (If-None-Match is the more accurate replacement) Previously, all four conditional headers were evaluated independently. This caused incorrect 412 responses when If-Match succeeded but If-Unmodified-Since failed (should return 200 per AWS S3 behavior). Fix applied to both validateConditionalHeadersForReads (GET/HEAD) and validateConditionalHeaders (PUT) paths. Co-Authored-By: Claude Opus 4.6 * s3api: add conditional header combination tests for GetObjectAttributes Test the RFC 7232 combined conditional header semantics: - If-Match=true + If-Unmodified-Since=false => 200 (If-Unmodified-Since ignored) - If-None-Match=false + If-Modified-Since=true => 304 (If-Modified-Since ignored) - If-None-Match=true + If-Modified-Since=false => 200 (If-Modified-Since ignored) - If-Match=true + If-Unmodified-Since=true => 200 - If-Match=false => 412 regardless Co-Authored-By: Claude Opus 4.6 * s3api: document Checksum attribute as not yet populated Checksum is accepted in validation (so clients requesting it don't get a 400 error, matching AWS behavior for objects without checksums) but SeaweedFS does not yet store S3 checksums. Add a comment explaining this and noting where to populate it when checksum storage is added. Co-Authored-By: Claude Opus 4.6 * s3api: add s3:GetObjectAttributes IAM action for ?attributes query Previously, GET /{object}?attributes resolved to s3:GetObject via the fallback path since resolveFromQueryParameters had no case for the "attributes" query parameter. Add S3_ACTION_GET_OBJECT_ATTRIBUTES constant ("s3:GetObjectAttributes") and a branch in resolveFromQueryParameters to return it for GET requests with the "attributes" query parameter, so IAM policies can distinguish GetObjectAttributes from GetObject. Co-Authored-By: Claude Opus 4.6 * s3api: evaluate conditional headers after version resolution Move conditional header evaluation (If-Match, If-None-Match, etc.) to after the version resolution step in GetObjectAttributesHandler. This ensures that when a specific versionId is requested, conditions are checked against the correct version entry rather than always against the latest version. Co-Authored-By: Claude Opus 4.6 * s3api: use bounded HTTP client in GetObjectAttributes tests Replace http.DefaultClient with a timeout-aware http.Client (10s) in the signedGetObjectAttributes helper and testGetObjectAttributesInvalid to prevent tests from hanging indefinitely. Co-Authored-By: Claude Opus 4.6 * s3api: check attributes query before versionId in action resolver Move the GetObjectAttributes action check before the versionId check in resolveFromQueryParameters. This fixes GET /bucket/key?attributes&versionId=xyz being incorrectly classified as s3:GetObjectVersion instead of s3:GetObjectAttributes. Co-Authored-By: Claude Opus 4.6 * s3api: add tests for versioned conditional headers and action resolver Add integration test that verifies conditional headers (If-Match, If-None-Match) are evaluated against the requested version entry, not the latest version. This covers the fix in 55c409dec. Add unit test for ResolveS3Action verifying that the attributes query parameter takes precedence over versionId, so GET ?attributes&versionId resolves to s3:GetObjectAttributes. This covers the fix in b92c61c95. Co-Authored-By: Claude Opus 4.6 * s3api: guard negative chunk indices and rename PartsCount field Add bounds checks for b.StartChunk >= 0 and b.EndChunk >= 0 in buildObjectAttributesParts to prevent panics from corrupted metadata with negative index values. Rename ObjectAttributesParts.PartsCount to TotalPartsCount to match the AWS SDK v2 Go field naming convention, while preserving the XML element name "PartsCount" via the struct tag. Co-Authored-By: Claude Opus 4.6 * s3api: reject malformed max-parts and part-number-marker headers Return ErrInvalidMaxParts and ErrInvalidPartNumberMarker when the X-Amz-Max-Parts or X-Amz-Part-Number-Marker headers contain non-integer or negative values, matching ListObjectPartsHandler behavior. Previously these were silently ignored with defaults. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- test/s3/normal/get_object_attributes_test.go | 573 ++++++++++++++++++ weed/s3api/s3_action_resolver.go | 7 + weed/s3api/s3_action_resolver_test.go | 62 ++ weed/s3api/s3_constants/header.go | 6 + weed/s3api/s3_constants/s3_action_strings.go | 3 +- .../s3api/s3api_object_handlers_attributes.go | 326 ++++++++++ weed/s3api/s3api_object_handlers_put.go | 28 +- weed/s3api/s3api_server.go | 2 + weed/s3api/s3err/s3api_errors.go | 8 + 9 files changed, 1008 insertions(+), 7 deletions(-) create mode 100644 test/s3/normal/get_object_attributes_test.go create mode 100644 weed/s3api/s3_action_resolver_test.go create mode 100644 weed/s3api/s3api_object_handlers_attributes.go diff --git a/test/s3/normal/get_object_attributes_test.go b/test/s3/normal/get_object_attributes_test.go new file mode 100644 index 000000000..95dc7d8f7 --- /dev/null +++ b/test/s3/normal/get_object_attributes_test.go @@ -0,0 +1,573 @@ +package example + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "strings" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws" + v1credentials "github.com/aws/aws-sdk-go/aws/credentials" + v1signer "github.com/aws/aws-sdk-go/aws/signer/v4" + v1s3 "github.com/aws/aws-sdk-go/service/s3" + v2aws "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/credentials" + v2s3 "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newS3V2Client creates an AWS SDK v2 S3 client from the test cluster. +func newS3V2Client(cluster *TestCluster) *v2s3.Client { + return v2s3.New(v2s3.Options{ + Region: testRegion, + BaseEndpoint: v2aws.String(cluster.s3Endpoint), + Credentials: v2aws.NewCredentialsCache(credentials.NewStaticCredentialsProvider(testAccessKey, testSecretKey, "")), + UsePathStyle: true, + }) +} + +func TestGetObjectAttributes(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + cluster, err := startMiniCluster(t) + require.NoError(t, err) + defer cluster.Stop() + + t.Run("Basic", func(t *testing.T) { + testGetObjectAttributesBasic(t, cluster) + }) + t.Run("MultipartObject", func(t *testing.T) { + testGetObjectAttributesMultipart(t, cluster) + }) + t.Run("SelectiveAttributes", func(t *testing.T) { + testGetObjectAttributesSelective(t, cluster) + }) + t.Run("InvalidAttribute", func(t *testing.T) { + testGetObjectAttributesInvalid(t, cluster) + }) + t.Run("NonExistentObject", func(t *testing.T) { + testGetObjectAttributesNotFound(t, cluster) + }) + t.Run("VersionedObject", func(t *testing.T) { + testGetObjectAttributesVersioned(t, cluster) + }) + t.Run("ConditionalHeaders", func(t *testing.T) { + testGetObjectAttributesConditionalHeaders(t, cluster) + }) + t.Run("VersionedConditionalHeaders", func(t *testing.T) { + testGetObjectAttributesVersionedConditionalHeaders(t, cluster) + }) +} + +func testGetObjectAttributesBasic(t *testing.T, cluster *TestCluster) { + bucketName := createTestBucket(t, cluster, "test-goa-basic-") + objectKey := "test-object.txt" + objectData := "Hello, GetObjectAttributes!" + + _, err := cluster.s3Client.PutObject(&v1s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + Body: bytes.NewReader([]byte(objectData)), + }) + require.NoError(t, err) + + client := newS3V2Client(cluster) + resp, err := client.GetObjectAttributes(context.Background(), &v2s3.GetObjectAttributesInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String(objectKey), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesEtag, + types.ObjectAttributesStorageClass, + types.ObjectAttributesObjectSize, + types.ObjectAttributesObjectParts, + }, + }) + require.NoError(t, err) + + // ETag should be present and non-empty + require.NotNil(t, resp.ETag) + assert.NotEmpty(t, *resp.ETag) + assert.False(t, strings.Contains(*resp.ETag, `"`), "ETag in XML body should not have quotes") + + // ObjectSize should match + require.NotNil(t, resp.ObjectSize) + assert.Equal(t, int64(len(objectData)), *resp.ObjectSize) + + // StorageClass should be STANDARD (default) + assert.Equal(t, "STANDARD", string(resp.StorageClass)) + + // ObjectParts should be nil for non-multipart objects + assert.Nil(t, resp.ObjectParts) + + // LastModified header should be present + assert.NotNil(t, resp.LastModified) + + t.Logf("Basic GetObjectAttributes passed: ETag=%s, Size=%d, StorageClass=%s", + *resp.ETag, *resp.ObjectSize, resp.StorageClass) +} + +func testGetObjectAttributesMultipart(t *testing.T, cluster *TestCluster) { + bucketName := createTestBucket(t, cluster, "test-goa-mp-") + objectKey := "test-multipart.bin" + + // Create a 2-part multipart upload + part1Data := bytes.Repeat([]byte("A"), 5*1024*1024) // 5MB (minimum part size) + part2Data := bytes.Repeat([]byte("B"), 3*1024*1024) // 3MB + + initResp, err := cluster.s3Client.CreateMultipartUpload(&v1s3.CreateMultipartUploadInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + }) + require.NoError(t, err) + uploadID := initResp.UploadId + + part1Resp, err := cluster.s3Client.UploadPart(&v1s3.UploadPartInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + PartNumber: aws.Int64(1), + UploadId: uploadID, + Body: bytes.NewReader(part1Data), + }) + require.NoError(t, err) + + part2Resp, err := cluster.s3Client.UploadPart(&v1s3.UploadPartInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + PartNumber: aws.Int64(2), + UploadId: uploadID, + Body: bytes.NewReader(part2Data), + }) + require.NoError(t, err) + + _, err = cluster.s3Client.CompleteMultipartUpload(&v1s3.CompleteMultipartUploadInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + UploadId: uploadID, + MultipartUpload: &v1s3.CompletedMultipartUpload{ + Parts: []*v1s3.CompletedPart{ + {ETag: part1Resp.ETag, PartNumber: aws.Int64(1)}, + {ETag: part2Resp.ETag, PartNumber: aws.Int64(2)}, + }, + }, + }) + require.NoError(t, err) + + // Wait briefly for metadata to settle + time.Sleep(200 * time.Millisecond) + + client := newS3V2Client(cluster) + resp, err := client.GetObjectAttributes(context.Background(), &v2s3.GetObjectAttributesInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String(objectKey), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesObjectParts, + types.ObjectAttributesObjectSize, + }, + }) + require.NoError(t, err) + + require.NotNil(t, resp.ObjectSize) + assert.Equal(t, int64(len(part1Data)+len(part2Data)), *resp.ObjectSize) + + require.NotNil(t, resp.ObjectParts, "ObjectParts should be present for multipart objects") + assert.Equal(t, int32(2), *resp.ObjectParts.TotalPartsCount) + require.Len(t, resp.ObjectParts.Parts, 2) + assert.Equal(t, int32(1), *resp.ObjectParts.Parts[0].PartNumber) + assert.Equal(t, int64(len(part1Data)), *resp.ObjectParts.Parts[0].Size) + assert.Equal(t, int32(2), *resp.ObjectParts.Parts[1].PartNumber) + assert.Equal(t, int64(len(part2Data)), *resp.ObjectParts.Parts[1].Size) + + // Test pagination: MaxParts=1 + resp2, err := client.GetObjectAttributes(context.Background(), &v2s3.GetObjectAttributesInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String(objectKey), + MaxParts: v2aws.Int32(1), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesObjectParts, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp2.ObjectParts) + assert.Len(t, resp2.ObjectParts.Parts, 1) + assert.True(t, *resp2.ObjectParts.IsTruncated) + assert.Equal(t, int32(2), *resp2.ObjectParts.TotalPartsCount) + + t.Logf("Multipart GetObjectAttributes passed: %d parts, total size %d", + *resp.ObjectParts.TotalPartsCount, *resp.ObjectSize) +} + +func testGetObjectAttributesSelective(t *testing.T, cluster *TestCluster) { + bucketName := createTestBucket(t, cluster, "test-goa-sel-") + objectKey := "test-selective.txt" + objectData := "Selective attributes test" + + _, err := cluster.s3Client.PutObject(&v1s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + Body: bytes.NewReader([]byte(objectData)), + }) + require.NoError(t, err) + + client := newS3V2Client(cluster) + + // Request only ETag + resp, err := client.GetObjectAttributes(context.Background(), &v2s3.GetObjectAttributesInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String(objectKey), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesEtag, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp.ETag) + assert.NotEmpty(t, *resp.ETag) + assert.Nil(t, resp.ObjectSize, "ObjectSize should not be present when not requested") + assert.Empty(t, string(resp.StorageClass), "StorageClass should not be present when not requested") + assert.Nil(t, resp.ObjectParts, "ObjectParts should not be present when not requested") + + t.Logf("Selective GetObjectAttributes passed: ETag=%s", *resp.ETag) +} + +func testGetObjectAttributesInvalid(t *testing.T, cluster *TestCluster) { + bucketName := createTestBucket(t, cluster, "test-goa-inv-") + objectKey := "test-object.txt" + + _, err := cluster.s3Client.PutObject(&v1s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + Body: bytes.NewReader([]byte("test")), + }) + require.NoError(t, err) + + // Use raw HTTP to send an invalid attribute name since the SDK validates + reqURL := fmt.Sprintf("%s/%s/%s?attributes", cluster.s3Endpoint, bucketName, objectKey) + req, err := http.NewRequest("GET", reqURL, nil) + require.NoError(t, err) + req.Header.Set("X-Amz-Object-Attributes", "InvalidAttr") + + signer := v1signer.NewSigner(v1credentials.NewStaticCredentials(testAccessKey, testSecretKey, "")) + _, err = signer.Sign(req, nil, "s3", testRegion, time.Now()) + require.NoError(t, err) + + client := &http.Client{Timeout: 10 * time.Second} + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + + assert.Equal(t, 400, resp.StatusCode) + t.Logf("Invalid attribute test passed: got %d", resp.StatusCode) +} + +func testGetObjectAttributesNotFound(t *testing.T, cluster *TestCluster) { + bucketName := createTestBucket(t, cluster, "test-goa-nf-") + + client := newS3V2Client(cluster) + _, err := client.GetObjectAttributes(context.Background(), &v2s3.GetObjectAttributesInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("nonexistent-key"), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesEtag, + }, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "NoSuchKey") + + t.Logf("NotFound GetObjectAttributes passed") +} + +func testGetObjectAttributesVersioned(t *testing.T, cluster *TestCluster) { + client := newS3V2Client(cluster) + bucketName := createTestBucket(t, cluster, "test-goa-ver-") + + // Enable versioning + _, err := client.PutBucketVersioning(context.Background(), &v2s3.PutBucketVersioningInput{ + Bucket: v2aws.String(bucketName), + VersioningConfiguration: &types.VersioningConfiguration{ + Status: types.BucketVersioningStatusEnabled, + }, + }) + require.NoError(t, err) + time.Sleep(200 * time.Millisecond) + + // Put two versions of the same object + v1Data := "version 1 content" + putResp1, err := client.PutObject(context.Background(), &v2s3.PutObjectInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("versioned-key"), + Body: strings.NewReader(v1Data), + }) + require.NoError(t, err) + require.NotNil(t, putResp1.VersionId) + versionId1 := *putResp1.VersionId + + v2Data := "version 2 content - longer" + putResp2, err := client.PutObject(context.Background(), &v2s3.PutObjectInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("versioned-key"), + Body: strings.NewReader(v2Data), + }) + require.NoError(t, err) + require.NotNil(t, putResp2.VersionId) + versionId2 := *putResp2.VersionId + + assert.NotEqual(t, versionId1, versionId2, "versions should differ") + + // GetObjectAttributes for latest version (v2) + resp, err := client.GetObjectAttributes(context.Background(), &v2s3.GetObjectAttributesInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("versioned-key"), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesObjectSize, + types.ObjectAttributesEtag, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp.ObjectSize) + assert.Equal(t, int64(len(v2Data)), *resp.ObjectSize) + require.NotNil(t, resp.VersionId) + assert.Equal(t, versionId2, *resp.VersionId) + + // GetObjectAttributes for specific older version (v1) + resp1, err := client.GetObjectAttributes(context.Background(), &v2s3.GetObjectAttributesInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("versioned-key"), + VersionId: v2aws.String(versionId1), + ObjectAttributes: []types.ObjectAttributes{ + types.ObjectAttributesObjectSize, + types.ObjectAttributesEtag, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp1.ObjectSize) + assert.Equal(t, int64(len(v1Data)), *resp1.ObjectSize) + require.NotNil(t, resp1.VersionId) + assert.Equal(t, versionId1, *resp1.VersionId) + + t.Logf("Versioned GetObjectAttributes passed: v1 size=%d (id=%s), v2 size=%d (id=%s)", + *resp1.ObjectSize, versionId1, *resp.ObjectSize, versionId2) +} + +// signedGetObjectAttributes creates a signed GET request for ?attributes with custom headers. +func signedGetObjectAttributes(t *testing.T, cluster *TestCluster, bucketName, objectKey string, extraHeaders map[string]string) *http.Response { + reqURL := fmt.Sprintf("%s/%s/%s?attributes", cluster.s3Endpoint, bucketName, objectKey) + req, err := http.NewRequest("GET", reqURL, nil) + require.NoError(t, err) + req.Header.Set("X-Amz-Object-Attributes", "ETag,ObjectSize") + for k, v := range extraHeaders { + req.Header.Set(k, v) + } + signer := v1signer.NewSigner(v1credentials.NewStaticCredentials(testAccessKey, testSecretKey, "")) + _, err = signer.Sign(req, nil, "s3", testRegion, time.Now()) + require.NoError(t, err) + client := &http.Client{Timeout: 10 * time.Second} + resp, err := client.Do(req) + require.NoError(t, err) + return resp +} + +func testGetObjectAttributesConditionalHeaders(t *testing.T, cluster *TestCluster) { + bucketName := createTestBucket(t, cluster, "test-goa-cond-") + objectKey := "cond-test.txt" + + _, err := cluster.s3Client.PutObject(&v1s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + Body: bytes.NewReader([]byte("conditional headers test")), + }) + require.NoError(t, err) + + // Get the ETag and Last-Modified for the object + headResp, err := cluster.s3Client.HeadObject(&v1s3.HeadObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + }) + require.NoError(t, err) + etag := aws.StringValue(headResp.ETag) + lastModified := headResp.LastModified + require.NotNil(t, lastModified) + + pastDate := lastModified.Add(-1 * time.Hour).UTC().Format(http.TimeFormat) + futureDate := lastModified.Add(1 * time.Hour).UTC().Format(http.TimeFormat) + + // RFC 7232: If-Match true + If-Unmodified-Since false => 200 OK + // If-Unmodified-Since is ignored when If-Match is present + t.Run("IfMatch_true_IfUnmodifiedSince_false", func(t *testing.T) { + resp := signedGetObjectAttributes(t, cluster, bucketName, objectKey, map[string]string{ + "If-Match": etag, + "If-Unmodified-Since": pastDate, // object was modified after this => false + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 200, resp.StatusCode, + "If-Match=true should return 200 even when If-Unmodified-Since=false (RFC 7232 Section 3.4)") + }) + + // RFC 7232: If-None-Match false + If-Modified-Since true => 304 Not Modified + // If-Modified-Since is ignored when If-None-Match is present + t.Run("IfNoneMatch_false_IfModifiedSince_true", func(t *testing.T) { + resp := signedGetObjectAttributes(t, cluster, bucketName, objectKey, map[string]string{ + "If-None-Match": etag, + "If-Modified-Since": pastDate, // object was modified after this => true + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 304, resp.StatusCode, + "If-None-Match=false (ETag match) should return 304 even when If-Modified-Since=true (RFC 7232 Section 3.3)") + }) + + // If-Match succeeds, If-Unmodified-Since also succeeds => 200 + t.Run("IfMatch_true_IfUnmodifiedSince_true", func(t *testing.T) { + resp := signedGetObjectAttributes(t, cluster, bucketName, objectKey, map[string]string{ + "If-Match": etag, + "If-Unmodified-Since": futureDate, + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 200, resp.StatusCode) + }) + + // If-None-Match passes (ETag differs), If-Modified-Since ignored => 200 + // Per RFC 7232, If-Modified-Since is ignored when If-None-Match is present + t.Run("IfNoneMatch_true_IfModifiedSince_ignored", func(t *testing.T) { + resp := signedGetObjectAttributes(t, cluster, bucketName, objectKey, map[string]string{ + "If-None-Match": `"nonexistent-etag"`, + "If-Modified-Since": futureDate, // would fail alone, but is ignored + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 200, resp.StatusCode, + "If-None-Match=true means If-Modified-Since is ignored, should return 200 (RFC 7232 Section 3.3)") + }) + + // If-Match fails => 412 regardless of If-Unmodified-Since + t.Run("IfMatch_false", func(t *testing.T) { + resp := signedGetObjectAttributes(t, cluster, bucketName, objectKey, map[string]string{ + "If-Match": `"wrong-etag"`, + "If-Unmodified-Since": futureDate, + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 412, resp.StatusCode) + }) + + t.Logf("Conditional headers tests passed") +} + +// signedGetObjectAttributesVersioned creates a signed GET request for ?attributes&versionId=... with custom headers. +func signedGetObjectAttributesVersioned(t *testing.T, cluster *TestCluster, bucketName, objectKey, versionId string, extraHeaders map[string]string) *http.Response { + reqURL := fmt.Sprintf("%s/%s/%s?attributes&versionId=%s", cluster.s3Endpoint, bucketName, objectKey, versionId) + req, err := http.NewRequest("GET", reqURL, nil) + require.NoError(t, err) + req.Header.Set("X-Amz-Object-Attributes", "ETag,ObjectSize") + for k, v := range extraHeaders { + req.Header.Set(k, v) + } + signer := v1signer.NewSigner(v1credentials.NewStaticCredentials(testAccessKey, testSecretKey, "")) + _, err = signer.Sign(req, nil, "s3", testRegion, time.Now()) + require.NoError(t, err) + client := &http.Client{Timeout: 10 * time.Second} + resp, err := client.Do(req) + require.NoError(t, err) + return resp +} + +func testGetObjectAttributesVersionedConditionalHeaders(t *testing.T, cluster *TestCluster) { + client := newS3V2Client(cluster) + bucketName := createTestBucket(t, cluster, "test-goa-vcond-") + + // Enable versioning + _, err := client.PutBucketVersioning(context.Background(), &v2s3.PutBucketVersioningInput{ + Bucket: v2aws.String(bucketName), + VersioningConfiguration: &types.VersioningConfiguration{ + Status: types.BucketVersioningStatusEnabled, + }, + }) + require.NoError(t, err) + time.Sleep(200 * time.Millisecond) + + // Put two versions with different content (different ETags) + v1Data := "version 1 - original" + putResp1, err := client.PutObject(context.Background(), &v2s3.PutObjectInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("vcond-key"), + Body: strings.NewReader(v1Data), + }) + require.NoError(t, err) + require.NotNil(t, putResp1.VersionId) + vid1 := *putResp1.VersionId + + v2Data := "version 2 - updated content" + putResp2, err := client.PutObject(context.Background(), &v2s3.PutObjectInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("vcond-key"), + Body: strings.NewReader(v2Data), + }) + require.NoError(t, err) + require.NotNil(t, putResp2.VersionId) + vid2 := *putResp2.VersionId + + // Get ETags for each version + headV1, err := client.HeadObject(context.Background(), &v2s3.HeadObjectInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("vcond-key"), + VersionId: v2aws.String(vid1), + }) + require.NoError(t, err) + etagV1 := *headV1.ETag + + headV2, err := client.HeadObject(context.Background(), &v2s3.HeadObjectInput{ + Bucket: v2aws.String(bucketName), + Key: v2aws.String("vcond-key"), + VersionId: v2aws.String(vid2), + }) + require.NoError(t, err) + etagV2 := *headV2.ETag + require.NotEqual(t, etagV1, etagV2, "versions should have different ETags") + + // If-Match with v1's ETag + versionId=v1 => 200 + // Before the fix, this would fail with 412 because conditional headers + // were evaluated against the latest version (v2) whose ETag differs + t.Run("IfMatch_v1_etag_versionId_v1", func(t *testing.T) { + resp := signedGetObjectAttributesVersioned(t, cluster, bucketName, "vcond-key", vid1, map[string]string{ + "If-Match": etagV1, + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 200, resp.StatusCode, + "If-Match with v1 ETag targeting versionId=v1 should return 200") + }) + + // If-Match with v2's ETag + versionId=v1 => 412 + // The ETag doesn't match v1, so this should fail + t.Run("IfMatch_v2_etag_versionId_v1", func(t *testing.T) { + resp := signedGetObjectAttributesVersioned(t, cluster, bucketName, "vcond-key", vid1, map[string]string{ + "If-Match": etagV2, + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 412, resp.StatusCode, + "If-Match with v2 ETag targeting versionId=v1 should return 412") + }) + + // If-None-Match with v1's ETag + versionId=v1 => 304 + t.Run("IfNoneMatch_v1_etag_versionId_v1", func(t *testing.T) { + resp := signedGetObjectAttributesVersioned(t, cluster, bucketName, "vcond-key", vid1, map[string]string{ + "If-None-Match": etagV1, + }) + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) + assert.Equal(t, 304, resp.StatusCode, + "If-None-Match with v1 ETag targeting versionId=v1 should return 304") + }) + + t.Logf("Versioned conditional headers tests passed: vid1=%s, vid2=%s", vid1, vid2) +} diff --git a/weed/s3api/s3_action_resolver.go b/weed/s3api/s3_action_resolver.go index f7acd88ae..1a9edfca8 100644 --- a/weed/s3api/s3_action_resolver.go +++ b/weed/s3api/s3_action_resolver.go @@ -159,6 +159,13 @@ func resolveFromQueryParameters(query url.Values, method string, hasObject bool) } } + // GetObjectAttributes (object-level only) + // Must be checked before versionId, because GET /bucket/key?attributes&versionId=xyz + // is a GetObjectAttributes request, not a GetObjectVersion request + if hasObject && query.Has("attributes") && method == http.MethodGet { + return s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES + } + // Versioning operations - distinguish between versionId (specific version) and versions (list versions) // versionId: Used to access/delete a specific version of an object (e.g., GET /bucket/key?versionId=xyz) if query.Has("versionId") { diff --git a/weed/s3api/s3_action_resolver_test.go b/weed/s3api/s3_action_resolver_test.go new file mode 100644 index 000000000..c95ec3972 --- /dev/null +++ b/weed/s3api/s3_action_resolver_test.go @@ -0,0 +1,62 @@ +package s3api + +import ( + "net/http" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" +) + +func TestResolveS3Action_AttributesBeforeVersionId(t *testing.T) { + tests := []struct { + name string + query string + method string + baseAction string + object string + want string + }{ + { + name: "attributes only", + query: "attributes", + method: http.MethodGet, + baseAction: s3_constants.ACTION_READ, + object: "key", + want: s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES, + }, + { + name: "attributes with versionId", + query: "attributes&versionId=abc123", + method: http.MethodGet, + baseAction: s3_constants.ACTION_READ, + object: "key", + want: s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES, + }, + { + name: "versionId only GET", + query: "versionId=abc123", + method: http.MethodGet, + baseAction: s3_constants.ACTION_READ, + object: "key", + want: s3_constants.S3_ACTION_GET_OBJECT_VERSION, + }, + { + name: "versionId only DELETE", + query: "versionId=abc123", + method: http.MethodDelete, + baseAction: s3_constants.ACTION_WRITE, + object: "key", + want: s3_constants.S3_ACTION_DELETE_OBJECT_VERSION, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, _ := http.NewRequest(tt.method, "http://localhost/bucket/"+tt.object+"?"+tt.query, nil) + got := ResolveS3Action(r, tt.baseAction, "bucket", tt.object) + if got != tt.want { + t.Errorf("ResolveS3Action() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/weed/s3api/s3_constants/header.go b/weed/s3api/s3_constants/header.go index 70f02e1cd..39529f7bc 100644 --- a/weed/s3api/s3_constants/header.go +++ b/weed/s3api/s3_constants/header.go @@ -45,6 +45,12 @@ const ( AmzObjectTaggingDirective = "X-Amz-Tagging-Directive" AmzTagCount = "x-amz-tagging-count" + // GetObjectAttributes headers + AmzObjectAttributes = "X-Amz-Object-Attributes" + AmzMaxParts = "X-Amz-Max-Parts" + AmzPartNumberMarker = "X-Amz-Part-Number-Marker" + AmzDeleteMarker = "X-Amz-Delete-Marker" + SeaweedFSUploadId = "X-Seaweedfs-Upload-Id" SeaweedFSMultipartPartsCount = "X-Seaweedfs-Multipart-Parts-Count" SeaweedFSMultipartPartBoundaries = "X-Seaweedfs-Multipart-Part-Boundaries" // JSON: [{part:1,start:0,end:2,etag:"abc"},{part:2,start:2,end:3,etag:"def"}] diff --git a/weed/s3api/s3_constants/s3_action_strings.go b/weed/s3api/s3_constants/s3_action_strings.go index 20e848997..46b3eb8c7 100644 --- a/weed/s3api/s3_constants/s3_action_strings.go +++ b/weed/s3api/s3_constants/s3_action_strings.go @@ -8,7 +8,8 @@ const ( S3_ACTION_PUT_OBJECT = "s3:PutObject" S3_ACTION_DELETE_OBJECT = "s3:DeleteObject" S3_ACTION_DELETE_OBJECT_VERSION = "s3:DeleteObjectVersion" - S3_ACTION_GET_OBJECT_VERSION = "s3:GetObjectVersion" + S3_ACTION_GET_OBJECT_VERSION = "s3:GetObjectVersion" + S3_ACTION_GET_OBJECT_ATTRIBUTES = "s3:GetObjectAttributes" // Object ACL operations S3_ACTION_GET_OBJECT_ACL = "s3:GetObjectAcl" diff --git a/weed/s3api/s3api_object_handlers_attributes.go b/weed/s3api/s3api_object_handlers_attributes.go new file mode 100644 index 000000000..8452b99fd --- /dev/null +++ b/weed/s3api/s3api_object_handlers_attributes.go @@ -0,0 +1,326 @@ +package s3api + +import ( + "encoding/json" + "encoding/xml" + "errors" + "net/http" + "strconv" + "strings" + "time" + + "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" +) + +const maxPartsListDefault = 1000 + +// GetObjectAttributesResponse is the XML response for GetObjectAttributes. +type GetObjectAttributesResponse struct { + XMLName xml.Name `xml:"GetObjectAttributesResponse"` + ETag string `xml:"ETag,omitempty"` + Checksum *ObjectAttributesChecksum `xml:"Checksum,omitempty"` + ObjectParts *ObjectAttributesParts `xml:"ObjectParts,omitempty"` + StorageClass string `xml:"StorageClass,omitempty"` + ObjectSize *int64 `xml:"ObjectSize,omitempty"` +} + +// ObjectAttributesChecksum holds checksum info for GetObjectAttributes. +type ObjectAttributesChecksum struct { + ChecksumCRC32 string `xml:"ChecksumCRC32,omitempty"` + ChecksumCRC32C string `xml:"ChecksumCRC32C,omitempty"` + ChecksumSHA1 string `xml:"ChecksumSHA1,omitempty"` + ChecksumSHA256 string `xml:"ChecksumSHA256,omitempty"` +} + +// ObjectAttributesParts holds parts info for GetObjectAttributes. +type ObjectAttributesParts struct { + IsTruncated bool `xml:"IsTruncated"` + MaxParts int `xml:"MaxParts"` + NextPartNumberMarker int `xml:"NextPartNumberMarker"` + PartNumberMarker int `xml:"PartNumberMarker"` + TotalPartsCount int `xml:"PartsCount"` + Parts []*ObjectAttributesPart `xml:"Part"` +} + +// ObjectAttributesPart holds info for a single part. +type ObjectAttributesPart struct { + PartNumber int `xml:"PartNumber"` + Size int64 `xml:"Size"` +} + +// parseObjectAttributes parses the X-Amz-Object-Attributes header. +func parseObjectAttributes(h http.Header) map[string]struct{} { + attrs := make(map[string]struct{}) + for _, headerVal := range h.Values(s3_constants.AmzObjectAttributes) { + for _, v := range strings.Split(strings.TrimSpace(headerVal), ",") { + v = strings.TrimSpace(v) + if v != "" { + attrs[v] = struct{}{} + } + } + } + return attrs +} + +// validateObjectAttributes checks that all requested attributes are valid. +func validateObjectAttributes(attrs map[string]struct{}) bool { + for attr := range attrs { + switch attr { + case "ETag", "Checksum", "StorageClass", "ObjectSize", "ObjectParts": + default: + return false + } + } + return true +} + +func (s3a *S3ApiServer) GetObjectAttributesHandler(w http.ResponseWriter, r *http.Request) { + bucket, object := s3_constants.GetBucketAndObject(r) + glog.V(3).Infof("GetObjectAttributesHandler %s %s", bucket, object) + + // Parse and validate requested attributes + requestedAttrs := parseObjectAttributes(r.Header) + if len(requestedAttrs) == 0 || !validateObjectAttributes(requestedAttrs) { + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidAttributeName) + return + } + + // Parse optional pagination headers for ObjectParts + maxParts := maxPartsListDefault + if v := r.Header.Get(s3_constants.AmzMaxParts); v != "" { + parsed, err := strconv.Atoi(v) + if err != nil || parsed < 0 { + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidMaxParts) + return + } + maxParts = parsed + if maxParts > maxPartsListDefault { + maxParts = maxPartsListDefault + } + } + partNumberMarker := 0 + if v := r.Header.Get(s3_constants.AmzPartNumberMarker); v != "" { + parsed, err := strconv.Atoi(v) + if err != nil || parsed < 0 { + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidPartNumberMarker) + return + } + partNumberMarker = parsed + } + + // Check for specific version ID + versionId := r.URL.Query().Get("versionId") + + var ( + entry *filer_pb.Entry + err error + ) + + versioningConfigured, err := s3a.isVersioningConfigured(bucket) + if err != nil { + if err == filer_pb.ErrNotFound { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) + return + } + glog.Errorf("Error checking versioning for bucket %s: %v", bucket, err) + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + return + } + + if versioningConfigured { + var targetVersionId string + + if versionId != "" { + entry, err = s3a.getSpecificObjectVersion(bucket, object, versionId) + if err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + return + } + targetVersionId = versionId + } else { + bucketDir := s3a.bucketDir(bucket) + normalizedObject := s3_constants.NormalizeObjectKey(object) + versionsDir := normalizedObject + s3_constants.VersionsFolder + + versionsEntry, versionsErr := s3a.getEntry(bucketDir, versionsDir) + if versionsErr == nil && versionsEntry != nil { + entry, err = s3a.getLatestObjectVersion(bucket, object) + if err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + return + } + } else if errors.Is(versionsErr, filer_pb.ErrNotFound) { + regularEntry, regularErr := s3a.getEntry(bucketDir, normalizedObject) + if regularErr == nil && regularEntry != nil { + entry = regularEntry + targetVersionId = "null" + } else { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + return + } + } else { + entry, err = s3a.getLatestObjectVersion(bucket, object) + if err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + return + } + } + + if targetVersionId == "" { + if entry.Extended != nil { + if versionIdBytes, exists := entry.Extended[s3_constants.ExtVersionIdKey]; exists { + targetVersionId = string(versionIdBytes) + } + } + if targetVersionId == "" { + targetVersionId = "null" + } + } + } + + // Check for delete marker + if entry.Extended != nil { + if deleteMarker, exists := entry.Extended[s3_constants.ExtDeleteMarkerKey]; exists && string(deleteMarker) == "true" { + w.Header().Set(s3_constants.AmzDeleteMarker, "true") + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + return + } + } + + w.Header().Set("x-amz-version-id", targetVersionId) + } else { + entry, err = s3a.fetchObjectEntry(bucket, object) + if err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + return + } + if entry == nil { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + return + } + } + + // Evaluate conditional headers against the resolved entry (after version resolution) + // This ensures conditions are checked against the correct version, not always the latest + if s3a.hasConditionalHeaders(r) { + headers, errCode := parseConditionalHeaders(r) + if errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + result := s3a.validateConditionalHeadersForReads(r, headers, entry, bucket, object) + if result.ErrorCode != s3err.ErrNone { + glog.V(3).Infof("GetObjectAttributesHandler: Conditional header check failed for %s/%s with error %v", bucket, object, result.ErrorCode) + if result.ErrorCode == s3err.ErrNotModified && result.ETag != "" { + w.Header().Set("ETag", result.ETag) + } + s3err.WriteErrorResponse(w, r, result.ErrorCode) + return + } + } + + if entry == nil { + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + return + } + + // Build response with only requested attributes + resp := &GetObjectAttributesResponse{} + + if _, ok := requestedAttrs["ETag"]; ok { + etag := s3a.getObjectETag(entry) + resp.ETag = strings.Trim(etag, `"`) + } + + if _, ok := requestedAttrs["StorageClass"]; ok { + storageClass := "STANDARD" + if entry.Extended != nil { + if sc, exists := entry.Extended[s3_constants.AmzStorageClass]; exists && len(sc) > 0 { + storageClass = string(sc) + } + } + resp.StorageClass = storageClass + } + + // Checksum: accepted in validation so clients don't get a 400, but SeaweedFS + // does not yet store S3 checksums (CRC32, CRC32C, SHA1, SHA256), so + // resp.Checksum is intentionally left nil. When checksum storage is added, + // populate resp.Checksum here. + + if _, ok := requestedAttrs["ObjectSize"]; ok { + var size int64 + if entry.Attributes != nil { + size = int64(entry.Attributes.FileSize) + } + resp.ObjectSize = &size + } + + if _, ok := requestedAttrs["ObjectParts"]; ok { + resp.ObjectParts = s3a.buildObjectAttributesParts(entry, maxParts, partNumberMarker) + } + + // Set Last-Modified, remove Content-Type + if entry.Attributes != nil && entry.Attributes.Mtime > 0 { + w.Header().Set("Last-Modified", time.Unix(entry.Attributes.Mtime, 0).UTC().Format(http.TimeFormat)) + } + w.Header().Del("Content-Type") + + writeSuccessResponseXML(w, r, resp) +} + +// buildObjectAttributesParts builds the ObjectParts section from multipart metadata. +func (s3a *S3ApiServer) buildObjectAttributesParts(entry *filer_pb.Entry, maxParts, partNumberMarker int) *ObjectAttributesParts { + if entry.Extended == nil { + return nil + } + + boundariesJSON, exists := entry.Extended[s3_constants.SeaweedFSMultipartPartBoundaries] + if !exists { + return nil + } + + var boundaries []PartBoundaryInfo + if err := json.Unmarshal(boundariesJSON, &boundaries); err != nil { + glog.Warningf("GetObjectAttributes: failed to unmarshal part boundaries: %v", err) + return nil + } + + if len(boundaries) == 0 { + return nil + } + + parts := &ObjectAttributesParts{ + PartNumberMarker: partNumberMarker, + MaxParts: maxParts, + TotalPartsCount: len(boundaries), + } + + chunks := entry.GetChunks() + for _, b := range boundaries { + if b.PartNumber <= partNumberMarker { + continue + } + if len(parts.Parts) >= maxParts { + parts.IsTruncated = true + break + } + + var partSize int64 + if b.StartChunk >= 0 && b.EndChunk >= 0 && b.StartChunk < len(chunks) && b.EndChunk <= len(chunks) && b.StartChunk < b.EndChunk { + for ci := b.StartChunk; ci < b.EndChunk; ci++ { + partSize += int64(chunks[ci].Size) + } + } + + parts.NextPartNumberMarker = b.PartNumber + parts.Parts = append(parts.Parts, &ObjectAttributesPart{ + PartNumber: b.PartNumber, + Size: partSize, + }) + } + + return parts +} diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 7ac8b04e2..4b4772e2e 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -1658,7 +1658,11 @@ func (s3a *S3ApiServer) validateConditionalHeaders(r *http.Request, headers cond objectExists := entry != nil // For PUT requests, all specified conditions must be met. - // The evaluation order follows AWS S3 behavior for consistency. + // The evaluation order follows RFC 7232 Section 6 and AWS S3 behavior: + // 1. If-Match + // 2. If-Unmodified-Since (ignored when If-Match is present, per RFC 7232 Section 3.4) + // 3. If-None-Match + // 4. If-Modified-Since (ignored when If-None-Match is present, per RFC 7232 Section 3.3) // 1. Check If-Match if headers.ifMatch != "" { @@ -1679,7 +1683,9 @@ func (s3a *S3ApiServer) validateConditionalHeaders(r *http.Request, headers cond } // 2. Check If-Unmodified-Since - if !headers.ifUnmodifiedSince.IsZero() { + // Per RFC 7232 Section 3.4: "A recipient MUST ignore If-Unmodified-Since + // if the request contains an If-Match header field" + if !headers.ifUnmodifiedSince.IsZero() && headers.ifMatch == "" { if objectExists { if entry.Attributes != nil { objectModTime := time.Unix(entry.Attributes.Mtime, 0) @@ -1713,7 +1719,9 @@ func (s3a *S3ApiServer) validateConditionalHeaders(r *http.Request, headers cond } // 4. Check If-Modified-Since - if !headers.ifModifiedSince.IsZero() { + // Per RFC 7232 Section 3.3: "A recipient MUST ignore If-Modified-Since + // if the request contains an If-None-Match header field" + if !headers.ifModifiedSince.IsZero() && headers.ifNoneMatch == "" { if objectExists { if entry.Attributes != nil { objectModTime := time.Unix(entry.Attributes.Mtime, 0) @@ -1802,7 +1810,11 @@ func (s3a *S3ApiServer) validateConditionalHeadersForReads(r *http.Request, head } // Object exists - check all conditions - // The evaluation order follows AWS S3 behavior for consistency. + // The evaluation order follows RFC 7232 Section 6 and AWS S3 behavior: + // 1. If-Match + // 2. If-Unmodified-Since (ignored when If-Match is present, per RFC 7232 Section 3.4) + // 3. If-None-Match + // 4. If-Modified-Since (ignored when If-None-Match is present, per RFC 7232 Section 3.3) // 1. Check If-Match (412 Precondition Failed if fails) if headers.ifMatch != "" { @@ -1821,7 +1833,9 @@ func (s3a *S3ApiServer) validateConditionalHeadersForReads(r *http.Request, head } // 2. Check If-Unmodified-Since (412 Precondition Failed if fails) - if !headers.ifUnmodifiedSince.IsZero() { + // Per RFC 7232 Section 3.4: "A recipient MUST ignore If-Unmodified-Since + // if the request contains an If-Match header field" + if !headers.ifUnmodifiedSince.IsZero() && headers.ifMatch == "" { objectModTime := time.Unix(entry.Attributes.Mtime, 0) if objectModTime.After(headers.ifUnmodifiedSince) { glog.V(3).Infof("validateConditionalHeadersForReads: If-Unmodified-Since failed - object modified after %s", r.Header.Get(s3_constants.IfUnmodifiedSince)) @@ -1848,7 +1862,9 @@ func (s3a *S3ApiServer) validateConditionalHeadersForReads(r *http.Request, head } // 4. Check If-Modified-Since (304 Not Modified if fails) - if !headers.ifModifiedSince.IsZero() { + // Per RFC 7232 Section 3.3: "A recipient MUST ignore If-Modified-Since + // if the request contains an If-None-Match header field" + if !headers.ifModifiedSince.IsZero() && headers.ifNoneMatch == "" { objectModTime := time.Unix(entry.Attributes.Mtime, 0) if !objectModTime.After(headers.ifModifiedSince) { // Use production getObjectETag method diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index c2d25b4f8..c88a1ea0b 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -605,6 +605,8 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // ListMultipartUploads bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.ListMultipartUploadsHandler, ACTION_READ)), "GET")).Queries("uploads", "") + // GetObjectAttributes + bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectAttributesHandler, ACTION_READ)), "GET")).Queries("attributes", "") // GetObjectTagging bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectTaggingHandler, ACTION_READ)), "GET")).Queries("tagging", "") // PutObjectTagging diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go index 215d90262..309f543fb 100644 --- a/weed/s3api/s3err/s3api_errors.go +++ b/weed/s3api/s3err/s3api_errors.go @@ -143,6 +143,8 @@ const ( // Bucket encryption errors ErrNoSuchBucketEncryptionConfiguration ErrInvalidStorageClass + + ErrInvalidAttributeName ) // Error message constants for checksum validation @@ -600,6 +602,12 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The storage class you specified is not valid", HTTPStatusCode: http.StatusBadRequest, }, + + ErrInvalidAttributeName: { + Code: "InvalidArgument", + Description: "Invalid attribute name specified", + HTTPStatusCode: http.StatusBadRequest, + }, } // GetAPIError provides API Error for input API error code.