Improve appendQuotedPath and AppendQuotedArg (#654)

* Update bytesconv.go
* Modify tests
* Remove unnecessary comments
* Update args_test.go
* Add test for appendQuotedPath
* Fix 11202 issue
* Fix some problems

args_test.go

@@ -3,6 +3,7 @@ package fasthttp
 import (
 	"bytes"
 	"fmt"
+	"net/url"
 	"reflect"
 	"strings"
 	"testing"
@@ -170,8 +171,16 @@ func TestArgsPeekMulti(t *testing.T) {
 
 func TestArgsEscape(t *testing.T) {
 	testArgsEscape(t, "foo", "bar", "foo=bar")
-	testArgsEscape(t, "f.o,1:2/4", "~`!@#$%^&*()_-=+\\|/[]{};:'\"<>,./?",
-		"f.o%2C1%3A2%2F4=%7E%60%21%40%23%24%25%5E%26*%28%29_-%3D%2B%5C%7C%2F%5B%5D%7B%7D%3B%3A%27%22%3C%3E%2C.%2F%3F")
+
+	// Test all characters
+	k := "f.o,1:2/4"
+	var v = make([]byte, 256)
+	for i := 0; i < 256; i++ {
+		v[i] = byte(i)
+	}
+	u := url.Values{}
+	u.Add(k, string(v))
+	testArgsEscape(t, k, string(v), u.Encode())
 }
 
 func testArgsEscape(t *testing.T, k, v, expectedS string) {
@@ -183,6 +192,30 @@ func testArgsEscape(t *testing.T, k, v, expectedS string) {
 	}
 }
 
+func TestPathEscape(t *testing.T) {
+	testPathEscape(t, "/foo/bar")
+	testPathEscape(t, "")
+	testPathEscape(t, "/")
+	testPathEscape(t, "//")
+	testPathEscape(t, "*") // See https://github.com/golang/go/issues/11202
+
+	// Test all characters
+	var pathSegment = make([]byte, 256)
+	for i := 0; i < 256; i++ {
+		pathSegment[i] = byte(i)
+	}
+	testPathEscape(t, "/foo/"+string(pathSegment))
+}
+
+func testPathEscape(t *testing.T, s string) {
+	u := url.URL{Path: s}
+	expectedS := u.EscapedPath()
+	res := string(appendQuotedPath(nil, []byte(s)))
+	if res != expectedS {
+		t.Fatalf("unexpected args %q. Expecting %q.", res, expectedS)
+	}
+}
+
 func TestArgsWriteTo(t *testing.T) {
 	s := "foo=bar&baz=123&aaa=bbb"
 
@@ -344,7 +377,7 @@ func TestArgsString(t *testing.T) {
 	testArgsString(t, &a, "foo=bar")
 	testArgsString(t, &a, "foo=bar&baz=sss")
 	testArgsString(t, &a, "")
-	testArgsString(t, &a, "f%20o=x.x*-_8x%D0%BF%D1%80%D0%B8%D0%B2%D0%B5aaa&sdf=ss")
+	testArgsString(t, &a, "f+o=x.x%2A-_8x%D0%BF%D1%80%D0%B8%D0%B2%D0%B5aaa&sdf=ss")
 	testArgsString(t, &a, "=asdfsdf")
 }
 

bytesconv.go

@@ -393,6 +393,58 @@ func s2b(s string) (b []byte) {
 	return b
 }
 
+var quotedArgShouldEscapeTable = func() [256]bool {
+	// According to RFC 3986 §2.3
+	var a [256]bool
+	for i := 0; i < 256; i++ {
+		a[i] = true
+	}
+
+	// ALPHA
+	for i := int('a'); i <= int('z'); i++ {
+		a[i] = false
+	}
+	for i := int('A'); i <= int('Z'); i++ {
+		a[i] = false
+	}
+
+	// DIGIT
+	for i := int('0'); i <= int('9'); i++ {
+		a[i] = false
+	}
+
+	// Unreserved characters
+	a[int('-')] = false
+	a[int('_')] = false
+	a[int('.')] = false
+	a[int('~')] = false
+
+	return a
+}()
+
+var quotedPathShouldEscapeTable = func() [256]bool {
+	// The implementation here equal to net/url shouldEscape(s, encodePath)
+	//
+	// The RFC allows : @ & = + $ but saves / ; , for assigning
+	// meaning to individual path segments. This package
+	// only manipulates the path as a whole, so we allow those
+	// last three as well. That leaves only ? to escape.
+	var a = quotedArgShouldEscapeTable
+
+	// '$', '&', '+', ',', '/', ':', ';', '=', '@'
+	a[int('$')] = false
+	a[int('&')] = false
+	a[int('+')] = false
+	a[int(',')] = false
+	a[int('/')] = false
+	a[int(':')] = false
+	a[int(';')] = false
+	a[int('=')] = false
+	a[int('@')] = false
+
+	return a
+}()
+
 // AppendUnquotedArg appends url-decoded src to dst and returns appended dst.
 //
 // dst may point to src. In this case src will be overwritten.
@@ -403,33 +455,29 @@ func AppendUnquotedArg(dst, src []byte) []byte {
 // AppendQuotedArg appends url-encoded src to dst and returns appended dst.
 func AppendQuotedArg(dst, src []byte) []byte {
 	for _, c := range src {
-		// See http://www.w3.org/TR/html5/forms.html#form-submission-algorithm
-		if c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' ||
-			c == '*' || c == '-' || c == '.' || c == '_' {
-			dst = append(dst, c)
-		} else {
+		switch {
+		case c == ' ':
+			dst = append(dst, '+')
+		case quotedArgShouldEscapeTable[int(c)]:
 			dst = append(dst, '%', hexCharUpper(c>>4), hexCharUpper(c&15))
+		default:
+			dst = append(dst, c)
 		}
 	}
 	return dst
 }
 
 func appendQuotedPath(dst, src []byte) []byte {
+	// Fix issue in https://github.com/golang/go/issues/11202
+	if len(src) == 1 && src[0] == '*' {
+		return append(dst, '*')
+	}
+
 	for _, c := range src {
-		// From the spec: http://tools.ietf.org/html/rfc3986#section-3.3
-		// an path can contain zero or more of pchar that is defined as follows:
-		// pchar       = unreserved / pct-encoded / sub-delims / ":" / "@"
-		// pct-encoded = "%" HEXDIG HEXDIG
-		// unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"
-		// sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
-		//             / "*" / "+" / "," / ";" / "="
-		if c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' ||
-			c == '-' || c == '.' || c == '_' || c == '~' || c == '!' || c == '$' ||
-			c == '&' || c == '\'' || c == '(' || c == ')' || c == '*' || c == '+' ||
-			c == ',' || c == ';' || c == '=' || c == ':' || c == '@' || c == '/' {
-			dst = append(dst, c)
-		} else {
+		if quotedPathShouldEscapeTable[int(c)] {
 			dst = append(dst, '%', hexCharUpper(c>>4), hexCharUpper(c&15))
+		} else {
+			dst = append(dst, c)
 		}
 	}
 	return dst