Do not unescape forward slashes in the request path (#146).

Cesar Blum Silveira · Cesar Blum Silveira · commit 88b1e3697602 · 2015-12-22T14:17:21.000-08:00
diff --git a/global.json b/global.json
@@ -1,3 +1,3 @@
 {
-    "projects": ["src"]
+    "projects": ["src", "test"]
 }
diff --git a/src/Microsoft.Net.Http.Server/RequestProcessing/RequestUriBuilder.cs b/src/Microsoft.Net.Http.Server/RequestProcessing/RequestUriBuilder.cs
@@ -50,8 +50,8 @@ internal sealed class RequestUriBuilder
         private StringBuilder _requestUriString;
 
         // The raw path is parsed by looping through all characters from left to right. 'rawOctets'
-        // is used to store consecutive percent encoded octets as actual byte values: e.g. for path /pa%C3%84th%2F/
-        // rawOctets will be set to { 0xC3, 0x84 } when we reach character 't' and it will be { 0x2F } when
+        // is used to store consecutive percent encoded octets as actual byte values: e.g. for path /pa%C3%84th%20/
+        // rawOctets will be set to { 0xC3, 0x84 } when we reach character 't' and it will be { 0x20 } when
         // we reach the final '/'. I.e. after a sequence of percent encoded octets ends, we use rawOctets as 
         // input to the encoding and percent encode the resulting string into UTF-8 octets.
         //
@@ -68,7 +68,7 @@ internal sealed class RequestUriBuilder
         static RequestUriBuilder()
         {
             // TODO: False triggers more detailed/correct parsing, but it's rather slow.
-            UseCookedRequestUrl = true; // SettingsSectionInternal.Section.HttpListenerUnescapeRequestUrl;
+            UseCookedRequestUrl = false; // SettingsSectionInternal.Section.HttpListenerUnescapeRequestUrl;
             Utf8Encoding = new UTF8Encoding(false, true);
 #if DOTNET5_4
             AnsiEncoding = Utf8Encoding;
@@ -323,7 +323,7 @@ private ParsingResult ParseRawPath(Encoding encoding)
                 if (current == '%')
                 {
                     // Assert is enough, since http.sys accepted the request string already. This should never happen.
-                    Debug.Assert(index + 2 < _rawPath.Length, "Expected >=2 characters after '%' (e.g. %2F)");
+                    Debug.Assert(index + 2 < _rawPath.Length, "Expected >=2 characters after '%' (e.g. %20)");
 
                     index++;
                     current = _rawPath[index];
@@ -332,9 +332,7 @@ private ParsingResult ParseRawPath(Encoding encoding)
                         // We found "%u" which means, we have a Unicode code point of the form "%uXXXX".
                         Debug.Assert(index + 4 < _rawPath.Length, "Expected >=4 characters after '%u' (e.g. %u0062)");
 
-                        // Decode the content of rawOctets into percent encoded UTF-8 characters and append them
-                        // to requestUriString.
-                        if (!EmptyDecodeAndAppendRawOctetsList(encoding))
+                        if (!EmptyDecodeAndAppendDecodedOctetsList(encoding))
                         {
                             return ParsingResult.EncodingError;
                         }
@@ -346,19 +344,26 @@ private ParsingResult ParseRawPath(Encoding encoding)
                     }
                     else
                     {
-                        // We found '%', but not followed by 'u', i.e. we have a percent encoded octed: %XX 
-                        if (!AddPercentEncodedOctetToRawOctetsList(encoding, _rawPath.Substring(index, 2)))
+                        // We found '%', but not followed by 'u', i.e. we have a percent encoded octet: %XX
+                        var octetString = _rawPath.Substring(index, 2);
+
+                        // Leave %2F as is, otherwise add to raw octets list for unescaping
+                        if (octetString == "2F" || octetString == "2f")
+                        {
+                            _requestUriString.Append('%');
+                            _requestUriString.Append(octetString);
+                        }
+                        else if (!AddPercentEncodedOctetToRawOctetsList(encoding, octetString))
                         {
                             return ParsingResult.InvalidString;
                         }
+
                         index += 2;
                     }
                 }
                 else
                 {
-                    // We found a non-'%' character: decode the content of rawOctets into percent encoded
-                    // UTF-8 characters and append it to the result. 
-                    if (!EmptyDecodeAndAppendRawOctetsList(encoding))
+                    if (!EmptyDecodeAndAppendDecodedOctetsList(encoding))
                     {
                         return ParsingResult.EncodingError;
                     }
@@ -370,7 +375,7 @@ private ParsingResult ParseRawPath(Encoding encoding)
 
             // if the raw path ends with a sequence of percent encoded octets, make sure those get added to the
             // result (requestUriString).
-            if (!EmptyDecodeAndAppendRawOctetsList(encoding))
+            if (!EmptyDecodeAndAppendDecodedOctetsList(encoding))
             {
                 return ParsingResult.EncodingError;
             }
@@ -424,7 +429,7 @@ private bool AddPercentEncodedOctetToRawOctetsList(Encoding encoding, string esc
             return true;
         }
 
-        private bool EmptyDecodeAndAppendRawOctetsList(Encoding encoding)
+        private bool EmptyDecodeAndAppendDecodedOctetsList(Encoding encoding)
         {
             if (_rawOctets.Count == 0)
             {
@@ -436,30 +441,17 @@ private bool EmptyDecodeAndAppendRawOctetsList(Encoding encoding)
             {
                 // If the encoding can get a string out of the byte array, this is a valid string in the
                 // 'encoding' encoding.
-                byte[] bytes = _rawOctets.ToArray();
+                var bytes = _rawOctets.ToArray();
                 decodedString = encoding.GetString(bytes, 0, bytes.Length);
 
-                if (encoding == Utf8Encoding)
-                {
-                    AppendOctetsPercentEncoded(_requestUriString, bytes);
-                }
-                else
-                {
-                    AppendOctetsPercentEncoded(_requestUriString, Utf8Encoding.GetBytes(decodedString));
-                }
-
+                _requestUriString.Append(decodedString);
                 _rawOctets.Clear();
 
                 return true;
             }
             catch (DecoderFallbackException e)
             {
-                LogWarning("EmptyDecodeAndAppendRawOctetsList", "Can't convert bytes: " + GetOctetsAsString(_rawOctets), e.Message);
-            }
-            catch (EncoderFallbackException e)
-            {
-                // If utf8Encoding.GetBytes() fails
-                LogWarning("EmptyDecodeAndAppendRawOctetsList", "Can't convert bytes: " + decodedString, e.Message);
+                LogWarning(nameof(EmptyDecodeAndAppendDecodedOctetsList), "Can't convert bytes: " + GetOctetsAsString(_rawOctets), e.Message);
             }
 
             return false;
diff --git a/test/Microsoft.AspNet.Server.WebListener.FunctionalTests/RequestTests.cs b/test/Microsoft.AspNet.Server.WebListener.FunctionalTests/RequestTests.cs
@@ -87,6 +87,7 @@ public async Task Request_SimpleGet_Success()
         [InlineData("/basepath/", "/basepath/subpath", "/basepath", "/subpath")]
         [InlineData("/base path/", "/base%20path/sub path", "/base path", "/sub path")]
         [InlineData("/base葉path/", "/base%E8%91%89path/sub%E8%91%89path", "/base葉path", "/sub葉path")]
+        [InlineData("/basepath/", "/basepath/sub%2Fpath", "/basepath", "/sub%2Fpath")]
         public async Task Request_PathSplitting(string pathBase, string requestPath, string expectedPathBase, string expectedPath)
         {
             string root;
diff --git a/test/Microsoft.Net.Http.Server.FunctionalTests/RequestTests.cs b/test/Microsoft.Net.Http.Server.FunctionalTests/RequestTests.cs
@@ -56,6 +56,7 @@ public async Task Request_SimpleGet_Success()
         [InlineData("/basepath/", "/basepath/subpath", "/basepath", "/subpath")]
         [InlineData("/base path/", "/base%20path/sub path", "/base path", "/sub path")]
         [InlineData("/base葉path/", "/base%E8%91%89path/sub%E8%91%89path", "/base葉path", "/sub葉path")]
+        [InlineData("/basepath/", "/basepath/sub%2Fpath", "/basepath", "/sub%2Fpath")]
         public async Task Request_PathSplitting(string pathBase, string requestPath, string expectedPathBase, string expectedPath)
         {
             string root;
@@ -70,8 +71,8 @@ public async Task Request_PathSplitting(string pathBase, string requestPath, str
 
                 // Request Keys
                 Assert.Equal("http", request.Scheme);
-                Assert.Equal(expectedPath, request.Path);
                 Assert.Equal(expectedPathBase, request.PathBase);
+                Assert.Equal(expectedPath, request.Path);
                 Assert.Equal(string.Empty, request.QueryString);
                 context.Dispose();
 

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,3 @@`
`1`	`1`	`{`
`2`		`- "projects": ["src"]`
	`2`	`+ "projects": ["src", "test"]`
`3`	`3`	`}`
Original file line number	Diff line number	Diff line change
`@@ -50,8 +50,8 @@ internal sealed class RequestUriBuilder`
`50`	`50`	`private StringBuilder _requestUriString;`
`51`	`51`
`52`	`52`	`// The raw path is parsed by looping through all characters from left to right. 'rawOctets'`
`53`		`- // is used to store consecutive percent encoded octets as actual byte values: e.g. for path /pa%C3%84th%2F/`
`54`		`- // rawOctets will be set to { 0xC3, 0x84 } when we reach character 't' and it will be { 0x2F } when`
	`53`	`+ // is used to store consecutive percent encoded octets as actual byte values: e.g. for path /pa%C3%84th%20/`
	`54`	`+ // rawOctets will be set to { 0xC3, 0x84 } when we reach character 't' and it will be { 0x20 } when`
`55`	`55`	`// we reach the final '/'. I.e. after a sequence of percent encoded octets ends, we use rawOctets as`
`56`	`56`	`// input to the encoding and percent encode the resulting string into UTF-8 octets.`
`57`	`57`	`//`
`@@ -68,7 +68,7 @@ internal sealed class RequestUriBuilder`
`68`	`68`	`static RequestUriBuilder()`
`69`	`69`	`{`
`70`	`70`	`// TODO: False triggers more detailed/correct parsing, but it's rather slow.`
`71`		`- UseCookedRequestUrl = true; // SettingsSectionInternal.Section.HttpListenerUnescapeRequestUrl;`
	`71`	`+ UseCookedRequestUrl = false; // SettingsSectionInternal.Section.HttpListenerUnescapeRequestUrl;`
`72`	`72`	`Utf8Encoding = new UTF8Encoding(false, true);`
`73`	`73`	`#if DOTNET5_4`
`74`	`74`	`AnsiEncoding = Utf8Encoding;`
`@@ -323,7 +323,7 @@ private ParsingResult ParseRawPath(Encoding encoding)`
`323`	`323`	`if (current == '%')`
`324`	`324`	`{`
`325`	`325`	`// Assert is enough, since http.sys accepted the request string already. This should never happen.`
`326`		`- Debug.Assert(index + 2 < _rawPath.Length, "Expected >=2 characters after '%' (e.g. %2F)");`
	`326`	`+ Debug.Assert(index + 2 < _rawPath.Length, "Expected >=2 characters after '%' (e.g. %20)");`
`327`	`327`
`328`	`328`	`index++;`
`329`	`329`	`current = _rawPath[index];`
`@@ -332,9 +332,7 @@ private ParsingResult ParseRawPath(Encoding encoding)`
`332`	`332`	`// We found "%u" which means, we have a Unicode code point of the form "%uXXXX".`
`333`	`333`	`Debug.Assert(index + 4 < _rawPath.Length, "Expected >=4 characters after '%u' (e.g. %u0062)");`
`334`	`334`
`335`		`- // Decode the content of rawOctets into percent encoded UTF-8 characters and append them`
`336`		`- // to requestUriString.`
`337`		`- if (!EmptyDecodeAndAppendRawOctetsList(encoding))`
	`335`	`+ if (!EmptyDecodeAndAppendDecodedOctetsList(encoding))`
`338`	`336`	`{`
`339`	`337`	`return ParsingResult.EncodingError;`
`340`	`338`	`}`
`@@ -346,19 +344,26 @@ private ParsingResult ParseRawPath(Encoding encoding)`
`346`	`344`	`}`
`347`	`345`	`else`
`348`	`346`	`{`
`349`		`- // We found '%', but not followed by 'u', i.e. we have a percent encoded octed: %XX`
`350`		`- if (!AddPercentEncodedOctetToRawOctetsList(encoding, _rawPath.Substring(index, 2)))`
	`347`	`+ // We found '%', but not followed by 'u', i.e. we have a percent encoded octet: %XX`
	`348`	`+ var octetString = _rawPath.Substring(index, 2);`
	`349`	`+`
	`350`	`+ // Leave %2F as is, otherwise add to raw octets list for unescaping`
	`351`	`+ if (octetString == "2F" \|\| octetString == "2f")`
	`352`	`+ {`
	`353`	`+ _requestUriString.Append('%');`
	`354`	`+ _requestUriString.Append(octetString);`
	`355`	`+ }`
	`356`	`+ else if (!AddPercentEncodedOctetToRawOctetsList(encoding, octetString))`
`351`	`357`	`{`
`352`	`358`	`return ParsingResult.InvalidString;`
`353`	`359`	`}`
	`360`	`+`
`354`	`361`	`index += 2;`
`355`	`362`	`}`
`356`	`363`	`}`
`357`	`364`	`else`
`358`	`365`	`{`
`359`		`- // We found a non-'%' character: decode the content of rawOctets into percent encoded`
`360`		`- // UTF-8 characters and append it to the result.`
`361`		`- if (!EmptyDecodeAndAppendRawOctetsList(encoding))`
	`366`	`+ if (!EmptyDecodeAndAppendDecodedOctetsList(encoding))`
`362`	`367`	`{`
`363`	`368`	`return ParsingResult.EncodingError;`
`364`	`369`	`}`
`@@ -370,7 +375,7 @@ private ParsingResult ParseRawPath(Encoding encoding)`
`370`	`375`
`371`	`376`	`// if the raw path ends with a sequence of percent encoded octets, make sure those get added to the`
`372`	`377`	`// result (requestUriString).`
`373`		`- if (!EmptyDecodeAndAppendRawOctetsList(encoding))`
	`378`	`+ if (!EmptyDecodeAndAppendDecodedOctetsList(encoding))`
`374`	`379`	`{`
`375`	`380`	`return ParsingResult.EncodingError;`
`376`	`381`	`}`
`@@ -424,7 +429,7 @@ private bool AddPercentEncodedOctetToRawOctetsList(Encoding encoding, string esc`
`424`	`429`	`return true;`
`425`	`430`	`}`
`426`	`431`
`427`		`- private bool EmptyDecodeAndAppendRawOctetsList(Encoding encoding)`
	`432`	`+ private bool EmptyDecodeAndAppendDecodedOctetsList(Encoding encoding)`
`428`	`433`	`{`
`429`	`434`	`if (_rawOctets.Count == 0)`
`430`	`435`	`{`
`@@ -436,30 +441,17 @@ private bool EmptyDecodeAndAppendRawOctetsList(Encoding encoding)`
`436`	`441`	`{`
`437`	`442`	`// If the encoding can get a string out of the byte array, this is a valid string in the`
`438`	`443`	`// 'encoding' encoding.`
`439`		`- byte[] bytes = _rawOctets.ToArray();`
	`444`	`+ var bytes = _rawOctets.ToArray();`
`440`	`445`	`decodedString = encoding.GetString(bytes, 0, bytes.Length);`
`441`	`446`
`442`		`- if (encoding == Utf8Encoding)`
`443`		`- {`
`444`		`- AppendOctetsPercentEncoded(_requestUriString, bytes);`
`445`		`- }`
`446`		`- else`
`447`		`- {`
`448`		`- AppendOctetsPercentEncoded(_requestUriString, Utf8Encoding.GetBytes(decodedString));`
`449`		`- }`
`450`		`-`
	`447`	`+ _requestUriString.Append(decodedString);`
`451`	`448`	`_rawOctets.Clear();`
`452`	`449`
`453`	`450`	`return true;`
`454`	`451`	`}`
`455`	`452`	`catch (DecoderFallbackException e)`
`456`	`453`	`{`
`457`		`- LogWarning("EmptyDecodeAndAppendRawOctetsList", "Can't convert bytes: " + GetOctetsAsString(_rawOctets), e.Message);`
`458`		`- }`
`459`		`- catch (EncoderFallbackException e)`
`460`		`- {`
`461`		`- // If utf8Encoding.GetBytes() fails`
`462`		`- LogWarning("EmptyDecodeAndAppendRawOctetsList", "Can't convert bytes: " + decodedString, e.Message);`
	`454`	`+ LogWarning(nameof(EmptyDecodeAndAppendDecodedOctetsList), "Can't convert bytes: " + GetOctetsAsString(_rawOctets), e.Message);`
`463`	`455`	`}`
`464`	`456`
`465`	`457`	`return false;`
Original file line number	Diff line number	Diff line change
`@@ -87,6 +87,7 @@ public async Task Request_SimpleGet_Success()`
`87`	`87`	`[InlineData("/basepath/", "/basepath/subpath", "/basepath", "/subpath")]`
`88`	`88`	`[InlineData("/base path/", "/base%20path/sub path", "/base path", "/sub path")]`
`89`	`89`	`[InlineData("/base葉path/", "/base%E8%91%89path/sub%E8%91%89path", "/base葉path", "/sub葉path")]`
	`90`	`+ [InlineData("/basepath/", "/basepath/sub%2Fpath", "/basepath", "/sub%2Fpath")]`
`90`	`91`	`public async Task Request_PathSplitting(string pathBase, string requestPath, string expectedPathBase, string expectedPath)`
`91`	`92`	`{`
`92`	`93`	`string root;`