Remove cookie name decoding (#368)

Tratcher · web-flow · commit 535ab4ccc4dd · 2020-08-12T16:51:22.000-07:00
diff --git a/src/Microsoft.Owin/Infrastructure/OwinHelpers.cs b/src/Microsoft.Owin/Infrastructure/OwinHelpers.cs
@@ -530,13 +530,13 @@ internal static IDictionary<string, string> GetCookies(IOwinRequest request)
             if (request.Get<string>("Microsoft.Owin.Cookies#text") != text)
             {
                 cookies.Clear();
-                ParseDelimited(text, SemicolonAndComma, AddCookieCallback, cookies);
+                ParseDelimited(text, SemicolonAndComma, AddCookieCallback, decodePlus: false, decodeKey: false, state: cookies);
                 request.Set("Microsoft.Owin.Cookies#text", text);
             }
             return cookies;
         }
 
-        internal static void ParseDelimited(string text, char[] delimiters, Action<string, string, object> callback, object state)
+        internal static void ParseDelimited(string text, char[] delimiters, Action<string, string, object> callback, bool decodePlus, bool decodeKey, object state)
         {
             int textLength = text.Length;
             int equalIndex = text.IndexOf('=');
@@ -560,10 +560,17 @@ internal static void ParseDelimited(string text, char[] delimiters, Action<strin
                     }
                     string name = text.Substring(scanIndex, equalIndex - scanIndex);
                     string value = text.Substring(equalIndex + 1, delimiterIndex - equalIndex - 1);
-                    callback(
-                        Uri.UnescapeDataString(name.Replace('+', ' ')),
-                        Uri.UnescapeDataString(value.Replace('+', ' ')),
-                        state);
+                    if (decodePlus)
+                    {
+                        name.Replace('+', ' ');
+                        value.Replace('+', ' ');
+                    }
+                    if (decodeKey)
+                    {
+                        name = Uri.UnescapeDataString(name);
+                    }
+                    value = Uri.UnescapeDataString(value);
+                    callback(name, value, state);
                     equalIndex = text.IndexOf('=', delimiterIndex);
                     if (equalIndex == -1)
                     {
@@ -799,7 +806,7 @@ internal static IDictionary<string, string[]> GetQuery(IOwinRequest request)
             {
                 query.Clear();
                 var accumulator = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);
-                ParseDelimited(text, AmpersandAndSemicolon, AppendItemCallback, accumulator);
+                ParseDelimited(text, AmpersandAndSemicolon, AppendItemCallback, decodePlus: true, decodeKey: true, state: accumulator);
                 foreach (var kv in accumulator)
                 {
                     query.Add(kv.Key, kv.Value.ToArray());
@@ -813,7 +820,7 @@ internal static IFormCollection GetForm(string text)
         {
             IDictionary<string, string[]> form = new Dictionary<string, string[]>(StringComparer.OrdinalIgnoreCase);
             var accumulator = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);
-            ParseDelimited(text, new[] { '&' }, AppendItemCallback, accumulator);
+            ParseDelimited(text, new[] { '&' }, AppendItemCallback, decodePlus: false, decodeKey: true, state: accumulator);
             foreach (var kv in accumulator)
             {
                 form.Add(kv.Key, kv.Value.ToArray());
diff --git a/src/Microsoft.Owin/RequestCookieCollection.cs b/src/Microsoft.Owin/RequestCookieCollection.cs
@@ -37,8 +37,11 @@ public string this[string key]
             get
             {
                 string value;
-                Store.TryGetValue(key, out value);
-                return value;
+                if (Store.TryGetValue(key, out value) || Store.TryGetValue(Uri.EscapeDataString(key), out value))
+                {
+                    return value;
+                }
+                return null;
             }
         }
 
diff --git a/tests/Microsoft.Owin.Tests/FormsTests.cs b/tests/Microsoft.Owin.Tests/FormsTests.cs
@@ -14,7 +14,7 @@ public class FormsTests
         private static readonly string[] RawValues = new[] { "v1", "v2, v3", "\"v4, b\"", "v5, v6", "v7", };
         private const string JoinedValues = "v1,v2, v3,\"v4, b\",v5, v6,v7";
 
-        private const string OriginalFormsString = "q1=v1&q2=v2,b&q3=v3&q3=v4&q4&q5=v5&q5=v5";
+        private const string OriginalFormsString = "q1=v1&q2=v2,b&q3=v3&q3=v4&q4&q5=v5&q5=v+5";
 
         [Fact]
         public void ParseForm()
@@ -30,7 +30,7 @@ public void ParseForm()
             Assert.Equal("v2,b", form.Get("Q2"));
             Assert.Equal("v3,v4", form.Get("q3"));
             Assert.Null(form.Get("q4"));
-            Assert.Equal("v5,v5", form.Get("Q5"));
+            Assert.Equal("v5,v+5", form.Get("Q5"));
             Assert.True(stream.CanRead);
         }
 
@@ -89,7 +89,7 @@ public void ReadFromStream()
             Assert.Equal("v2,b", form.Get("Q2"));
             Assert.Equal("v3,v4", form.Get("q3"));
             Assert.Null(form.Get("q4"));
-            Assert.Equal("v5,v5", form.Get("Q5"));
+            Assert.Equal("v5,v+5", form.Get("Q5"));
         }
 
         [Fact]
@@ -107,14 +107,14 @@ public void ReadFromStreamTwice()
             Assert.Equal("v2,b", form.Get("Q2"));
             Assert.Equal("v3,v4", form.Get("q3"));
             Assert.Null(form.Get("q4"));
-            Assert.Equal("v5,v5", form.Get("Q5"));
+            Assert.Equal("v5,v+5", form.Get("Q5"));
 
             form = request.ReadFormAsync().Result;
             Assert.Equal("v1", form.Get("q1"));
             Assert.Equal("v2,b", form.Get("Q2"));
             Assert.Equal("v3,v4", form.Get("q3"));
             Assert.Null(form.Get("q4"));
-            Assert.Equal("v5,v5", form.Get("Q5"));
+            Assert.Equal("v5,v+5", form.Get("Q5"));
         }
     }
 }
