#814 Rework CookieAuth for compat with CookiePolicy.

Author: Tratcher

src/Microsoft.AspNetCore.Authentication.Cookies/ChunkingCookieManager.cs

@@ -18,13 +18,16 @@ namespace Microsoft.AspNetCore.Authentication.Cookies
     /// </summary>
     public class ChunkingCookieManager : ICookieManager
     {
-        public ChunkingCookieManager(UrlEncoder urlEncoder)
+        private const string ChunkKeySuffix = "C";
+        private const string ChunkCountPrefix = "chunks-";
+
+        public ChunkingCookieManager()
         {
             // Lowest common denominator. Safari has the lowest known limit (4093), and we leave little extra just in case.
             // See http://browsercookielimits.x64.me/.
-            ChunkSize = 4090;
+            // Leave at least 20 in case CookiePolicy tries to add 'secure' and/or 'httponly'.
+            ChunkSize = 4070;
             ThrowForPartialCookies = true;
-            Encoder = urlEncoder ?? UrlEncoder.Default;
         }
 
         /// <summary>
@@ -41,14 +44,12 @@ public ChunkingCookieManager(UrlEncoder urlEncoder)
         /// </summary>
         public bool ThrowForPartialCookies { get; set; }
 
-        private UrlEncoder Encoder { get; set; }
-
-        // Parse the "chunks:XX" to determine how many chunks there should be.
+        // Parse the "chunks-XX" to determine how many chunks there should be.
         private static int ParseChunksCount(string value)
         {
-            if (value != null && value.StartsWith("chunks:", StringComparison.Ordinal))
+            if (value != null && value.StartsWith(ChunkCountPrefix, StringComparison.Ordinal))
             {
-                var chunksCountString = value.Substring("chunks:".Length);
+                var chunksCountString = value.Substring(ChunkCountPrefix.Length);
                 int chunksCount;
                 if (int.TryParse(chunksCountString, NumberStyles.None, CultureInfo.InvariantCulture, out chunksCount))
                 {
@@ -60,7 +61,7 @@ private static int ParseChunksCount(string value)
 
         /// <summary>
         /// Get the reassembled cookie. Non chunked cookies are returned normally.
-        /// Cookies with missing chunks just have their "chunks:XX" header returned.
+        /// Cookies with missing chunks just have their "chunks-XX" header returned.
         /// </summary>
         /// <param name="context"></param>
         /// <param name="key"></param>
@@ -82,11 +83,10 @@ public string GetRequestCookie(HttpContext context, string key)
             var chunksCount = ParseChunksCount(value);
             if (chunksCount > 0)
             {
-                var quoted = false;
                 var chunks = new string[chunksCount];
                 for (var chunkId = 1; chunkId <= chunksCount; chunkId++)
                 {
-                    var chunk = requestCookies[key + "C" + chunkId.ToString(CultureInfo.InvariantCulture)];
+                    var chunk = requestCookies[key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture)];
                     if (string.IsNullOrEmpty(chunk))
                     {
                         if (ThrowForPartialCookies)
@@ -102,28 +102,19 @@ public string GetRequestCookie(HttpContext context, string key)
                         // Missing chunk, abort by returning the original cookie value. It may have been a false positive?
                         return value;
                     }
-                    if (IsQuoted(chunk))
-                    {
-                        // Note: Since we assume these cookies were generated by our code, then we can assume that if one cookie has quotes then they all do.
-                        quoted = true;
-                        chunk = RemoveQuotes(chunk);
-                    }
+
                     chunks[chunkId - 1] = chunk;
                 }
-                var merged = string.Join(string.Empty, chunks);
-                if (quoted)
-                {
-                    merged = Quote(merged);
-                }
-                return merged;
+
+                return string.Join(string.Empty, chunks);
             }
             return value;
         }
 
         /// <summary>
         /// Appends a new response cookie to the Set-Cookie header. If the cookie is larger than the given size limit
         /// then it will be broken down into multiple cookies as follows:
-        /// Set-Cookie: CookieName=chunks:3; path=/
+        /// Set-Cookie: CookieName=chunks-3; path=/
         /// Set-Cookie: CookieNameC1=Segment1; path=/
         /// Set-Cookie: CookieNameC2=Segment2; path=/
         /// Set-Cookie: CookieNameC3=Segment3; path=/
@@ -149,9 +140,7 @@ public void AppendResponseCookie(HttpContext context, string key, string value,
                 throw new ArgumentNullException(nameof(options));
             }
 
-            var escapedKey = Encoder.Encode(key);
-
-            var template = new SetCookieHeaderValue(escapedKey)
+            var template = new SetCookieHeaderValue(key)
             {
                 Domain = options.Domain,
                 Expires = options.Expires,
@@ -163,22 +152,14 @@ public void AppendResponseCookie(HttpContext context, string key, string value,
             var templateLength = template.ToString().Length;
 
             value = value ?? string.Empty;
-            var quoted = false;
-            if (IsQuoted(value))
-            {
-                quoted = true;
-                value = RemoveQuotes(value);
-            }
-            var escapedValue = Encoder.Encode(value);
 
             // Normal cookie
-            var responseHeaders = context.Response.Headers;
-            if (!ChunkSize.HasValue || ChunkSize.Value > templateLength + escapedValue.Length + (quoted ? 2 : 0))
+            var responseCookies = context.Response.Cookies;
+            if (!ChunkSize.HasValue || ChunkSize.Value > templateLength + value.Length)
             {
-                template.Value = quoted ? Quote(escapedValue) : escapedValue;
-                responseHeaders.Append(Constants.Headers.SetCookie, template.ToString());
+                responseCookies.Append(key, value, options);
             }
-            else if (ChunkSize.Value < templateLength + (quoted ? 2 : 0) + 10)
+            else if (ChunkSize.Value < templateLength + 10)
             {
                 // 10 is the minimum data we want to put in an individual cookie, including the cookie chunk identifier "CXX".
                 // No room for data, we can't chunk the options and name
@@ -188,30 +169,25 @@ public void AppendResponseCookie(HttpContext context, string key, string value,
             {
                 // Break the cookie down into multiple cookies.
                 // Key = CookieName, value = "Segment1Segment2Segment2"
-                // Set-Cookie: CookieName=chunks:3; path=/
+                // Set-Cookie: CookieName=chunks-3; path=/
                 // Set-Cookie: CookieNameC1="Segment1"; path=/
                 // Set-Cookie: CookieNameC2="Segment2"; path=/
                 // Set-Cookie: CookieNameC3="Segment3"; path=/
-                var dataSizePerCookie = ChunkSize.Value - templateLength - (quoted ? 2 : 0) - 3; // Budget 3 chars for the chunkid.
-                var cookieChunkCount = (int)Math.Ceiling(escapedValue.Length * 1.0 / dataSizePerCookie);
+                var dataSizePerCookie = ChunkSize.Value - templateLength - 3; // Budget 3 chars for the chunkid.
+                var cookieChunkCount = (int)Math.Ceiling(value.Length * 1.0 / dataSizePerCookie);
 
-                template.Value = "chunks:" + cookieChunkCount.ToString(CultureInfo.InvariantCulture);
-                responseHeaders.Append(Constants.Headers.SetCookie, template.ToString());
+                responseCookies.Append(key, ChunkCountPrefix + cookieChunkCount.ToString(CultureInfo.InvariantCulture), options);
 
-                var chunks = new string[cookieChunkCount];
                 var offset = 0;
                 for (var chunkId = 1; chunkId <= cookieChunkCount; chunkId++)
                 {
-                    var remainingLength = escapedValue.Length - offset;
+                    var remainingLength = value.Length - offset;
                     var length = Math.Min(dataSizePerCookie, remainingLength);
-                    var segment = escapedValue.Substring(offset, length);
+                    var segment = value.Substring(offset, length);
                     offset += length;
 
-                    template.Name = escapedKey + "C" + chunkId.ToString(CultureInfo.InvariantCulture);
-                    template.Value = quoted ? Quote(segment) : segment;
-                    chunks[chunkId - 1] = template.ToString();
+                    responseCookies.Append(key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture), segment, options);
                 }
-                responseHeaders.Append(Constants.Headers.SetCookie, chunks);
             }
         }
 
@@ -239,17 +215,16 @@ public void DeleteCookie(HttpContext context, string key, CookieOptions options)
                 throw new ArgumentNullException(nameof(options));
             }
 
-            var escapedKey = Encoder.Encode(key);
             var keys = new List<string>();
-            keys.Add(escapedKey + "=");
+            keys.Add(key + "=");
 
             var requestCookie = context.Request.Cookies[key];
             var chunks = ParseChunksCount(requestCookie);
             if (chunks > 0)
             {
                 for (int i = 1; i <= chunks + 1; i++)
                 {
-                    var subkey = escapedKey + "C" + i.ToString(CultureInfo.InvariantCulture);
+                    var subkey = key + ChunkKeySuffix + i.ToString(CultureInfo.InvariantCulture);
                     keys.Add(subkey + "=");
                 }
             }
@@ -304,35 +279,5 @@ public void DeleteCookie(HttpContext context, string key, CookieOptions options)
                     });
             }
         }
