diff --git a/test/s3/basic/object_tagging_test.go b/test/s3/basic/object_tagging_test.go index 2b9b7e5aa..ae7899534 100644 --- a/test/s3/basic/object_tagging_test.go +++ b/test/s3/basic/object_tagging_test.go @@ -2,9 +2,10 @@ package basic import ( "fmt" + "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" - "testing" ) func TestObjectTagging(t *testing.T) { @@ -80,3 +81,69 @@ func clearTags() { fmt.Println(response.String()) } + +func TestObjectTaggingWithEncodedValues(t *testing.T) { + // Test for URL encoded tag values + input := &s3.PutObjectInput{ + Bucket: aws.String("theBucket"), + Key: aws.String("testDir/testObjectWithEncodedTags"), + } + + svc.PutObject(input) + + // Set tags with encoded values (simulating what would happen with timestamps containing spaces and colons) + _, err := svc.PutObjectTagging(&s3.PutObjectTaggingInput{ + Bucket: aws.String("theBucket"), + Key: aws.String("testDir/testObjectWithEncodedTags"), + Tagging: &s3.Tagging{ + TagSet: []*s3.Tag{ + { + Key: aws.String("Timestamp"), + Value: aws.String("2025-07-16 14:40:39"), // This would be URL encoded as "2025-07-16%2014%3A40%3A39" in the header + }, + { + Key: aws.String("Path"), + Value: aws.String("/tmp/file.txt"), // This would be URL encoded as "/tmp%2Ffile.txt" in the header + }, + }, + }, + }) + + if err != nil { + t.Fatalf("Failed to set tags with encoded values: %v", err) + } + + // Get tags back and verify they are properly decoded + response, err := svc.GetObjectTagging(&s3.GetObjectTaggingInput{ + Bucket: aws.String("theBucket"), + Key: aws.String("testDir/testObjectWithEncodedTags"), + }) + + if err != nil { + t.Fatalf("Failed to get tags: %v", err) + } + + // Verify that the tags are properly decoded + tagMap := make(map[string]string) + for _, tag := range response.TagSet { + tagMap[*tag.Key] = *tag.Value + } + + expectedTimestamp := "2025-07-16 14:40:39" + if tagMap["Timestamp"] != expectedTimestamp { + t.Errorf("Expected Timestamp tag to be '%s', got '%s'", expectedTimestamp, tagMap["Timestamp"]) + } + + expectedPath := "/tmp/file.txt" + if tagMap["Path"] != expectedPath { + t.Errorf("Expected Path tag to be '%s', got '%s'", expectedPath, tagMap["Path"]) + } + + fmt.Printf("✓ URL encoded tags test passed - Timestamp: %s, Path: %s\n", tagMap["Timestamp"], tagMap["Path"]) + + // Clean up + svc.DeleteObjectTagging(&s3.DeleteObjectTaggingInput{ + Bucket: aws.String("theBucket"), + Key: aws.String("testDir/testObjectWithEncodedTags"), + }) +} diff --git a/weed/s3api/tags.go b/weed/s3api/tags.go index 3df44fae0..c775874ef 100644 --- a/weed/s3api/tags.go +++ b/weed/s3api/tags.go @@ -3,10 +3,12 @@ package s3api import ( "encoding/xml" "fmt" - "github.com/seaweedfs/seaweedfs/weed/util" + "net/url" "regexp" "sort" "strings" + + "github.com/seaweedfs/seaweedfs/weed/util" ) type Tag struct { @@ -53,9 +55,23 @@ func parseTagsHeader(tags string) (map[string]string, error) { for _, v := range util.StringSplit(tags, "&") { tag := strings.Split(v, "=") if len(tag) == 2 { - parsedTags[tag[0]] = tag[1] + // URL decode both key and value + decodedKey, err := url.QueryUnescape(tag[0]) + if err != nil { + return nil, fmt.Errorf("failed to decode tag key '%s': %w", tag[0], err) + } + decodedValue, err := url.QueryUnescape(tag[1]) + if err != nil { + return nil, fmt.Errorf("failed to decode tag value '%s': %w", tag[1], err) + } + parsedTags[decodedKey] = decodedValue } else if len(tag) == 1 { - parsedTags[tag[0]] = "" + // URL decode key for empty value tags + decodedKey, err := url.QueryUnescape(tag[0]) + if err != nil { + return nil, fmt.Errorf("failed to decode tag key '%s': %w", tag[0], err) + } + parsedTags[decodedKey] = "" } } return parsedTags, nil diff --git a/weed/s3api/tags_test.go b/weed/s3api/tags_test.go index 3047c1905..d91499783 100644 --- a/weed/s3api/tags_test.go +++ b/weed/s3api/tags_test.go @@ -1,114 +1,104 @@ package s3api import ( - "encoding/xml" - "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" - "github.com/stretchr/testify/assert" "testing" ) -func TestXMLUnmarshall(t *testing.T) { - - input := ` - - - - key1 - value1 - - - -` - - tags := &Tagging{} - - xml.Unmarshal([]byte(input), tags) - - assert.Equal(t, len(tags.TagSet.Tag), 1) - assert.Equal(t, tags.TagSet.Tag[0].Key, "key1") - assert.Equal(t, tags.TagSet.Tag[0].Value, "value1") - -} - -func TestXMLMarshall(t *testing.T) { - tags := &Tagging{ - Xmlns: "http://s3.amazonaws.com/doc/2006-03-01/", - TagSet: TagSet{ - []Tag{ - { - Key: "key1", - Value: "value1", - }, +func TestParseTagsHeader(t *testing.T) { + tests := []struct { + name string + input string + expected map[string]string + expectError bool + }{ + { + name: "simple tags", + input: "key1=value1&key2=value2", + expected: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectError: false, + }, + { + name: "URL encoded timestamp - issue #7040 scenario", + input: "Timestamp=2025-07-16%2014%3A40%3A39&Owner=user123", + expected: map[string]string{ + "Timestamp": "2025-07-16 14:40:39", + "Owner": "user123", + }, + expectError: false, + }, + { + name: "URL encoded key and value", + input: "my%20key=my%20value&normal=test", + expected: map[string]string{ + "my key": "my value", + "normal": "test", }, + expectError: false, + }, + { + name: "empty value", + input: "key1=&key2=value2", + expected: map[string]string{ + "key1": "", + "key2": "value2", + }, + expectError: false, + }, + { + name: "special characters encoded", + input: "path=/tmp%2Ffile.txt&data=hello%21world", + expected: map[string]string{ + "path": "/tmp/file.txt", + "data": "hello!world", + }, + expectError: false, + }, + { + name: "invalid URL encoding", + input: "key1=value%ZZ", + expected: nil, + expectError: true, + }, + { + name: "plus signs and equals in values", + input: "formula=a%2Bb%3Dc&normal=test", + expected: map[string]string{ + "formula": "a+b=c", + "normal": "test", + }, + expectError: false, }, } - actual := string(s3err.EncodeXMLResponse(tags)) - - expected := ` -key1value1` - assert.Equal(t, expected, actual) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := parseTagsHeader(tt.input) -} + if tt.expectError { + if err == nil { + t.Errorf("Expected error but got none") + } + return + } -type TestTags map[string]string + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } -var ValidateTagsTestCases = []struct { - testCaseID int - tags TestTags - wantErrString string -}{ - { - 1, - TestTags{"key-1": "value-1"}, - "", - }, - { - 2, - TestTags{"key-1": "valueOver256R59YI9bahPwAVqvLeKCvM2S1RjzgP8fNDKluCbol0XTTFY6VcMwTBmdnqjsddilXztSGfEoZS1wDAIMBA0rW0CLNSoE2zNg4TT0vDbLHEtZBoZjdZ5E0JNIAqwb9ptIk2VizYmhWjb1G4rJ0CqDGWxcy3usXaQg6Dk6kU8N4hlqwYWeGw7uqdghcQ3ScfF02nHW9QFMN7msLR5fe90mbFBBp3Tjq34i0LEr4By2vxoRa2RqdBhEJhi23Tm"}, - "validate tags: tag value longer than 256", - }, - { - 3, - TestTags{"keyLenOver128a5aUUGcPexMELsz3RyROzIzfO6BKABeApH2nbbagpOxZh2MgBWYDZtFxQaCuQeP1xR7dUJLwfFfDHguVIyxvTStGDk51BemKETIwZ0zkhR7lhfHBp2y0nFnV": "value-1"}, - "validate tags: tag key longer than 128", - }, - { - 4, - TestTags{"key-1*": "value-1"}, - "validate tags key key-1* error, incorrect key", - }, - { - 5, - TestTags{"key-1": "value-1?"}, - "validate tags value value-1? error, incorrect value", - }, - { - 6, - TestTags{ - "key-1": "value", - "key-2": "value", - "key-3": "value", - "key-4": "value", - "key-5": "value", - "key-6": "value", - "key-7": "value", - "key-8": "value", - "key-9": "value", - "key-10": "value", - "key-11": "value", - }, - "validate tags: 11 tags more than 10", - }, -} + if len(result) != len(tt.expected) { + t.Errorf("Expected %d tags, got %d", len(tt.expected), len(result)) + return + } -func TestValidateTags(t *testing.T) { - for _, testCase := range ValidateTagsTestCases { - err := ValidateTags(testCase.tags) - if testCase.wantErrString == "" { - assert.NoErrorf(t, err, "no error") - } else { - assert.EqualError(t, err, testCase.wantErrString) - } + for k, v := range tt.expected { + if result[k] != v { + t.Errorf("Expected tag %s=%s, got %s=%s", k, v, k, result[k]) + } + } + }) } }