Browse Source

Fix partial chunked cookies (#41646)

Chris Ross 3 years ago
parent
commit
395eeea8a9

+ 45 - 10
src/Security/CookiePolicy/test/CookieChunkingTests.cs

@@ -127,22 +127,57 @@ public class CookieChunkingTests
     public void DeleteChunkedCookieWithOptions_AllDeleted()
     {
         HttpContext context = new DefaultHttpContext();
-        context.Request.Headers.Append("Cookie", "TestCookie=chunks-7");
+        context.Request.Headers.Append("Cookie", "TestCookie=chunks-7;TestCookieC1=1;TestCookieC2=2;TestCookieC3=3;TestCookieC4=4;TestCookieC5=5;TestCookieC6=6;TestCookieC7=7");
 
         new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
         var cookies = context.Response.Headers["Set-Cookie"];
         Assert.Equal(8, cookies.Count);
         Assert.Equal(new[]
         {
-                "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-                "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-                "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-                "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-                "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-                "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-                "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-                "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
-            }, cookies);
+            "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+        }, cookies);
+    }
+
+    [Fact]
+    public void DeleteChunkedCookieWithMissingRequestCookies_OnlyPresentCookiesDeleted()
+    {
+        HttpContext context = new DefaultHttpContext();
+        context.Request.Headers.Append("Cookie", "TestCookie=chunks-7;TestCookieC1=1;TestCookieC2=2");
+
+        new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
+        var cookies = context.Response.Headers["Set-Cookie"];
+        Assert.Equal(3, cookies.Count);
+        Assert.Equal(new[]
+        {
+            "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+        }, cookies);
+    }
+
+    [Fact]
+    public void DeleteChunkedCookieWithMissingRequestCookies_StopsAtMissingChunk()
+    {
+        HttpContext context = new DefaultHttpContext();
+        // C3 is missing so we don't try to delete C4 either.
+        context.Request.Headers.Append("Cookie", "TestCookie=chunks-7;TestCookieC1=1;TestCookieC2=2;TestCookieC4=4");
+
+        new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
+        var cookies = context.Response.Headers["Set-Cookie"];
+        Assert.Equal(3, cookies.Count);
+        Assert.Equal(new[]
+        {
+            "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+            "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure",
+        }, cookies);
     }
 
     [Fact]

+ 16 - 7
src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs

@@ -103,7 +103,7 @@ internal sealed class ChunkingCookieManager
         var chunksCount = ParseChunksCount(value);
         if (chunksCount > 0)
         {
-            var chunks = new string[chunksCount];
+            var chunks = new List<string>(10); // The client may not have sent all of the chunks, don't allocate based on chunksCount.
             for (var chunkId = 1; chunkId <= chunksCount; chunkId++)
             {
                 var chunk = requestCookies[key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture)];
@@ -128,7 +128,7 @@ internal sealed class ChunkingCookieManager
                     return value;
                 }
 
-                chunks[chunkId - 1] = chunk;
+                chunks.Add(chunk);
             }
 
             return string.Join(string.Empty, chunks);
@@ -252,17 +252,26 @@ internal sealed class ChunkingCookieManager
         }
 
         var keys = new List<string>
-            {
-                key + "="
-            };
+        {
+            key + "="
+        };
 
-        var requestCookie = context.Request.Cookies[key];
-        var chunks = ParseChunksCount(requestCookie);
+        var requestCookies = context.Request.Cookies;
+        var requestCookie = requestCookies[key];
+        long chunks = ParseChunksCount(requestCookie);
         if (chunks > 0)
         {
             for (var i = 1; i <= chunks + 1; i++)
             {
                 var subkey = key + ChunkKeySuffix + i.ToString(CultureInfo.InvariantCulture);
+
+                // Only delete cookies we received. We received the chunk count cookie so we should have received the others too.
+                if (string.IsNullOrEmpty(requestCookies[subkey]))
+                {
+                    chunks = i - 1;
+                    break;
+                }
+
                 keys.Add(subkey + "=");
             }
         }