Make Encoding URL more compliant to S3 spec (#7360)

There is no written specification about how to encode key names
when url encoding type is passed.

However, this change will encode URLs as url.QueryEscape() does
while considering AWS S3 exceptions.

cmd/api-response.go

@@ -305,21 +305,6 @@ func getObjectLocation(r *http.Request, domains []string, bucket, object string)
 	return u.String()
 }
 
-// s3EncodeName encodes string in response when encodingType
-// is specified in AWS S3 requests.
-func s3EncodeName(name string, encodingType string) (result string) {
-	// Quick path to exit
-	if encodingType == "" {
-		return name
-	}
-	encodingType = strings.ToLower(encodingType)
-	switch encodingType {
-	case "url":
-		return url.QueryEscape(name)
-	}
-	return name
-}
-
 // generates ListBucketsResponse from array of BucketInfo which can be
 // serialized to match XML and JSON API spec output.
 func generateListBucketsResponse(buckets []BucketInfo) ListBucketsResponse {

cmd/api-utils.go

@@ -0,0 +1,107 @@
+/*
+ * Minio Cloud Storage, (C) 2019 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import (
+	"strings"
+)
+
+func shouldEscape(c byte) bool {
+	if 'A' <= c && c <= 'Z' || 'a' <= c && c <= 'z' || '0' <= c && c <= '9' {
+		return false
+	}
+
+	switch c {
+	case '-', '_', '.', '/', '*':
+		return false
+	}
+	return true
+}
+
+// s3URLEncode is based on Golang's url.QueryEscape() code,
+// while considering some S3 exceptions:
+//	- Avoid encoding '/' and '*'
+//	- Force encoding of '~'
+func s3URLEncode(s string) string {
+	spaceCount, hexCount := 0, 0
+	for i := 0; i < len(s); i++ {
+		c := s[i]
+		if shouldEscape(c) {
+			if c == ' ' {
+				spaceCount++
+			} else {
+				hexCount++
+			}
+		}
+	}
+
+	if spaceCount == 0 && hexCount == 0 {
+		return s
+	}
+
+	var buf [64]byte
+	var t []byte
+
+	required := len(s) + 2*hexCount
+	if required <= len(buf) {
+		t = buf[:required]
+	} else {
+		t = make([]byte, required)
+	}
+
+	if hexCount == 0 {
+		copy(t, s)
+		for i := 0; i < len(s); i++ {
+			if s[i] == ' ' {
+				t[i] = '+'
+			}
+		}
+		return string(t)
+	}
+
+	j := 0
+	for i := 0; i < len(s); i++ {
+		switch c := s[i]; {
+		case c == ' ':
+			t[j] = '+'
+			j++
+		case shouldEscape(c):
+			t[j] = '%'
+			t[j+1] = "0123456789ABCDEF"[c>>4]
+			t[j+2] = "0123456789ABCDEF"[c&15]
+			j += 3
+		default:
+			t[j] = s[i]
+			j++
+		}
+	}
+	return string(t)
+}
+
+// s3EncodeName encodes string in response when encodingType is specified in AWS S3 requests.
+func s3EncodeName(name string, encodingType string) (result string) {
+	// Quick path to exit
+	if encodingType == "" {
+		return name
+	}
+	encodingType = strings.ToLower(encodingType)
+	switch encodingType {
+	case "url":
+		return s3URLEncode(name)
+	}
+	return name
+}

cmd/api-utils_test.go

@@ -0,0 +1,49 @@
+/*
+ * Minio Cloud Storage, (C) 2019 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestS3EncodeName(t *testing.T) {
+	testCases := []struct {
+		inputText, encodingType, expectedOutput string
+	}{
+		{"a b", "", "a b"},
+		{"a b", "url", "a+b"},
+		{"p- ", "url", "p-+"},
+		{"p-%", "url", "p-%25"},
+		{"p/", "url", "p/"},
+		{"p/", "url", "p/"},
+		{"~user", "url", "%7Euser"},
+		{"*user", "url", "*user"},
+		{"user+password", "url", "user%2Bpassword"},
+		{"_user", "url", "_user"},
+		{"firstname.lastname", "url", "firstname.lastname"},
+	}
+	for i, testCase := range testCases {
+		t.Run(fmt.Sprintf("Test%d", i+1), func(t *testing.T) {
+			outputText := s3EncodeName(testCase.inputText, testCase.encodingType)
+			if testCase.expectedOutput != outputText {
+				t.Errorf("Expected `%s`, got `%s`", testCase.expectedOutput, outputText)
+			}
+
+		})
+	}
+}