Skip to content

Commit f5cf9c4

Browse files
authored
Handle mismatched cookie chunk counts (#463)
1 parent 535e7e2 commit f5cf9c4

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public string GetRequestCookie(IOwinContext context, string key)
7373
if (chunksCount > 0)
7474
{
7575
bool quoted = false;
76-
string[] chunks = new string[chunksCount];
76+
var chunks = new List<string>(10); // chunksCount may be wrong, don't trust it.
7777
for (int chunkId = 1; chunkId <= chunksCount; chunkId++)
7878
{
7979
string chunk = requestCookies[key + "C" + chunkId.ToString(CultureInfo.InvariantCulture)];
@@ -98,7 +98,8 @@ public string GetRequestCookie(IOwinContext context, string key)
9898
quoted = true;
9999
chunk = RemoveQuotes(chunk);
100100
}
101-
chunks[chunkId - 1] = chunk;
101+
102+
chunks.Add(chunk);
102103
}
103104
string merged = string.Join(string.Empty, chunks);
104105
if (quoted)
@@ -236,13 +237,22 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
236237
List<string> keys = new List<string>();
237238
keys.Add(escapedKey + "=");
238239

239-
string requestCookie = context.Request.Cookies[key];
240-
int chunks = ParseChunksCount(requestCookie);
240+
var requestCookies = context.Request.Cookies;
241+
var requestCookie = requestCookies[key];
242+
long chunks = ParseChunksCount(requestCookie);
241243
if (chunks > 0)
242244
{
243245
for (int i = 1; i <= chunks + 1; i++)
244246
{
245247
string subkey = escapedKey + "C" + i.ToString(CultureInfo.InvariantCulture);
248+
249+
// Only delete cookies we received. We received the chunk count cookie so we should have received the others too.
250+
if (string.IsNullOrEmpty(requestCookies[subkey]))
251+
{
252+
chunks = i - 1;
253+
break;
254+
}
255+
246256
keys.Add(subkey + "=");
247257
}
248258
}

tests/Microsoft.Owin.Tests/CookieChunkingTests.cs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public void GetLargeChunkedCookieWithMissingChunk_ThrowingDisabled_NotReassemble
146146
public void DeleteChunkedCookieWithOptions_AllDeleted()
147147
{
148148
IOwinContext context = new OwinContext();
149-
context.Request.Headers.AppendValues("Cookie", "TestCookie=chunks:7");
149+
context.Request.Headers.AppendValues("Cookie", "TestCookie=chunks:7;TestCookieC1=1;TestCookieC2=2;TestCookieC3=3;TestCookieC4=4;TestCookieC5=5;TestCookieC6=6;TestCookieC7=7");
150150

151151
new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com" });
152152
var cookies = context.Response.Headers.GetValues("Set-Cookie");
@@ -163,5 +163,38 @@ public void DeleteChunkedCookieWithOptions_AllDeleted()
163163
"TestCookieC7=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT",
164164
}, cookies);
165165
}
166+
167+
[Fact]
168+
public void DeleteChunkedCookieWithMissingRequestCookies_OnlyPresentCookiesDeleted()
169+
{
170+
IOwinContext context = new OwinContext();
171+
context.Request.Headers.Append("Cookie", "TestCookie=chunks:7;TestCookieC1=1;TestCookieC2=2");
172+
new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
173+
var cookies = context.Response.Headers.GetValues("Set-Cookie");
174+
Assert.Equal(3, cookies.Count);
175+
Assert.Equal(new[]
176+
{
177+
"TestCookie=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
178+
"TestCookieC1=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
179+
"TestCookieC2=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
180+
}, cookies);
181+
}
182+
183+
[Fact]
184+
public void DeleteChunkedCookieWithMissingRequestCookies_StopsAtMissingChunk()
185+
{
186+
IOwinContext context = new OwinContext();
187+
// C3 is missing so we don't try to delete C4 either.
188+
context.Request.Headers.Append("Cookie", "TestCookie=chunks:7;TestCookieC1=1;TestCookieC2=2;TestCookieC4=4");
189+
new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true });
190+
var cookies = context.Response.Headers.GetValues("Set-Cookie");
191+
Assert.Equal(3, cookies.Count);
192+
Assert.Equal(new[]
193+
{
194+
"TestCookie=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
195+
"TestCookieC1=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
196+
"TestCookieC2=; domain=foo.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT; secure",
197+
}, cookies);
198+
}
166199
}
167200
}

0 commit comments

Comments
 (0)