Delete multiple cookie with same-key when Path is different but Domain is specified #32897
Conversation
ghost
added
area-runtime
community-contribution
|
|
||
| var cookieHeaderValues = headers.SetCookie.ToArray(); | ||
| Assert.True(cookieHeaderValues.Length == testCookies.Length); | ||
| Assert.All(cookieHeaderValues, cookie => Assert.Contains("path=/", cookie)); |
There was a problem hiding this comment.
We should be verifying the full path and domain are set in the response
https://tools.ietf.org/search/rfc6265#section-3.1
Finally, to remove a cookie, the server returns a Set-Cookie header
with an expiration date in the past. The server will be successful
in removing the cookie only if the Path and the Domain attribute in
the Set-Cookie header match the values used when the cookie was
created.
Co-authored-by: Brennan <brecon@microsoft.com>
| Assert.Equal(testCookies.Length, cookieHeaderValues.Length); | ||
|
|
||
| var deletedCookies = cookieHeaderValues.ToArray(); | ||
| Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/")); |
There was a problem hiding this comment.
Can't test for just "path=/" because it matches both cookies, so we're not actually testing that both are there 😄
There was a problem hiding this comment.
Effectively 🙂.
In this case, we can improve the testing with specific path "/path1/", "path2/",and test the matching.
| { | ||
| rejectPredicate = (value, encKeyPlusEquals, opts) => | ||
| value.StartsWith(encKeyPlusEquals, StringComparison.OrdinalIgnoreCase) && | ||
| value.IndexOf($"domain={opts.Domain}", StringComparison.OrdinalIgnoreCase) != -1 && |
There was a problem hiding this comment.
nit: in the future we can consider removing these string allocs, don't change this PR
|
Thanks @wcontayon ! |
amcasey
added
area-networking
5 participants
PR Title
Fix multiple cookies deletion with same-key when Path is different but Domain is specified
PR Description
Update the cookie reject predicate to add a new condition that take into account the domain and the path.
(https://github.com/wcontayon/AspNetCore/blob/038557748532e6cc0eeff304ab5cbd7f147f7e72/src/Http/Http/src/Internal/ResponseCookies.cs#L162-L168)
Addresses #30579