-
-        private static bool IsQuoted(string value)
-        {
-            if (value == null)
-            {
-                throw new ArgumentNullException(nameof(value));
-            }
-
-            return value.Length >= 2 && value[0] == '"' && value[value.Length - 1] == '"';
-        }
-
-        private static string RemoveQuotes(string value)
-        {
-            if (value == null)
-            {
-                throw new ArgumentNullException(nameof(value));
-            }
-
-            return value.Substring(1, value.Length - 2);
-        }
-
-        private static string Quote(string value)
-        {
-            if (value == null)
-            {
-                throw new ArgumentNullException(nameof(value));
-            }
-
-            return '"' + value + '"';
-        }
     }
 }

src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationMiddleware.cs

@@ -42,7 +42,7 @@ public CookieAuthenticationMiddleware(
             }
             if (Options.CookieManager == null)
             {
-                Options.CookieManager = new ChunkingCookieManager(urlEncoder);
+                Options.CookieManager = new ChunkingCookieManager();
             }
             if (!Options.LoginPath.HasValue)
             {

test/Microsoft.AspNetCore.Authentication.Test/Cookies/Infrastructure/CookieChunkingTests.cs

@@ -15,7 +15,7 @@ public void AppendLargeCookie_Appended()
             HttpContext context = new DefaultHttpContext();
 
             string testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
-            new ChunkingCookieManager(null) { ChunkSize = null }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions());
+            new ChunkingCookieManager() { ChunkSize = null }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions());
             var values = context.Response.Headers["Set-Cookie"];
             Assert.Equal(1, values.Count);
             Assert.Equal("TestCookie=" + testString + "; path=/", values[0]);
