Merge pull request #146 from flrdv/master

v0.17.3.1

Author: flrdv

internal/protocol/http1/parser.go

@@ -152,9 +152,6 @@ path:
 				if len(data[i+1:]) >= 2 {
 					// fast path
 					c := (hexconv.Halfbyte[data[i+1]] << 4) | hexconv.Halfbyte[data[i+2]]
-					if strutil.IsASCIINonprintable(c) {
-						return true, nil, status.ErrBadRequest
-					}
 					if strutil.IsURLUnsafeChar(c) {
 						data[i+1] |= 0x20
 						data[i+2] |= 0x20
@@ -234,9 +231,6 @@ pathDecode2Char:
 		}
 
 		char := (hexconv.Halfbyte[p.urlEncodedChar] << 4) | hexconv.Halfbyte[data[0]]
-		if strutil.IsASCIINonprintable(char) {
-			return true, nil, status.ErrBadRequest
-		}
 		if strutil.IsURLUnsafeChar(char) {
 			if !requestLine.AppendBytes('%', p.urlEncodedChar|0x20, data[0]|0x20) {
 				return true, nil, status.ErrURITooLong
@@ -350,9 +344,6 @@ paramsValue:
 				if len(data[i+1:]) >= 2 {
 					// fast path
 					c := (hexconv.Halfbyte[data[i+1]] << 4) | hexconv.Halfbyte[data[i+2]]
-					if strutil.IsASCIINonprintable(c) {
-						return true, nil, status.ErrBadParams
-					}
 
 					if !requestLine.AppendByte(c) {
 						return true, nil, status.ErrTooLongRequestLine
@@ -412,9 +403,6 @@ paramsValueDecode2Char:
 		}
 
 		char := (hexconv.Halfbyte[p.urlEncodedChar] << 4) | hexconv.Halfbyte[data[0]]
-		if strutil.IsASCIINonprintable(char) {
-			return true, nil, status.ErrBadParams
-		}
 
 		if !requestLine.AppendByte(char) {
 			return true, nil, status.ErrTooLongRequestLine

internal/protocol/http1/parser_test.go

@@ -334,9 +334,19 @@ func TestParser(t *testing.T) {
 			require.Equal(t, "/AAA", path)
 		})
 
-		t.Run("path nonprintable", func(t *testing.T) {
-			_, err := parsePath("/%41%07")
-			require.EqualError(t, err, status.ErrBadRequest.Error())
+		t.Run("path urlencoded unicode", func(t *testing.T) {
+			t.Run("fastpath", func(t *testing.T) {
+				parser, request := getParser(config.Default())
+				_, _, err := parser.Parse([]byte("GET /%D0%9F%D0%B0%D0%B2%D0%BB%D0%BE "))
+				require.NoError(t, err)
+				require.Equal(t, "/%d0%9f%d0%b0%d0%b2%d0%bb%d0%be", request.Path)
+			})
+
+			t.Run("slowpath", func(t *testing.T) {
+				request, err := parseRequestLine("/%D0%9F%D0%B0%D0%B2%D0%BB%D0%BE")
+				require.NoError(t, err)
+				require.Equal(t, "/%d0%9f%d0%b0%d0%b2%d0%bb%d0%be", request.Path)
+			})
 		})
 
 		t.Run("unsafe", func(t *testing.T) {
@@ -395,14 +405,6 @@ func TestParser(t *testing.T) {
 				require.Equal(t, "Slava Ukraini", params.Value("hello world"))
 			})
 
-			t.Run("fastpath nonprintable", func(t *testing.T) {
-				_, err := parseParams("hello%07=world")
-				require.EqualError(t, err, status.ErrBadParams.Error())
-
-				_, err = parseParams("hello=wor%07ld")
-				require.EqualError(t, err, status.ErrBadParams.Error())
-			})
-
 			splitchars := func(str string) []string {
 				byChars := make([]string, len(str))
 				for i := range str {
@@ -420,12 +422,19 @@ func TestParser(t *testing.T) {
 				require.Equal(t, "Heroyam Slava", params.Value("Slava Ukraini"))
 			})
 
-			t.Run("slowpath nonprintable", func(t *testing.T) {
-				_, err := parseParams(splitchars("h%07ey=hello")...)
-				require.EqualError(t, err, status.ErrBadParams.Error())
+			t.Run("allow unicode values", func(t *testing.T) {
+				t.Run("fastpath", func(t *testing.T) {
+					parser, request := getParser(config.Default())
+					_, _, err := parser.Parse([]byte("GET /?n=Слава+Україні "))
+					require.NoError(t, err)
+					require.Equal(t, "Слава Україні", request.Params.Value("n"))
+				})
 
-				_, err = parseParams(splitchars("hey=he%07llo")...)
-				require.EqualError(t, err, status.ErrBadParams.Error())
+				t.Run("slowpath", func(t *testing.T) {
+					request, err := parseRequestLine("/?n=Слава+Україні")
+					require.NoError(t, err)
+					require.Equal(t, "Слава Україні", request.Params.Value("n"))
+				})
 			})
 		})
 	})
@@ -534,18 +543,13 @@ func TestParser(t *testing.T) {
 				{
 					Name:      "encoded nonprintable",
 					Sample:    "/%ff",
-					WantError: status.ErrBadRequest,
+					WantError: nil,
 				},
 				{
 					Name:      "param key nonprintable",
 					Sample:    "/?he%07llo=world",
 					WantError: status.ErrBadParams,
 				},
-				{
-					Name:      "param value nonprintable",
-					Sample:    "/?hello=wor%07ld",
-					WantError: status.ErrBadParams,
-				},
 				{
 					Name:      "fragment",
 					Sample:    "/hello#Section1",

internal/strutil/url.go

@@ -8,7 +8,7 @@ import (
 
 // IsURLUnsafeChar tells whether it's safe to decode an urlencoded character.
 func IsURLUnsafeChar(c byte) bool {
-	return c == '/'
+	return c == '/' || IsASCIINonprintable(c)
 }
 
 // URLDecode decodes an urlencoded string and tells whether the string was properly formed.
@@ -37,9 +37,6 @@ func URLDecode(str string) (string, bool) {
 		}
 
 		char := (x << 4) | y
-		if IsASCIINonprintable(char) {
-			return "", false
-		}
 		if IsURLUnsafeChar(char) {
 			b.Write([]byte{'%', c1 | 0x20, c2 | 0x20})
 			continue

internal/strutil/url_test.go

@@ -19,15 +19,9 @@ func TestURLDecode(t *testing.T) {
 		}
 	})
 
-	t.Run("unsafe char", func(t *testing.T) {
-		res, ok := URLDecode("%61%2f")
+	t.Run("unsafe char normalization", func(t *testing.T) {
+		res, ok := URLDecode("%61%2f%D0")
 		require.True(t, ok)
-		require.Equal(t, "a%2f", res)
-	})
-
-	t.Run("unsafe normalization", func(t *testing.T) {
-		res, ok := URLDecode("%61%2F")
-		require.True(t, ok)
-		require.Equal(t, "a%2f", res)
+		require.Equal(t, "a%2f%d0", res)
 	})
 }