diff --git a/tests/Microsoft.Owin.Tests/Microsoft.Owin.Tests.csproj b/tests/Microsoft.Owin.Tests/Microsoft.Owin.Tests.csproj
@@ -63,6 +63,7 @@
     <Compile Include="Builder\AppBuilderTestExtensions.cs" />
     <Compile Include="Builder\AppBuilderTests.cs" />
     <Compile Include="CookieChunkingTests.cs" />
+    <Compile Include="RequestCookieTests.cs" />
     <Compile Include="FormsTests.cs" />
     <Compile Include="HeaderTests.cs" />
     <Compile Include="HostStringTests.cs" />
diff --git a/tests/Microsoft.Owin.Tests/RequestCookieTests.cs b/tests/Microsoft.Owin.Tests/RequestCookieTests.cs
@@ -0,0 +1,31 @@
+﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Linq;
+using Xunit;
+using Xunit.Extensions;
+
+namespace Microsoft.Owin.Tests
+{
+    public class RequestCookieTests
+    {
+        [Theory]
+        [InlineData("key=value", "key", "value")]
+        [InlineData("__secure-key=value", "__secure-key", "value")]
+        [InlineData("key%2C=%21value", "key,", "!value")]
+        [InlineData("ke%23y%2C=val%5Eue", "ke#y,", "val^ue")]
+        [InlineData("base64=QUI%2BREU%2FRw%3D%3D", "base64", "QUI+REU/Rw==")]
+        [InlineData("base64=QUI+REU/Rw==", "base64", "QUI+REU/Rw==")]
+        public void UnEscapesValues(string input, string expectedKey, string expectedValue)
+        {
+            var context = new OwinRequest();
+            context.Headers["Cookie"] = input;
+            var cookies = context.Cookies;
+
+            Assert.Equal(1, cookies.Count());
+            Assert.Equal(Uri.EscapeDataString(expectedKey), cookies.Single().Key);
+            Assert.Equal(expectedValue, cookies[expectedKey]);
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -530,13 +530,13 @@ internal static IDictionary<string, string> GetCookies(IOwinRequest request)`
`530`	`530`	`if (request.Get<string>("Microsoft.Owin.Cookies#text") != text)`
`531`	`531`	`{`
`532`	`532`	`cookies.Clear();`
`533`		`- ParseDelimited(text, SemicolonAndComma, AddCookieCallback, cookies);`
	`533`	`+ ParseDelimited(text, SemicolonAndComma, AddCookieCallback, decodePlus: false, decodeKey: false, state: cookies);`
`534`	`534`	`request.Set("Microsoft.Owin.Cookies#text", text);`
`535`	`535`	`}`
`536`	`536`	`return cookies;`
`537`	`537`	`}`
`538`	`538`
`539`		`- internal static void ParseDelimited(string text, char[] delimiters, Action<string, string, object> callback, object state)`
	`539`	`+ internal static void ParseDelimited(string text, char[] delimiters, Action<string, string, object> callback, bool decodePlus, bool decodeKey, object state)`
`540`	`540`	`{`
`541`	`541`	`int textLength = text.Length;`
`542`	`542`	`int equalIndex = text.IndexOf('=');`
`@@ -560,10 +560,17 @@ internal static void ParseDelimited(string text, char[] delimiters, Action<strin`
`560`	`560`	`}`
`561`	`561`	`string name = text.Substring(scanIndex, equalIndex - scanIndex);`
`562`	`562`	`string value = text.Substring(equalIndex + 1, delimiterIndex - equalIndex - 1);`
`563`		`- callback(`
`564`		`- Uri.UnescapeDataString(name.Replace('+', ' ')),`
`565`		`- Uri.UnescapeDataString(value.Replace('+', ' ')),`
`566`		`- state);`
	`563`	`+ if (decodePlus)`
	`564`	`+ {`
	`565`	`+ name.Replace('+', ' ');`
	`566`	`+ value.Replace('+', ' ');`
	`567`	`+ }`
	`568`	`+ if (decodeKey)`
	`569`	`+ {`
	`570`	`+ name = Uri.UnescapeDataString(name);`
	`571`	`+ }`
	`572`	`+ value = Uri.UnescapeDataString(value);`
	`573`	`+ callback(name, value, state);`
`567`	`574`	`equalIndex = text.IndexOf('=', delimiterIndex);`
`568`	`575`	`if (equalIndex == -1)`
`569`	`576`	`{`
`@@ -799,7 +806,7 @@ internal static IDictionary<string, string[]> GetQuery(IOwinRequest request)`
`799`	`806`	`{`
`800`	`807`	`query.Clear();`
`801`	`808`	`var accumulator = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);`
`802`		`- ParseDelimited(text, AmpersandAndSemicolon, AppendItemCallback, accumulator);`
	`809`	`+ ParseDelimited(text, AmpersandAndSemicolon, AppendItemCallback, decodePlus: true, decodeKey: true, state: accumulator);`
`803`	`810`	`foreach (var kv in accumulator)`
`804`	`811`	`{`
`805`	`812`	`query.Add(kv.Key, kv.Value.ToArray());`
`@@ -813,7 +820,7 @@ internal static IFormCollection GetForm(string text)`
`813`	`820`	`{`
`814`	`821`	`IDictionary<string, string[]> form = new Dictionary<string, string[]>(StringComparer.OrdinalIgnoreCase);`
`815`	`822`	`var accumulator = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);`
`816`		`- ParseDelimited(text, new[] { '&' }, AppendItemCallback, accumulator);`
	`823`	`+ ParseDelimited(text, new[] { '&' }, AppendItemCallback, decodePlus: false, decodeKey: true, state: accumulator);`
`817`	`824`	`foreach (var kv in accumulator)`
`818`	`825`	`{`
`819`	`826`	`form.Add(kv.Key, kv.Value.ToArray());`
Original file line number	Diff line number	Diff line change
`@@ -37,8 +37,11 @@ public string this[string key]`
`37`	`37`	`get`
`38`	`38`	`{`
`39`	`39`	`string value;`
`40`		`- Store.TryGetValue(key, out value);`
`41`		`- return value;`
	`40`	`+ if (Store.TryGetValue(key, out value) \|\| Store.TryGetValue(Uri.EscapeDataString(key), out value))`
	`41`	`+ {`
	`42`	`+ return value;`
	`43`	`+ }`
	`44`	`+ return null;`
`42`	`45`	`}`
`43`	`46`	`}`
`44`	`47`