@@ -27,12 +27,12 @@ public void AppendLargeCookieWithLimit_Chunked()
             HttpContext context = new DefaultHttpContext();
 
             string testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
-            new ChunkingCookieManager(null) { ChunkSize = 30 }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions());
+            new ChunkingCookieManager() { ChunkSize = 30 }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions());
             var values = context.Response.Headers["Set-Cookie"];
             Assert.Equal(9, values.Count);
             Assert.Equal<string[]>(new[]
             {
-                "TestCookie=chunks:8; path=/",
+                "TestCookie=chunks-8; path=/",
                 "TestCookieC1=abcdefgh; path=/",
                 "TestCookieC2=ijklmnop; path=/",
                 "TestCookieC3=qrstuvwx; path=/",
@@ -44,36 +44,13 @@ public void AppendLargeCookieWithLimit_Chunked()
             }, values);
         }
 
-        [Fact]
-        public void AppendLargeQuotedCookieWithLimit_QuotedChunked()
-        {
-            HttpContext context = new DefaultHttpContext();
-
-            string testString = "\"abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ\"";
-            new ChunkingCookieManager(null) { ChunkSize = 32 }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions());
-            var values = context.Response.Headers["Set-Cookie"];
-            Assert.Equal(9, values.Count);
-            Assert.Equal<string[]>(new[]
-            {
-                "TestCookie=chunks:8; path=/",
-                "TestCookieC1=\"abcdefgh\"; path=/",
-                "TestCookieC2=\"ijklmnop\"; path=/",
-                "TestCookieC3=\"qrstuvwx\"; path=/",
-                "TestCookieC4=\"yz012345\"; path=/",
-                "TestCookieC5=\"6789ABCD\"; path=/",
-                "TestCookieC6=\"EFGHIJKL\"; path=/",
-                "TestCookieC7=\"MNOPQRST\"; path=/",
-                "TestCookieC8=\"UVWXYZ\"; path=/",
-            }, values);
-        }
-
         [Fact]
         public void GetLargeChunkedCookie_Reassembled()
         {
             HttpContext context = new DefaultHttpContext();
             context.Request.Headers["Cookie"] = new[]
             {
-                "TestCookie=chunks:7",
+                "TestCookie=chunks-7",
                 "TestCookieC1=abcdefghi",
                 "TestCookieC2=jklmnopqr",
                 "TestCookieC3=stuvwxyz0",
@@ -83,39 +60,18 @@ public void GetLargeChunkedCookie_Reassembled()
                 "TestCookieC7=STUVWXYZ"
             };
 
-            string result = new ChunkingCookieManager(null).GetRequestCookie(context, "TestCookie");
+            string result = new ChunkingCookieManager().GetRequestCookie(context, "TestCookie");
             string testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
             Assert.Equal(testString, result);
         }
 
