Browse Source

s3: fix if-match error (#7277)

* s3: fix if-match error

* add more checks

* minor

* minor

---------

Co-authored-by: chrislu <chris.lu@gmail.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
pull/7236/merge
Taehyung Lim 2 days ago
committed by GitHub
parent
commit
d4790cb8e6
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 9
      .github/workflows/test-s3-over-https-using-awscli.yml
  2. 81
      weed/s3api/s3api_conditional_headers_test.go
  3. 5
      weed/s3api/s3api_object_handlers_put.go

9
.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

81
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

5
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)
}

Loading…
Cancel
Save