tag parsing decode url encoded

fix https://github.com/seaweedfs/seaweedfs/issues/7040
2025-09-19 17:07:57 +08:00 · 2025-07-28 02:49:43 -07:00
parent a4df110e77
commit 124c4281a8
3 changed files with 175 additions and 102 deletions
--- 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"),
+	})
+}
--- 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
--- 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 := `<?xml version="1.0" encoding="UTF-8"?>
-<Tagging xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
-   <TagSet>
-      <Tag>
-         <Key>key1</Key>
-         <Value>value1</Value>
-      </Tag>
-   </TagSet>
-</Tagging>
-`
-
-	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))
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := parseTagsHeader(tt.input)

-	expected := `<?xml version="1.0" encoding="UTF-8"?>
-<Tagging xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><TagSet><Tag><Key>key1</Key><Value>value1</Value></Tag></TagSet></Tagging>`
-	assert.Equal(t, expected, actual)
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("Expected error but got none")
+				}
+				return
+			}

-}
+			if err != nil {
+				t.Errorf("Unexpected error: %v", err)
+				return
+			}

-type TestTags map[string]string
+			if len(result) != len(tt.expected) {
+				t.Errorf("Expected %d tags, got %d", len(tt.expected), len(result))
+				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",
-	},
-}
-
-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])
+				}
+			}
+		})
 	}
 }