-        [Fact]
-        public void GetLargeChunkedCookieWithQuotes_Reassembled()
-        {
-            HttpContext context = new DefaultHttpContext();
-            context.Request.Headers["Cookie"] = new[]
-            {
-                "TestCookie=chunks:7",
-                "TestCookieC1=\"abcdefghi\"",
-                "TestCookieC2=\"jklmnopqr\"",
-                "TestCookieC3=\"stuvwxyz0\"",
-                "TestCookieC4=\"123456789\"",
-                "TestCookieC5=\"ABCDEFGHI\"",
-                "TestCookieC6=\"JKLMNOPQR\"",
-                "TestCookieC7=\"STUVWXYZ\""
-            };
-
-            string result = new ChunkingCookieManager(null).GetRequestCookie(context, "TestCookie");
-            string testString = "\"abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ\"";
-            Assert.Equal(testString, result);
-        }
-
         [Fact]
         public void GetLargeChunkedCookieWithMissingChunk_ThrowingEnabled_Throws()
         {
             HttpContext context = new DefaultHttpContext();
             context.Request.Headers["Cookie"] = new[]
             {
-                "TestCookie=chunks:7",
+                "TestCookie=chunks-7",
                 "TestCookieC1=abcdefghi",
                 // Missing chunk "TestCookieC2=jklmnopqr",
                 "TestCookieC3=stuvwxyz0",
@@ -125,7 +81,7 @@ public void GetLargeChunkedCookieWithMissingChunk_ThrowingEnabled_Throws()
                 "TestCookieC7=STUVWXYZ"
             };
 
-            Assert.Throws<FormatException>(() => new ChunkingCookieManager(null).GetRequestCookie(context, "TestCookie"));
+            Assert.Throws<FormatException>(() => new ChunkingCookieManager().GetRequestCookie(context, "TestCookie"));
         }
 
         [Fact]
@@ -134,7 +90,7 @@ public void GetLargeChunkedCookieWithMissingChunk_ThrowingDisabled_NotReassemble
             HttpContext context = new DefaultHttpContext();
             context.Request.Headers["Cookie"] = new[]
             {
-                "TestCookie=chunks:7",
+                "TestCookie=chunks-7",
                 "TestCookieC1=abcdefghi",
                 // Missing chunk "TestCookieC2=jklmnopqr",
                 "TestCookieC3=stuvwxyz0",
@@ -144,18 +100,18 @@ public void GetLargeChunkedCookieWithMissingChunk_ThrowingDisabled_NotReassemble
                 "TestCookieC7=STUVWXYZ"
             };
 
-            string result = new ChunkingCookieManager(null) { ThrowForPartialCookies = false }.GetRequestCookie(context, "TestCookie");
-            string testString = "chunks:7";
+            string result = new ChunkingCookieManager() { ThrowForPartialCookies = false }.GetRequestCookie(context, "TestCookie");
+            string testString = "chunks-7";
             Assert.Equal(testString, result);
         }
 
         [Fact]
         public void DeleteChunkedCookieWithOptions_AllDeleted()
         {
             HttpContext context = new DefaultHttpContext();
-            context.Request.Headers.Append("Cookie", "TestCookie=chunks:7");
+            context.Request.Headers.Append("Cookie", "TestCookie=chunks-7");
 
-            new ChunkingCookieManager(null).DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com" });
+            new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com" });
             var cookies = context.Response.Headers["Set-Cookie"];
             Assert.Equal(8, cookies.Count);
             Assert.Equal(new[]