diff --git a/weed/s3api/s3api_object_copy_handlers.go b/weed/s3api/s3api_object_copy_handlers.go index 8e62deb94..127a56bb9 100644 --- a/weed/s3api/s3api_object_copy_handlers.go +++ b/weed/s3api/s3api_object_copy_handlers.go @@ -287,7 +287,7 @@ func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, rep if err != nil { return nil, err } - err = validateTags(parsedTags) + err = ValidateTags(parsedTags) if err != nil { return nil, err } diff --git a/weed/s3api/s3api_object_copy_handlers_test.go b/weed/s3api/s3api_object_copy_handlers_test.go index 3cb2b0562..1740d426b 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,7 @@ func TestProcessMetadata(t *testing.T) { reqHeader := transferHToHeader(tc.request) existing := transferHToHeader(tc.existing) replaceMeta, replaceTagging := replaceDirective(reqHeader) - + fmt.Println("test") err := processMetadata(reqHeader, existing, replaceMeta, replaceTagging, func(_ string, _ string) (tags map[string]string, err error) { return tc.getTags, nil }, "", "") diff --git a/weed/s3api/s3api_object_tagging_handlers.go b/weed/s3api/s3api_object_tagging_handlers.go index 35da1cc20..8e1993584 100644 --- a/weed/s3api/s3api_object_tagging_handlers.go +++ b/weed/s3api/s3api_object_tagging_handlers.go @@ -62,7 +62,7 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R return } tags := tagging.ToTags() - err = validateTags(tags) + err = ValidateTags(tags) if err != nil { glog.Errorf("PutObjectTaggingHandler ValidateTags error %s: %v", r.URL, err) s3err.WriteErrorResponse(w, r, s3err.ErrInvalidTag) diff --git a/weed/s3api/tags.go b/weed/s3api/tags.go index de65df2f9..d49db6894 100644 --- a/weed/s3api/tags.go +++ b/weed/s3api/tags.go @@ -54,24 +54,24 @@ func parseTagsHeader(tags string) (map[string]string, error) { return parsedTags, nil } -func validateTags(tags map[string]string) error { +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 %s longer than 128", k) + 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 %w ", k, err) + 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 %s longer than 256", v) + 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 %w ", v, err) + if !validateValue || err != nil { + return fmt.Errorf("validate tags value %s error, incorrect value", v) } } 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) + } + } +}