From 60ee03e3c1c51358eab3cce2e2c9d15ec3152d13 Mon Sep 17 00:00:00 2001 From: chrislu Date: Wed, 29 Oct 2025 11:58:53 -0700 Subject: [PATCH] add more checks --- weed/s3api/s3api_conditional_headers_test.go | 76 ++++++++++++++++++++ weed/s3api/s3api_object_handlers_put.go | 3 +- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/weed/s3api/s3api_conditional_headers_test.go b/weed/s3api/s3api_conditional_headers_test.go index 9a810c15e..5da78e13f 100644 --- a/weed/s3api/s3api_conditional_headers_test.go +++ b/weed/s3api/s3api_conditional_headers_test.go @@ -671,6 +671,82 @@ 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) + md5Bytes := []byte{0x66, 0x37, 0x26, 0x33, 0x0a, 0xc2, 0x54, 0x63, 0x55, 0x81, 0xbe, 0x07, 0xa2, 0xa1, 0xde, 0xf6} + + 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 := "\"663726330ac254635581be07a2a1def6\"" + 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, "663726330ac254635581be07a2a1def6") + + 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 e56e23780..26d74b239 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -1085,7 +1085,8 @@ func (s3a *S3ApiServer) getObjectETag(entry *filer_pb.Entry) string { if etagBytes, hasETag := entry.Extended[s3_constants.ExtETagKey]; hasETag { return string(etagBytes) } - if entry.Attributes.Md5 != nil { + // Check for Md5 in Attributes (matches filer.ETag behavior) + if entry.Attributes != nil && entry.Attributes.Md5 != nil && len(entry.Attributes.Md5) > 0 { return fmt.Sprintf("\"%x\"", entry.Attributes.Md5) } // Fallback: calculate ETag from chunks