From 8182bb16a9e2baf3d656b2212dd458689c313b99 Mon Sep 17 00:00:00 2001 From: Chris Ross Date: Thu, 2 Jul 2020 16:20:55 -0700 Subject: [PATCH] Disable cookie name encoding/decoding. (#23579) --- .../src/Internal/RequestCookieCollection.cs | 5 +- src/Http/Http/src/Internal/ResponseCookies.cs | 9 ++-- .../test/Internal/DefaultHttpRequestTests.cs | 4 +- .../test/RequestCookiesCollectionTests.cs | 42 +++++++-------- src/Http/Http/test/ResponseCookiesTest.cs | 54 ++++++++++++------- 5 files changed, 67 insertions(+), 47 deletions(-) diff --git a/src/Http/Http/src/Internal/RequestCookieCollection.cs b/src/Http/Http/src/Internal/RequestCookieCollection.cs index 6c74a6769b..d6542a23d7 100644 --- a/src/Http/Http/src/Internal/RequestCookieCollection.cs +++ b/src/Http/Http/src/Internal/RequestCookieCollection.cs @@ -57,6 +57,9 @@ namespace Microsoft.AspNetCore.Http } public static RequestCookieCollection Parse(IList values) + => ParseInternal(values, AppContext.TryGetSwitch(ResponseCookies.EnableCookieNameEncoding, out var enabled) && enabled); + + internal static RequestCookieCollection ParseInternal(IList values, bool enableCookieNameEncoding) { if (values.Count == 0) { @@ -75,7 +78,7 @@ namespace Microsoft.AspNetCore.Http for (var i = 0; i < cookies.Count; i++) { var cookie = cookies[i]; - var name = Uri.UnescapeDataString(cookie.Name.Value); + var name = enableCookieNameEncoding ? Uri.UnescapeDataString(cookie.Name.Value) : cookie.Name.Value; var value = Uri.UnescapeDataString(cookie.Value.Value); store[name] = value; } diff --git a/src/Http/Http/src/Internal/ResponseCookies.cs b/src/Http/Http/src/Internal/ResponseCookies.cs index 1de354e342..d9adfb69f1 100644 --- a/src/Http/Http/src/Internal/ResponseCookies.cs +++ b/src/Http/Http/src/Internal/ResponseCookies.cs @@ -13,6 +13,9 @@ namespace Microsoft.AspNetCore.Http /// internal class ResponseCookies : IResponseCookies { + internal const string EnableCookieNameEncoding = "Microsoft.AspNetCore.Http.EnableCookieNameEncoding"; + internal bool _enableCookieNameEncoding = AppContext.TryGetSwitch(EnableCookieNameEncoding, out var enabled) && enabled; + /// /// Create a new wrapper. /// @@ -33,7 +36,7 @@ namespace Microsoft.AspNetCore.Http public void Append(string key, string value) { var setCookieHeaderValue = new SetCookieHeaderValue( - Uri.EscapeDataString(key), + _enableCookieNameEncoding ? Uri.EscapeDataString(key) : key, Uri.EscapeDataString(value)) { Path = "/" @@ -52,7 +55,7 @@ namespace Microsoft.AspNetCore.Http } var setCookieHeaderValue = new SetCookieHeaderValue( - Uri.EscapeDataString(key), + _enableCookieNameEncoding ? Uri.EscapeDataString(key) : key, Uri.EscapeDataString(value)) { Domain = options.Domain, @@ -83,7 +86,7 @@ namespace Microsoft.AspNetCore.Http throw new ArgumentNullException(nameof(options)); } - var encodedKeyPlusEquals = Uri.EscapeDataString(key) + "="; + var encodedKeyPlusEquals = (_enableCookieNameEncoding ? Uri.EscapeDataString(key) : key) + "="; bool domainHasValue = !string.IsNullOrEmpty(options.Domain); bool pathHasValue = !string.IsNullOrEmpty(options.Path); diff --git a/src/Http/Http/test/Internal/DefaultHttpRequestTests.cs b/src/Http/Http/test/Internal/DefaultHttpRequestTests.cs index da9a846277..ddd17f143d 100644 --- a/src/Http/Http/test/Internal/DefaultHttpRequestTests.cs +++ b/src/Http/Http/test/Internal/DefaultHttpRequestTests.cs @@ -175,7 +175,7 @@ namespace Microsoft.AspNetCore.Http Assert.Null(cookies0["key0"]); Assert.False(cookies0.ContainsKey("key0")); - var newCookies = new[] { "name0=value0%2C", "%5Ename1=value1" }; + var newCookies = new[] { "name0=value0%2C", "name1=value1" }; request.Headers["Cookie"] = newCookies; cookies0 = RequestCookieCollection.Parse(newCookies); @@ -183,7 +183,7 @@ namespace Microsoft.AspNetCore.Http Assert.Equal(cookies0, cookies1); Assert.Equal(2, cookies1.Count); Assert.Equal("value0,", cookies1["name0"]); - Assert.Equal("value1", cookies1["^name1"]); + Assert.Equal("value1", cookies1["name1"]); Assert.Equal(newCookies, request.Headers["Cookie"]); var cookies2 = new RequestCookieCollection(new Dictionary() diff --git a/src/Http/Http/test/RequestCookiesCollectionTests.cs b/src/Http/Http/test/RequestCookiesCollectionTests.cs index 0d3cf069ad..8d3dad77f6 100644 --- a/src/Http/Http/test/RequestCookiesCollectionTests.cs +++ b/src/Http/Http/test/RequestCookiesCollectionTests.cs @@ -9,28 +9,13 @@ namespace Microsoft.AspNetCore.Http.Tests { public class RequestCookiesCollectionTests { - public static TheoryData UnEscapesKeyValues_Data - { - get - { - // key, value, expected - return new TheoryData - { - { "key=value", "key", "value" }, - { "key%2C=%21value", "key,", "!value" }, - { "ke%23y%2C=val%5Eue", "ke#y,", "val^ue" }, - { "base64=QUI%2BREU%2FRw%3D%3D", "base64", "QUI+REU/Rw==" }, - { "base64=QUI+REU/Rw==", "base64", "QUI+REU/Rw==" }, - }; - } - } - [Theory] - [MemberData(nameof(UnEscapesKeyValues_Data))] - public void UnEscapesKeyValues( - string input, - string expectedKey, - string expectedValue) + [InlineData("key=value", "key", "value")] + [InlineData("key%2C=%21value", "key%2C", "!value")] + [InlineData("ke%23y%2C=val%5Eue", "ke%23y%2C", "val^ue")] + [InlineData("base64=QUI%2BREU%2FRw%3D%3D", "base64", "QUI+REU/Rw==")] + [InlineData("base64=QUI+REU/Rw==", "base64", "QUI+REU/Rw==")] + public void UnEscapesValues(string input, string expectedKey, string expectedValue) { var cookies = RequestCookieCollection.Parse(new StringValues(input)); @@ -38,5 +23,20 @@ namespace Microsoft.AspNetCore.Http.Tests Assert.Equal(expectedKey, cookies.Keys.Single()); Assert.Equal(expectedValue, cookies[expectedKey]); } + + [Theory] + [InlineData("key=value", "key", "value")] + [InlineData("key%2C=%21value", "key,", "!value")] + [InlineData("ke%23y%2C=val%5Eue", "ke#y,", "val^ue")] + [InlineData("base64=QUI%2BREU%2FRw%3D%3D", "base64", "QUI+REU/Rw==")] + [InlineData("base64=QUI+REU/Rw==", "base64", "QUI+REU/Rw==")] + public void AppContextSwitchUnEscapesKeysAndValues(string input, string expectedKey, string expectedValue) + { + var cookies = RequestCookieCollection.ParseInternal(new StringValues(input), enableCookieNameEncoding: true); + + Assert.Equal(1, cookies.Count); + Assert.Equal(expectedKey, cookies.Keys.Single()); + Assert.Equal(expectedValue, cookies[expectedKey]); + } } } diff --git a/src/Http/Http/test/ResponseCookiesTest.cs b/src/Http/Http/test/ResponseCookiesTest.cs index 23eea058de..8e4e60aaa9 100644 --- a/src/Http/Http/test/ResponseCookiesTest.cs +++ b/src/Http/Http/test/ResponseCookiesTest.cs @@ -88,31 +88,45 @@ namespace Microsoft.AspNetCore.Http.Tests Assert.Contains($"max-age={maxAgeTime.TotalSeconds.ToString()}", cookieHeaderValues[0]); } - public static TheoryData EscapesKeyValuesBeforeSettingCookieData - { - get - { - // key, value, object pool, expected - return new TheoryData - { - { "key", "value", "key=value" }, - { "key,", "!value", "key%2C=%21value" }, - { "ke#y,", "val^ue", "ke%23y%2C=val%5Eue" }, - { "base64", "QUI+REU/Rw==", "base64=QUI%2BREU%2FRw%3D%3D" }, - }; - } - } - [Theory] - [MemberData(nameof(EscapesKeyValuesBeforeSettingCookieData))] - public void EscapesKeyValuesBeforeSettingCookie( - string key, - string value, - string expected) + [InlineData("value", "key=value")] + [InlineData("!value", "key=%21value")] + [InlineData("val^ue", "key=val%5Eue")] + [InlineData("QUI+REU/Rw==", "key=QUI%2BREU%2FRw%3D%3D")] + public void EscapesValuesBeforeSettingCookie(string value, string expected) { var headers = new HeaderDictionary(); var cookies = new ResponseCookies(headers); + cookies.Append("key", value); + + var cookieHeaderValues = headers[HeaderNames.SetCookie]; + Assert.Single(cookieHeaderValues); + Assert.StartsWith(expected, cookieHeaderValues[0]); + } + + [Theory] + [InlineData("key,")] + [InlineData("ke@y")] + public void InvalidKeysThrow(string key) + { + var headers = new HeaderDictionary(); + var cookies = new ResponseCookies(headers); + + Assert.Throws(() => cookies.Append(key, "1")); + } + + [Theory] + [InlineData("key", "value", "key=value")] + [InlineData("key,", "!value", "key%2C=%21value")] + [InlineData("ke#y,", "val^ue", "ke%23y%2C=val%5Eue")] + [InlineData("base64", "QUI+REU/Rw==", "base64=QUI%2BREU%2FRw%3D%3D")] + public void AppContextSwitchEscapesKeysAndValuesBeforeSettingCookie(string key, string value, string expected) + { + var headers = new HeaderDictionary(); + var cookies = new ResponseCookies(headers); + cookies._enableCookieNameEncoding = true; + cookies.Append(key, value); var cookieHeaderValues = headers[HeaderNames.SetCookie];