From 038557748532e6cc0eeff304ab5cbd7f147f7e72 Mon Sep 17 00:00:00 2001 From: wcontayon Date: Fri, 21 May 2021 11:30:22 +0200 Subject: [PATCH 1/5] Edit cookie reject predicate --- src/Http/Http/src/Internal/ResponseCookies.cs | 16 +++++++++--- src/Http/Http/test/ResponseCookiesTest.cs | 26 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Http/Http/src/Internal/ResponseCookies.cs b/src/Http/Http/src/Internal/ResponseCookies.cs index 26c1de655ebb..72954898a6e9 100644 --- a/src/Http/Http/src/Internal/ResponseCookies.cs +++ b/src/Http/Http/src/Internal/ResponseCookies.cs @@ -154,17 +154,25 @@ public void Delete(string key, CookieOptions options) } var encodedKeyPlusEquals = (_enableCookieNameEncoding ? Uri.EscapeDataString(key) : key) + "="; - bool domainHasValue = !string.IsNullOrEmpty(options.Domain); - bool pathHasValue = !string.IsNullOrEmpty(options.Path); + bool domainAndPathHasValue = !string.IsNullOrEmpty(options.Domain) && !string.IsNullOrEmpty(options.Path); + bool domainOnlyHasValue = !string.IsNullOrEmpty(options.Domain) && string.IsNullOrEmpty(options.Path); + bool pathOnlyHasValue = !string.IsNullOrEmpty(options.Path) && string.IsNullOrEmpty(options.Domain); Func rejectPredicate; - if (domainHasValue) + if (domainAndPathHasValue) + { + rejectPredicate = (value, encKeyPlusEquals, opts) => + value.StartsWith(encKeyPlusEquals, StringComparison.OrdinalIgnoreCase) && + value.IndexOf($"domain={opts.Domain}", StringComparison.OrdinalIgnoreCase) != -1 && + value.IndexOf($"path={opts.Path}", StringComparison.OrdinalIgnoreCase) != -1; + } + else if (domainOnlyHasValue) { rejectPredicate = (value, encKeyPlusEquals, opts) => value.StartsWith(encKeyPlusEquals, StringComparison.OrdinalIgnoreCase) && value.IndexOf($"domain={opts.Domain}", StringComparison.OrdinalIgnoreCase) != -1; } - else if (pathHasValue) + else if (pathOnlyHasValue) { rejectPredicate = (value, encKeyPlusEquals, opts) => value.StartsWith(encKeyPlusEquals, StringComparison.OrdinalIgnoreCase) && diff --git a/src/Http/Http/test/ResponseCookiesTest.cs b/src/Http/Http/test/ResponseCookiesTest.cs index a0a224b29fd9..03a1ce1bbd7e 100644 --- a/src/Http/Http/test/ResponseCookiesTest.cs +++ b/src/Http/Http/test/ResponseCookiesTest.cs @@ -74,6 +74,32 @@ public void DeleteCookieShouldSetDefaultPath() Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookieHeaderValues[0]); } + [Fact] + public void DeleteCookieWithDomainAndPathShouldSetDefaultPath() + { + var headers = (IHeaderDictionary)new HeaderDictionary(); + var features = MakeFeatures(headers); + var responseCookies = new ResponseCookies(features); + + var testCookies = new (string key, string path, string domaine)[] + { + new ("key1", "/", null), + new ("key1", "/test/", null), + new ("key2", "/", "localhost"), + new ("key2", "/test/", "localhost"), + }; + + foreach (var cookie in testCookies) + { + responseCookies.Delete(cookie.key, new CookieOptions() { Domain = cookie.domaine, Path = cookie.path }); + } + + var cookieHeaderValues = headers.SetCookie.ToArray(); + Assert.True(cookieHeaderValues.Length == testCookies.Length); + Assert.All(cookieHeaderValues, cookie => Assert.Contains("path=/", cookie)); + Assert.All(cookieHeaderValues, cookie => Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookie)); + } + [Fact] public void DeleteCookieWithCookieOptionsShouldKeepPropertiesOfCookieOptions() { From 874325390db576b2e2eaf23961960fbefbb8a8f6 Mon Sep 17 00:00:00 2001 From: wcontayon Date: Fri, 21 May 2021 18:52:19 +0200 Subject: [PATCH 2/5] Update src/Http/Http/test/ResponseCookiesTest.cs Co-authored-by: Brennan --- src/Http/Http/test/ResponseCookiesTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http/test/ResponseCookiesTest.cs b/src/Http/Http/test/ResponseCookiesTest.cs index 03a1ce1bbd7e..a3ab3312b27e 100644 --- a/src/Http/Http/test/ResponseCookiesTest.cs +++ b/src/Http/Http/test/ResponseCookiesTest.cs @@ -81,7 +81,7 @@ public void DeleteCookieWithDomainAndPathShouldSetDefaultPath() var features = MakeFeatures(headers); var responseCookies = new ResponseCookies(features); - var testCookies = new (string key, string path, string domaine)[] + var testCookies = new (string Key, string Path, string Domain)[] { new ("key1", "/", null), new ("key1", "/test/", null), From bd09954f47e1ff3de1df82eb4d0c5644d63ebe34 Mon Sep 17 00:00:00 2001 From: wcontayon Date: Fri, 21 May 2021 21:09:36 +0200 Subject: [PATCH 3/5] Update ResponseCookiesTest --- src/Http/Http/test/ResponseCookiesTest.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Http/Http/test/ResponseCookiesTest.cs b/src/Http/Http/test/ResponseCookiesTest.cs index a3ab3312b27e..871310e659bd 100644 --- a/src/Http/Http/test/ResponseCookiesTest.cs +++ b/src/Http/Http/test/ResponseCookiesTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Linq; using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -91,12 +92,17 @@ public void DeleteCookieWithDomainAndPathShouldSetDefaultPath() foreach (var cookie in testCookies) { - responseCookies.Delete(cookie.key, new CookieOptions() { Domain = cookie.domaine, Path = cookie.path }); + responseCookies.Delete(cookie.Key, new CookieOptions() { Domain = cookie.Domain, Path = cookie.Path }); } var cookieHeaderValues = headers.SetCookie.ToArray(); - Assert.True(cookieHeaderValues.Length == testCookies.Length); - Assert.All(cookieHeaderValues, cookie => Assert.Contains("path=/", cookie)); + Assert.Equal(testCookies.Length, cookieHeaderValues.Length); + + var deletedCookies = cookieHeaderValues.ToArray(); + Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/")); + Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/test/")); + Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/") && cookie.Contains("domain=localhost")); + Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/test/") && cookie.Contains("domain=localhost")); Assert.All(cookieHeaderValues, cookie => Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookie)); } From c2f5914ff6e5558e0b94c601a5fe8d7596df74a1 Mon Sep 17 00:00:00 2001 From: wcontayon Date: Sat, 22 May 2021 00:36:19 +0200 Subject: [PATCH 4/5] Improve unit test with specific path value --- src/Http/Http/src/Internal/ResponseCookies.cs | 11 +++++------ src/Http/Http/test/ResponseCookiesTest.cs | 18 +++++++++--------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/Http/Http/src/Internal/ResponseCookies.cs b/src/Http/Http/src/Internal/ResponseCookies.cs index 72954898a6e9..e55a008ed1c0 100644 --- a/src/Http/Http/src/Internal/ResponseCookies.cs +++ b/src/Http/Http/src/Internal/ResponseCookies.cs @@ -154,25 +154,24 @@ public void Delete(string key, CookieOptions options) } var encodedKeyPlusEquals = (_enableCookieNameEncoding ? Uri.EscapeDataString(key) : key) + "="; - bool domainAndPathHasValue = !string.IsNullOrEmpty(options.Domain) && !string.IsNullOrEmpty(options.Path); - bool domainOnlyHasValue = !string.IsNullOrEmpty(options.Domain) && string.IsNullOrEmpty(options.Path); - bool pathOnlyHasValue = !string.IsNullOrEmpty(options.Path) && string.IsNullOrEmpty(options.Domain); + bool domainHasValue = !string.IsNullOrEmpty(options.Domain); + bool pathHasValue = !string.IsNullOrEmpty(options.Path); Func rejectPredicate; - if (domainAndPathHasValue) + if (domainHasValue && pathHasValue) { rejectPredicate = (value, encKeyPlusEquals, opts) => value.StartsWith(encKeyPlusEquals, StringComparison.OrdinalIgnoreCase) && value.IndexOf($"domain={opts.Domain}", StringComparison.OrdinalIgnoreCase) != -1 && value.IndexOf($"path={opts.Path}", StringComparison.OrdinalIgnoreCase) != -1; } - else if (domainOnlyHasValue) + else if (domainHasValue) { rejectPredicate = (value, encKeyPlusEquals, opts) => value.StartsWith(encKeyPlusEquals, StringComparison.OrdinalIgnoreCase) && value.IndexOf($"domain={opts.Domain}", StringComparison.OrdinalIgnoreCase) != -1; } - else if (pathOnlyHasValue) + else if (pathHasValue) { rejectPredicate = (value, encKeyPlusEquals, opts) => value.StartsWith(encKeyPlusEquals, StringComparison.OrdinalIgnoreCase) && diff --git a/src/Http/Http/test/ResponseCookiesTest.cs b/src/Http/Http/test/ResponseCookiesTest.cs index 871310e659bd..4522deebefb0 100644 --- a/src/Http/Http/test/ResponseCookiesTest.cs +++ b/src/Http/Http/test/ResponseCookiesTest.cs @@ -76,7 +76,7 @@ public void DeleteCookieShouldSetDefaultPath() } [Fact] - public void DeleteCookieWithDomainAndPathShouldSetDefaultPath() + public void DeleteCookieWithDomainAndPathDeletesPriorMatchingCookies() { var headers = (IHeaderDictionary)new HeaderDictionary(); var features = MakeFeatures(headers); @@ -84,10 +84,10 @@ public void DeleteCookieWithDomainAndPathShouldSetDefaultPath() var testCookies = new (string Key, string Path, string Domain)[] { - new ("key1", "/", null), - new ("key1", "/test/", null), - new ("key2", "/", "localhost"), - new ("key2", "/test/", "localhost"), + new ("key1", "/path1/", null), + new ("key1", "/path2/", null), + new ("key2", "/path1/", "localhost"), + new ("key2", "/path2/", "localhost"), }; foreach (var cookie in testCookies) @@ -99,10 +99,10 @@ public void DeleteCookieWithDomainAndPathShouldSetDefaultPath() Assert.Equal(testCookies.Length, cookieHeaderValues.Length); var deletedCookies = cookieHeaderValues.ToArray(); - Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/")); - Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/test/")); - Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/") && cookie.Contains("domain=localhost")); - Assert.Contains(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/test/") && cookie.Contains("domain=localhost")); + Assert.Single(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/path1/")); + Assert.Single(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/path2/")); + Assert.Single(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/path1/") && cookie.Contains("domain=localhost")); + Assert.Single(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/path2/") && cookie.Contains("domain=localhost")); Assert.All(cookieHeaderValues, cookie => Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookie)); } From dc5178876192974b29070b48a48bcd5262832254 Mon Sep 17 00:00:00 2001 From: wcontayon Date: Sat, 22 May 2021 12:31:45 +0200 Subject: [PATCH 5/5] Test remove prior matching cookie with domain and path --- src/Http/Http/test/ResponseCookiesTest.cs | 35 ++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/Http/Http/test/ResponseCookiesTest.cs b/src/Http/Http/test/ResponseCookiesTest.cs index 4522deebefb0..9d520fe95f4c 100644 --- a/src/Http/Http/test/ResponseCookiesTest.cs +++ b/src/Http/Http/test/ResponseCookiesTest.cs @@ -95,15 +95,42 @@ public void DeleteCookieWithDomainAndPathDeletesPriorMatchingCookies() responseCookies.Delete(cookie.Key, new CookieOptions() { Domain = cookie.Domain, Path = cookie.Path }); } - var cookieHeaderValues = headers.SetCookie.ToArray(); - Assert.Equal(testCookies.Length, cookieHeaderValues.Length); + var deletedCookies = headers.SetCookie.ToArray(); + Assert.Equal(testCookies.Length, deletedCookies.Length); - var deletedCookies = cookieHeaderValues.ToArray(); Assert.Single(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/path1/")); Assert.Single(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/path2/")); Assert.Single(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/path1/") && cookie.Contains("domain=localhost")); Assert.Single(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/path2/") && cookie.Contains("domain=localhost")); - Assert.All(cookieHeaderValues, cookie => Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookie)); + Assert.All(deletedCookies, cookie => Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookie)); + } + + [Fact] + public void DeleteRemovesCookieWithDomainAndPathCreatedByAdd() + { + var headers = (IHeaderDictionary)new HeaderDictionary(); + var features = MakeFeatures(headers); + var responseCookies = new ResponseCookies(features); + + var testCookies = new (string Key, string Path, string Domain)[] + { + new ("key1", "/path1/", null), + new ("key1", "/path1/", null), + new ("key2", "/path1/", "localhost"), + new ("key2", "/path1/", "localhost"), + }; + + foreach (var cookie in testCookies) + { + responseCookies.Append(cookie.Key, cookie.Key, new CookieOptions() { Domain = cookie.Domain, Path = cookie.Path }); + responseCookies.Delete(cookie.Key, new CookieOptions() { Domain = cookie.Domain, Path = cookie.Path }); + } + + var deletedCookies = headers.SetCookie.ToArray(); + Assert.Equal(2, deletedCookies.Length); + Assert.Single(deletedCookies, cookie => cookie.StartsWith("key1", StringComparison.InvariantCulture) && cookie.Contains("path=/path1/")); + Assert.Single(deletedCookies, cookie => cookie.StartsWith("key2", StringComparison.InvariantCulture) && cookie.Contains("path=/path1/") && cookie.Contains("domain=localhost")); + Assert.All(deletedCookies, cookie => Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookie)); } [Fact]