Merged PR 50419: Harden cookie parsing

Harden cookie parsing

Author: BrennanConroy

src/Microsoft.Owin/Infrastructure/OwinHelpers.cs

@@ -515,7 +515,7 @@ internal static partial class OwinHelpers
             }
         };
 
-        private static readonly char[] SemicolonAndComma = new[] { ';', ',' };
+        private static readonly char[] Semicolon = new[] { ';' };
 
         internal static IDictionary<string, string> GetCookies(IOwinRequest request)
         {
@@ -530,7 +530,7 @@ internal static IDictionary<string, string> GetCookies(IOwinRequest request)
             if (request.Get<string>("Microsoft.Owin.Cookies#text") != text)
             {
                 cookies.Clear();
-                ParseDelimited(text, SemicolonAndComma, AddCookieCallback, decodePlus: false, decodeKey: false, state: cookies);
+                ParseDelimited(text, Semicolon, AddCookieCallback, decodePlus: false, decodeKey: false, state: cookies);
                 request.Set("Microsoft.Owin.Cookies#text", text);
             }
             return cookies;
@@ -587,7 +587,9 @@ internal static partial class OwinHelpers
         public static string GetHeader(IDictionary<string, string[]> headers, string key)
         {
             string[] values = GetHeaderUnmodified(headers, key);
-            return values == null ? null : string.Join(",", values);
+            return values == null ? null
+                : string.Equals("cookie", key, StringComparison.OrdinalIgnoreCase)
+                    ? string.Join(";", values) : string.Join(",", values);
         }
 
         public static IEnumerable<string> GetHeaderSplit(IDictionary<string, string[]> headers, string key)

tests/Microsoft.Owin.Tests/RequestCookieTests.cs

@@ -1,7 +1,7 @@
 // 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;
 
 namespace Microsoft.Owin.Tests
@@ -11,8 +11,8 @@ 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("key%2C=%21value", "key%2C", "!value")]
+        [InlineData("ke%23y%2C=val%5Eue", "ke%23y%2C", "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)
@@ -22,8 +22,39 @@ public void UnEscapesValues(string input, string expectedKey, string expectedVal
             var cookies = context.Cookies;
 
             var cookie = Assert.Single(cookies);
-            Assert.Equal(Uri.EscapeDataString(expectedKey), cookie.Key);
+            Assert.Equal(expectedKey, cookie.Key);
             Assert.Equal(expectedValue, cookies[expectedKey]);
         }
+
+        [Theory]
+        [InlineData("key", null, null)]
+        [InlineData("=,key=value", new[] { "" }, new[] { ",key=value" })]
+        [InlineData(",key=value", new[] { ",key" }, new[] { "value" })]
+        [InlineData(",key=value; key=value2", new[] { ",key", "key" }, new[] { "value", "value2" })]
+        [InlineData("key=value; ,key2=value2", new[] { "key", ",key2" }, new[] { "value", "value2" })]
+        [InlineData("%6bey=value; key=value2", new[] { "%6bey", "key" }, new[] { "value", "value2" })]
+        [InlineData("key=value; key2=value2", new[] { "key", "key2" }, new[] { "value", "value2" })]
+        [InlineData("key=value; key=value2", new[] { "key" }, new[] { "value" })]
+        public void ParseCookies(string input, string[] expectedKeys, string[] expectedValues)
+        {
+            var context = new OwinRequest();
+            context.Headers["Cookie"] = input;
+            var cookies = context.Cookies.ToArray();
+
+            if (expectedKeys == null)
+            {
+                Assert.Empty(cookies);
+            }
+            else
+            {
+                Assert.Equal(expectedKeys.Length, cookies.Length);
+
+                for (var i = 0; i < expectedKeys.Length; i++)
+                {
+                    Assert.Equal(expectedKeys[i], cookies[i].Key);
+                    Assert.Equal(expectedValues[i], cookies[i].Value);
+                }
+            }
+        }
     }
 }