diff --git a/weed/s3api/s3_metadata_util.go b/weed/s3api/s3_metadata_util.go
index bcd62b0e9d..01e0adc194 100644
--- a/weed/s3api/s3_metadata_util.go
+++ b/weed/s3api/s3_metadata_util.go
@@ -2,7 +2,6 @@ package s3api
import (
"net/http"
- "net/url"
"strings"
"github.com/seaweedfs/seaweedfs/weed/glog"
@@ -52,25 +51,16 @@ func ParseS3Metadata(r *http.Request, existing map[string][]byte, isReplace bool
// Object tagging
if tags := r.Header.Get(s3_constants.AmzObjectTagging); tags != "" {
- // Use url.ParseQuery for robust parsing and automatic URL decoding
- parsedTags, err := url.ParseQuery(tags)
+ tagSet, err := parseTagsHeader(tags)
if err != nil {
- // Return proper S3 error instead of silently dropping tags
- glog.Warningf("Invalid S3 tag format in header '%s': %v", tags, err)
+ glog.Warningf("Invalid S3 tagging header: %v", err)
return nil, s3err.ErrInvalidTag
}
-
- // Validate: S3 spec does not allow duplicate tag keys
- for key, values := range parsedTags {
- if len(values) > 1 {
- glog.Warningf("Duplicate tag key '%s' in header '%s'", key, tags)
- return nil, s3err.ErrInvalidTag
- }
- // Tag value can be an empty string but not nil
- value := ""
- if len(values) > 0 {
- value = values[0]
- }
+ if err := ValidateTags(tagSet); err != nil {
+ glog.Warningf("Invalid tags (%d): %v", len(tagSet), err)
+ return nil, s3err.ErrInvalidTag
+ }
+ for key, value := range tagSet {
metadata[s3_constants.AmzObjectTagging+"-"+key] = []byte(value)
}
}
diff --git a/weed/s3api/s3api_bucket_tagging_handlers.go b/weed/s3api/s3api_bucket_tagging_handlers.go
index a1b116fd28..9541f38cb1 100644
--- a/weed/s3api/s3api_bucket_tagging_handlers.go
+++ b/weed/s3api/s3api_bucket_tagging_handlers.go
@@ -68,9 +68,14 @@ func (s3a *S3ApiServer) PutBucketTaggingHandler(w http.ResponseWriter, r *http.R
}
tags := tagging.ToTags()
+ // A key may appear only once in a tag set; the map keeps one of them.
+ if len(tags) != len(tagging.TagSet.Tag) {
+ glog.Errorf("PutBucketTagging duplicate tag keys %s", r.URL)
+ s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag)
+ return
+ }
- // Validate tags using existing validation
- err = ValidateTags(tags)
+ err = ValidateBucketTags(tags)
if err != nil {
glog.Errorf("PutBucketTagging ValidateTags error %s: %v", r.URL, err)
s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag)
diff --git a/weed/s3api/s3api_bucket_tagging_handlers_test.go b/weed/s3api/s3api_bucket_tagging_handlers_test.go
new file mode 100644
index 0000000000..a1cc26b165
--- /dev/null
+++ b/weed/s3api/s3api_bucket_tagging_handlers_test.go
@@ -0,0 +1,91 @@
+package s3api
+
+import (
+ "fmt"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/gorilla/mux"
+ "github.com/seaweedfs/seaweedfs/weed/pb/s3_pb"
+ "google.golang.org/protobuf/proto"
+)
+
+// A bucket's tag set holds up to 50 tags, unlike an object's 10, and no key
+// twice.
+func TestPutBucketTaggingTagCountLimit(t *testing.T) {
+ const bucket = "tag-limit-bucket"
+ for _, tc := range []struct {
+ tags int
+ repeatKey bool // the last tag repeats the first tag's key
+ emptyKey bool // the first tag has an empty key
+ wantCode int
+ }{
+ {tags: 10, wantCode: http.StatusOK},
+ {tags: 11, wantCode: http.StatusOK},
+ {tags: 50, wantCode: http.StatusOK},
+ {tags: 51, wantCode: http.StatusBadRequest},
+ {tags: 2, repeatKey: true, wantCode: http.StatusBadRequest},
+ {tags: 51, repeatKey: true, wantCode: http.StatusBadRequest},
+ {tags: 1, emptyKey: true, wantCode: http.StatusBadRequest},
+ } {
+ name := fmt.Sprintf("%d tags", tc.tags)
+ if tc.repeatKey {
+ name += " with a repeated key"
+ }
+ if tc.emptyKey {
+ name += " with an empty key"
+ }
+ t.Run(name, func(t *testing.T) {
+ filer := &fakeTxnFiler{}
+ s3a := newHeadBucketTestServer(t, filer)
+ s3a.iam = &IdentityAccessManagement{isAuthEnabled: true}
+ s3a.bucketConfigCache = NewBucketConfigCache(time.Minute)
+ s3a.bucketConfigCache.Set(bucket, &BucketConfig{Name: bucket})
+
+ var body strings.Builder
+ body.WriteString("")
+ for i := 0; i < tc.tags; i++ {
+ key := fmt.Sprintf("key%d", i)
+ if tc.repeatKey && i == tc.tags-1 {
+ key = "key0"
+ }
+ if tc.emptyKey && i == 0 {
+ key = ""
+ }
+ fmt.Fprintf(&body, "%svalue%d", key, i)
+ }
+ body.WriteString("")
+
+ req := httptest.NewRequest(http.MethodPut, "/"+bucket+"?tagging", strings.NewReader(body.String()))
+ req = mux.SetURLVars(req, map[string]string{"bucket": bucket})
+ rr := httptest.NewRecorder()
+ s3a.PutBucketTaggingHandler(rr, req)
+
+ if rr.Code != tc.wantCode {
+ t.Fatalf("status = %d, want %d; body: %s", rr.Code, tc.wantCode, rr.Body.String())
+ }
+ if tc.wantCode != http.StatusOK {
+ if !strings.Contains(rr.Body.String(), "InvalidTag") {
+ t.Fatalf("body = %s, want InvalidTag", rr.Body.String())
+ }
+ if filer.lastReq != nil {
+ t.Fatalf("rejected tag set was written to the filer")
+ }
+ return
+ }
+ if filer.lastReq == nil || len(filer.lastReq.Mutations) != 1 {
+ t.Fatalf("expected one bucket metadata write, got %+v", filer.lastReq)
+ }
+ var stored s3_pb.BucketMetadata
+ if err := proto.Unmarshal(filer.lastReq.Mutations[0].Content, &stored); err != nil {
+ t.Fatalf("decode stored bucket metadata: %v", err)
+ }
+ if len(stored.Tags) != tc.tags {
+ t.Fatalf("stored %d tags, want %d", len(stored.Tags), tc.tags)
+ }
+ })
+ }
+}
diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go
index b38a205f16..aa6682712a 100644
--- a/weed/s3api/s3api_object_handlers_put.go
+++ b/weed/s3api/s3api_object_handlers_put.go
@@ -527,6 +527,22 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader
if !s3_constants.IsValidBucketName(bucket) || (object != "" && !s3_constants.IsValidObjectKey(object)) {
return "", s3err.ErrInvalidRequest, SSEResponseMetadata{}
}
+
+ // Reject invalid tags before any chunks are uploaded so a failed request
+ // leaves nothing behind on the volume servers.
+ var tagSet map[string]string
+ if tagging := r.Header.Get(s3_constants.AmzObjectTagging); tagging != "" {
+ parsed, err := parseTagsHeader(tagging)
+ if err != nil {
+ glog.Warningf("putToFiler: invalid tagging header for %s: %v", filePath, err)
+ return "", s3err.ErrInvalidTag, SSEResponseMetadata{}
+ }
+ if err := ValidateTags(parsed); err != nil {
+ glog.Warningf("putToFiler: invalid tags (%d) for %s: %v", len(parsed), filePath, err)
+ return "", s3err.ErrInvalidTag, SSEResponseMetadata{}
+ }
+ tagSet = parsed
+ }
// NEW OPTIMIZATION: Write directly to volume servers, bypassing filer proxy
// This eliminates the filer proxy overhead for PUT operations
// Note: filePath is now passed directly instead of URL (no parsing needed)
@@ -938,26 +954,12 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader
entry.Extended[s3_constants.AmzStorageClass] = []byte(sc)
}
- // Parse and store object tags from X-Amz-Tagging header
- // Fix for GitHub issue #7589: Tags sent during object upload were not being stored
- if tagging := r.Header.Get(s3_constants.AmzObjectTagging); tagging != "" {
- parsedTags, err := url.ParseQuery(tagging)
- if err != nil {
- glog.Warningf("putToFiler: Invalid S3 tag format in header '%s': %v", tagging, err)
- return "", s3err.ErrInvalidTag, SSEResponseMetadata{}
- }
- for key, values := range parsedTags {
- if len(values) > 1 {
- glog.Warningf("putToFiler: Duplicate tag key '%s' in header", key)
- return "", s3err.ErrInvalidTag, SSEResponseMetadata{}
- }
- value := ""
- if len(values) > 0 {
- value = values[0]
- }
- entry.Extended[s3_constants.AmzObjectTagging+"-"+key] = []byte(value)
- }
- glog.V(3).Infof("putToFiler: stored %d tags from X-Amz-Tagging header", len(parsedTags))
+ // Store the tags validated before the chunk upload
+ for key, value := range tagSet {
+ entry.Extended[s3_constants.AmzObjectTagging+"-"+key] = []byte(value)
+ }
+ if len(tagSet) > 0 {
+ glog.V(3).Infof("putToFiler: stored %d tags from X-Amz-Tagging header", len(tagSet))
}
storeSSEMetadata(entry, sseResult)
diff --git a/weed/s3api/s3api_object_handlers_tagging.go b/weed/s3api/s3api_object_handlers_tagging.go
index 466d8bcfd4..c55b38c09c 100644
--- a/weed/s3api/s3api_object_handlers_tagging.go
+++ b/weed/s3api/s3api_object_handlers_tagging.go
@@ -139,6 +139,12 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R
return
}
tags := tagging.ToTags()
+ // A key may appear only once in a tag set; the map keeps one of them.
+ if len(tags) != len(tagging.TagSet.Tag) {
+ glog.Errorf("PutObjectTaggingHandler duplicate tag keys %s", r.URL)
+ s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag)
+ return
+ }
err = ValidateTags(tags)
if err != nil {
glog.Errorf("PutObjectTaggingHandler ValidateTags error %s: %v", r.URL, err)
diff --git a/weed/s3api/s3api_object_handlers_tagging_test.go b/weed/s3api/s3api_object_handlers_tagging_test.go
new file mode 100644
index 0000000000..05dec5a833
--- /dev/null
+++ b/weed/s3api/s3api_object_handlers_tagging_test.go
@@ -0,0 +1,29 @@
+package s3api
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "github.com/gorilla/mux"
+)
+
+func TestPutObjectTaggingRejectsDuplicateKeys(t *testing.T) {
+ s3a := newHeadBucketTestServer(t, &fakeTxnFiler{})
+ body := "" +
+ "a1" +
+ "a2" +
+ ""
+ req := httptest.NewRequest(http.MethodPut, "/b/o?tagging", strings.NewReader(body))
+ req = mux.SetURLVars(req, map[string]string{"bucket": "b", "object": "o"})
+ rr := httptest.NewRecorder()
+ s3a.PutObjectTaggingHandler(rr, req)
+
+ if rr.Code != http.StatusBadRequest {
+ t.Fatalf("status = %d, want %d; body: %s", rr.Code, http.StatusBadRequest, rr.Body.String())
+ }
+ if !strings.Contains(rr.Body.String(), "InvalidTag") {
+ t.Fatalf("body = %s, want InvalidTag", rr.Body.String())
+ }
+}
diff --git a/weed/s3api/s3tables/utils.go b/weed/s3api/s3tables/utils.go
index d83e498830..8c76b189af 100644
--- a/weed/s3api/s3tables/utils.go
+++ b/weed/s3api/s3tables/utils.go
@@ -9,6 +9,7 @@ import (
"regexp"
"strings"
"time"
+ "unicode/utf8"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
)
@@ -372,17 +373,25 @@ func arnPartitionForRegion(region string) string {
// ValidateTags validates tags for S3 Tables.
func ValidateTags(tags map[string]string) error {
- if len(tags) > 10 {
- return fmt.Errorf("validate tags: %d tags more than 10", len(tags))
+ return ValidateTagsWithLimit(tags, 10)
+}
+
+// ValidateTagsWithLimit validates a tag set that may hold up to maxTags tags.
+func ValidateTagsWithLimit(tags map[string]string, maxTags int) error {
+ if len(tags) > maxTags {
+ return fmt.Errorf("validate tags: %d tags more than %d", len(tags), maxTags)
}
for k, v := range tags {
- if len(k) > 128 {
+ if len(k) == 0 {
+ return fmt.Errorf("validate tags: empty tag key")
+ }
+ if utf8.RuneCountInString(k) > 128 {
return fmt.Errorf("validate tags: tag key longer than 128")
}
if !tagPattern.MatchString(k) {
return fmt.Errorf("validate tags key %s error, incorrect key", k)
}
- if len(v) > 256 {
+ if utf8.RuneCountInString(v) > 256 {
return fmt.Errorf("validate tags: tag value longer than 256")
}
if !tagPattern.MatchString(v) {
diff --git a/weed/s3api/tags.go b/weed/s3api/tags.go
index 76830ccf14..49cde79f59 100644
--- a/weed/s3api/tags.go
+++ b/weed/s3api/tags.go
@@ -63,6 +63,9 @@ func parseTagsHeader(tags string) (map[string]string, error) {
if err != nil {
return nil, fmt.Errorf("failed to decode tag key '%s': %w", key, err)
}
+ if _, exists := parsedTags[decodedKey]; exists {
+ return nil, fmt.Errorf("duplicate tag key '%s'", decodedKey)
+ }
if !hasValue {
parsedTags[decodedKey] = ""
continue
@@ -79,3 +82,11 @@ func parseTagsHeader(tags string) (map[string]string, error) {
func ValidateTags(tags map[string]string) error {
return s3tables.ValidateTags(tags)
}
+
+// maxBucketTags is how many tags a bucket's tag set can hold; an object's
+// tag set holds 10.
+const maxBucketTags = 50
+
+func ValidateBucketTags(tags map[string]string) error {
+ return s3tables.ValidateTagsWithLimit(tags, maxBucketTags)
+}
diff --git a/weed/s3api/tags_test.go b/weed/s3api/tags_test.go
index a7dd1b806e..f313806ae5 100644
--- a/weed/s3api/tags_test.go
+++ b/weed/s3api/tags_test.go
@@ -1,6 +1,7 @@
package s3api
import (
+ "strings"
"testing"
)
@@ -62,6 +63,18 @@ func TestParseTagsHeader(t *testing.T) {
expected: nil,
expectError: true,
},
+ {
+ name: "duplicate key",
+ input: "key1=value1&key1=value2",
+ expected: nil,
+ expectError: true,
+ },
+ {
+ name: "encoded duplicate key",
+ input: "key1=value1&%6bey1=value2",
+ expected: nil,
+ expectError: true,
+ },
{
name: "plus signs and equals in values",
input: "formula=a%2Bb%3Dc&normal=test",
@@ -120,3 +133,19 @@ func TestParseTagsHeader(t *testing.T) {
})
}
}
+
+func TestValidateTagsUnicodeLength(t *testing.T) {
+ // S3 counts tag length in characters, not bytes
+ multibyteKey := strings.Repeat("中", 128)
+ multibyteValue := strings.Repeat("中", 256)
+
+ if err := ValidateTags(map[string]string{multibyteKey: multibyteValue}); err != nil {
+ t.Errorf("Expected 128-char key and 256-char value to pass, got %v", err)
+ }
+ if err := ValidateTags(map[string]string{strings.Repeat("中", 129): "v"}); err == nil {
+ t.Error("Expected 129-char key to fail")
+ }
+ if err := ValidateTags(map[string]string{"k": strings.Repeat("中", 257)}); err == nil {
+ t.Error("Expected 257-char value to fail")
+ }
+}