fix parseTagging function and add test for validate tags

5 files changed, 85 insertions, 10 deletions

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)
+		}
+	}
+}