diff options
| -rw-r--r-- | .github/workflows/test-s3-over-https-using-awscli.yml | 9 | ||||
| -rw-r--r-- | weed/s3api/s3api_conditional_headers_test.go | 81 | ||||
| -rw-r--r-- | weed/s3api/s3api_object_handlers_put.go | 5 |
3 files changed, 95 insertions, 0 deletions
diff --git a/.github/workflows/test-s3-over-https-using-awscli.yml b/.github/workflows/test-s3-over-https-using-awscli.yml index f09d1c1aa..ff2e433f0 100644 --- a/.github/workflows/test-s3-over-https-using-awscli.yml +++ b/.github/workflows/test-s3-over-https-using-awscli.yml @@ -77,3 +77,12 @@ jobs: aws --no-verify-ssl s3 cp --no-progress s3://bucket/test-multipart downloaded diff -q generated downloaded rm -f generated downloaded + + - name: Test GetObject with If-Match + run: | + set -e + dd if=/dev/urandom of=generated bs=1M count=32 + ETAG=$(aws --no-verify-ssl s3api put-object --bucket bucket --key test-get-obj --body generated | jq -r .ETag) + aws --no-verify-ssl s3api get-object --bucket bucket --key test-get-obj --if-match ${ETAG:1:32} downloaded + diff -q generated downloaded + rm -f generated downloaded diff --git a/weed/s3api/s3api_conditional_headers_test.go b/weed/s3api/s3api_conditional_headers_test.go index 9a810c15e..834f57305 100644 --- a/weed/s3api/s3api_conditional_headers_test.go +++ b/weed/s3api/s3api_conditional_headers_test.go @@ -2,6 +2,7 @@ package s3api import ( "bytes" + "encoding/hex" "fmt" "net/http" "net/url" @@ -671,6 +672,86 @@ func TestETagMatching(t *testing.T) { } } +// TestGetObjectETagWithMd5AndChunks tests the fix for issue #7274 +// When an object has both Attributes.Md5 and multiple chunks, getObjectETag should +// prefer Attributes.Md5 to match the behavior of HeadObject and filer.ETag +func TestGetObjectETagWithMd5AndChunks(t *testing.T) { + s3a := NewS3ApiServerForTest() + if s3a == nil { + t.Skip("S3ApiServer not available for testing") + } + + // Create an object with both Md5 and multiple chunks (like in issue #7274) + // Md5: ZjcmMwrCVGNVgb4HoqHe9g== (base64) = 663726330ac254635581be07a2a1def6 (hex) + md5HexString := "663726330ac254635581be07a2a1def6" + md5Bytes, err := hex.DecodeString(md5HexString) + if err != nil { + t.Fatalf("failed to decode md5 hex string: %v", err) + } + + entry := &filer_pb.Entry{ + Name: "test-multipart-object", + Attributes: &filer_pb.FuseAttributes{ + Mtime: time.Now().Unix(), + FileSize: 5597744, + Md5: md5Bytes, + }, + // Two chunks - if we only used ETagChunks, it would return format "hash-2" + Chunks: []*filer_pb.FileChunk{ + { + FileId: "chunk1", + Offset: 0, + Size: 4194304, + ETag: "9+yCD2DGwMG5uKwAd+y04Q==", + }, + { + FileId: "chunk2", + Offset: 4194304, + Size: 1403440, + ETag: "cs6SVSTgZ8W3IbIrAKmklg==", + }, + }, + } + + // getObjectETag should return the Md5 in hex with quotes + expectedETag := "\"" + md5HexString + "\"" + actualETag := s3a.getObjectETag(entry) + + if actualETag != expectedETag { + t.Errorf("Expected ETag %s, got %s", expectedETag, actualETag) + } + + // Now test that conditional headers work with this ETag + bucket := "test-bucket" + object := "/test-object" + + // Test If-Match with the Md5-based ETag (should succeed) + t.Run("IfMatch_WithMd5BasedETag_ShouldSucceed", func(t *testing.T) { + getter := createMockEntryGetter(entry) + req := createTestGetRequest(bucket, object) + // Client sends the ETag from HeadObject (without quotes) + req.Header.Set(s3_constants.IfMatch, md5HexString) + + result := s3a.checkConditionalHeadersForReadsWithGetter(getter, req, bucket, object) + if result.ErrorCode != s3err.ErrNone { + t.Errorf("Expected ErrNone when If-Match uses Md5-based ETag, got %v (ETag was %s)", result.ErrorCode, actualETag) + } + }) + + // Test If-Match with chunk-based ETag format (should fail - this was the old incorrect behavior) + t.Run("IfMatch_WithChunkBasedETag_ShouldFail", func(t *testing.T) { + getter := createMockEntryGetter(entry) + req := createTestGetRequest(bucket, object) + // If we incorrectly calculated ETag from chunks, it would be in format "hash-2" + req.Header.Set(s3_constants.IfMatch, "123294de680f28bde364b81477549f7d-2") + + result := s3a.checkConditionalHeadersForReadsWithGetter(getter, req, bucket, object) + if result.ErrorCode != s3err.ErrPreconditionFailed { + t.Errorf("Expected ErrPreconditionFailed when If-Match uses chunk-based ETag format, got %v", result.ErrorCode) + } + }) +} + // TestConditionalHeadersIntegration tests conditional headers with full integration func TestConditionalHeadersIntegration(t *testing.T) { // This would be a full integration test that requires a running SeaweedFS instance diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index fb7d6c3a6..1fff23545 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -1257,6 +1257,11 @@ func (s3a *S3ApiServer) getObjectETag(entry *filer_pb.Entry) string { if etagBytes, hasETag := entry.Extended[s3_constants.ExtETagKey]; hasETag { return string(etagBytes) } + // Check for Md5 in Attributes (matches filer.ETag behavior) + // Note: len(nil slice) == 0 in Go, so no need for explicit nil check + if entry.Attributes != nil && len(entry.Attributes.Md5) > 0 { + return fmt.Sprintf("\"%x\"", entry.Attributes.Md5) + } // Fallback: calculate ETag from chunks return s3a.calculateETagFromChunks(entry.Chunks) } |