Handle mismatched cookie chunk counts (#463)

aspnet · May 10, 2022 · f5cf9c4 · f5cf9c4
1 parent 535e7e2
commit f5cf9c4
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 5 deletions.
diff --git a/src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs b/src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs
@@ -73,7 +73,7 @@ public string GetRequestCookie(IOwinContext context, string key)
             if (chunksCount > 0)
             {
                 bool quoted = false;
-                string[] chunks = new string[chunksCount];
+                var chunks = new List<string>(10); // chunksCount may be wrong, don't trust it.
                 for (int chunkId = 1; chunkId <= chunksCount; chunkId++)
                 {
                     string chunk = requestCookies[key + "C" + chunkId.ToString(CultureInfo.InvariantCulture)];
@@ -98,7 +98,8 @@ public string GetRequestCookie(IOwinContext context, string key)
                         quoted = true;
                         chunk = RemoveQuotes(chunk);
                     }
-                    chunks[chunkId - 1] = chunk;
+
+                    chunks.Add(chunk);
                 }
                 string merged = string.Join(string.Empty, chunks);
                 if (quoted)
@@ -236,13 +237,22 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
             List<string> keys = new List<string>();
             keys.Add(escapedKey + "=");
 
-            string requestCookie = context.Request.Cookies[key];
-            int chunks = ParseChunksCount(requestCookie);
+            var requestCookies = context.Request.Cookies;
+            var requestCookie = requestCookies[key];
+            long chunks = ParseChunksCount(requestCookie);
             if (chunks > 0)
             {
                 for (int i = 1; i <= chunks + 1; i++)
                 {
                     string subkey = escapedKey + "C" + 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 + "=");
                 }
             }

diff --git a/tests/Microsoft.Owin.Tests/CookieChunkingTests.cs b/tests/Microsoft.Owin.Tests/CookieChunkingTests.cs
@@ -146,7 +146,7 @@ public void GetLargeChunkedCookieWithMissingChunk_ThrowingDisabled_NotReassemble
         public void DeleteChunkedCookieWithOptions_AllDeleted()
         {
             IOwinContext context = new OwinContext();
-            context.Request.Headers.AppendValues("Cookie", "TestCookie=chunks:7");
+            context.Request.Headers.AppendValues("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" });
             var cookies = context.Response.Headers.GetValues("Set-Cookie");
@@ -163,5 +163,38 @@ public void DeleteChunkedCookieWithOptions_AllDeleted()
                 "TestCookieC7=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT",
             }, cookies);
         }
+
+        [Fact]
+        public void DeleteChunkedCookieWithMissingRequestCookies_OnlyPresentCookiesDeleted()
+        {
+            IOwinContext context = new OwinContext();
+            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.GetValues("Set-Cookie");
+            Assert.Equal(3, cookies.Count);
+            Assert.Equal(new[]
+            {
+                "TestCookie=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
+                "TestCookieC1=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
+                "TestCookieC2=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
+            }, cookies);
+        }
+
+        [Fact]
+        public void DeleteChunkedCookieWithMissingRequestCookies_StopsAtMissingChunk()
+        {
+            IOwinContext context = new OwinContext();
+            // 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.GetValues("Set-Cookie");
+            Assert.Equal(3, cookies.Count);
+            Assert.Equal(new[]
+            {
+                "TestCookie=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
+                "TestCookieC1=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
+                "TestCookieC2=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
+            }, cookies);
+        }
     }
 }