diff --git a/weed/s3api/s3api_object_copy_handlers.go b/weed/s3api/s3api_object_copy_handlers.go index 6b67ef337..950e7a8fb 100644 --- a/weed/s3api/s3api_object_copy_handlers.go +++ b/weed/s3api/s3api_object_copy_handlers.go @@ -45,7 +45,12 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request s3err.WriteErrorResponse(w, r, s3err.ErrInvalidCopySource) return } - entry.Extended = processMetadataBytes(r.Header, entry.Extended, replaceMeta, replaceTagging) + entry.Extended, err = processMetadataBytes(r.Header, entry.Extended, replaceMeta, replaceTagging) + if err != nil { + glog.Errorf("CopyObjectHandler ValidateTags error %s: %v", r.URL, err) + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag) + return + } err = s3a.touch(dir, name, entry) if err != nil { s3err.WriteErrorResponse(w, r, s3err.ErrInvalidCopySource) @@ -252,7 +257,7 @@ func processMetadata(reqHeader, existing http.Header, replaceMeta, replaceTaggin return } -func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, replaceMeta, replaceTagging bool) (metadata map[string][]byte) { +func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, replaceMeta, replaceTagging bool) (metadata map[string][]byte, err error) { metadata = make(map[string][]byte) if sc := existing[s3_constants.AmzStorageClass]; len(sc) > 0 { @@ -277,16 +282,18 @@ func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, rep } } } - if replaceTagging { if tags := reqHeader.Get(s3_constants.AmzObjectTagging); tags != "" { - for _, v := range strings.Split(tags, "&") { - tag := strings.Split(v, "=") - if len(tag) == 2 { - metadata[s3_constants.AmzObjectTagging+"-"+tag[0]] = []byte(tag[1]) - } else if len(tag) == 1 { - metadata[s3_constants.AmzObjectTagging+"-"+tag[0]] = nil - } + parsedTags, err := parseTagsHeader(tags) + if err != nil { + return nil, err + } + err = ValidateTags(parsedTags) + if err != nil { + return nil, err + } + for k, v := range parsedTags { + metadata[s3_constants.AmzObjectTagging+"-"+k] = []byte(v) } } } else { diff --git a/weed/s3api/s3api_object_copy_handlers_test.go b/weed/s3api/s3api_object_copy_handlers_test.go index 610b29a6b..29d519c24 100644 --- a/weed/s3api/s3api_object_copy_handlers_test.go +++ b/weed/s3api/s3api_object_copy_handlers_test.go @@ -332,6 +332,19 @@ var processMetadataBytesTestCases = []struct { "X-Amz-Tagging-type": "request", }, }, + + { + 108, + H{ + "User-Agent": "firefox", + "X-Amz-Meta-My-Meta": "request", + "X-Amz-Tagging": "A=B&a=b&type=request*", + s3_constants.AmzUserMetaDirective: DirectiveReplace, + s3_constants.AmzObjectTaggingDirective: DirectiveReplace, + }, + H{}, + H{}, + }, } func TestProcessMetadata(t *testing.T) { @@ -339,7 +352,6 @@ func TestProcessMetadata(t *testing.T) { reqHeader := transferHToHeader(tc.request) existing := transferHToHeader(tc.existing) replaceMeta, replaceTagging := replaceDirective(reqHeader) - err := processMetadata(reqHeader, existing, replaceMeta, replaceTagging, func(_ string, _ string) (tags map[string]string, err error) { return tc.getTags, nil }, "", "") @@ -367,7 +379,7 @@ func TestProcessMetadataBytes(t *testing.T) { reqHeader := transferHToHeader(tc.request) existing := transferHToBytesArr(tc.existing) replaceMeta, replaceTagging := replaceDirective(reqHeader) - extends := processMetadataBytes(reqHeader, existing, replaceMeta, replaceTagging) + extends, _ := processMetadataBytes(reqHeader, existing, replaceMeta, replaceTagging) result := transferBytesArrToH(extends) fmtTagging(result, tc.want) diff --git a/weed/s3api/s3api_object_tagging_handlers.go b/weed/s3api/s3api_object_tagging_handlers.go index 9fde0309c..1791d7dc8 100644 --- a/weed/s3api/s3api_object_tagging_handlers.go +++ b/weed/s3api/s3api_object_tagging_handlers.go @@ -62,23 +62,12 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R return } tags := tagging.ToTags() - if len(tags) > 10 { - glog.Errorf("PutObjectTaggingHandler tags %s: %d tags more than 10", r.URL, len(tags)) + err = ValidateTags(tags) + if err != nil { + glog.Errorf("PutObjectTaggingHandler ValidateTags error %s: %v", r.URL, err) s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag) return } - for k, v := range tags { - if len(k) > 128 { - glog.Errorf("PutObjectTaggingHandler tags %s: tag key %s longer than 128", r.URL, k) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag) - return - } - if len(v) > 256 { - glog.Errorf("PutObjectTaggingHandler tags %s: tag value %s longer than 256", r.URL, v) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag) - return - } - } if err = s3a.setTags(dir, name, tagging.ToTags()); err != nil { if err == filer_pb.ErrNotFound { diff --git a/weed/s3api/tags.go b/weed/s3api/tags.go index 979e5a80c..d49db6894 100644 --- a/weed/s3api/tags.go +++ b/weed/s3api/tags.go @@ -2,6 +2,9 @@ package s3api import ( "encoding/xml" + "fmt" + "regexp" + "strings" ) type Tag struct { @@ -37,3 +40,40 @@ func FromTags(tags map[string]string) (t *Tagging) { } return } + +func parseTagsHeader(tags string) (map[string]string, error) { + parsedTags := make(map[string]string) + for _, v := range strings.Split(tags, "&") { + tag := strings.Split(v, "=") + if len(tag) == 2 { + parsedTags[tag[0]] = tag[1] + } else if len(tag) == 1 { + parsedTags[tag[0]] = "" + } + } + return parsedTags, nil +} + +func ValidateTags(tags map[string]string) error { + if len(tags) > 10 { + return fmt.Errorf("validate tags: %d tags more than 10", len(tags)) + } + for k, v := range tags { + if len(k) > 128 { + return fmt.Errorf("validate tags: tag key longer than 128") + } + validateKey, err := regexp.MatchString(`^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$`, k) + if !validateKey || err != nil { + return fmt.Errorf("validate tags key %s error, incorrect key", k) + } + if len(v) > 256 { + return fmt.Errorf("validate tags: tag value longer than 256") + } + validateValue, err := regexp.MatchString(`^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$`, v) + if !validateValue || err != nil { + return fmt.Errorf("validate tags value %s error, incorrect value", v) + } + } + + return nil +} diff --git a/weed/s3api/tags_test.go b/weed/s3api/tags_test.go index d8beb1922..fb464fcae 100644 --- a/weed/s3api/tags_test.go +++ b/weed/s3api/tags_test.go @@ -50,3 +50,65 @@ func TestXMLMarshall(t *testing.T) { assert.Equal(t, expected, actual) } + +type TestTags map[string]string + +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) + } + } +}