Skip to content

Commit

Permalink
Remove cookie name decoding (#24264)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tratcher committed Aug 13, 2020
1 parent 0a0e1ad commit 3e29d0b
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 26 deletions.
1 change: 1 addition & 0 deletions eng/PatchConfig.props
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ Later on, this will be checked using this condition:
</PropertyGroup>
<PropertyGroup Condition=" '$(VersionPrefix)' == '2.1.22' ">
<PackagesInPatch>
Microsoft.AspNetCore.Http;
</PackagesInPatch>
</PropertyGroup>
</Project>
26 changes: 20 additions & 6 deletions src/Http/Http/src/Internal/RequestCookieCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ namespace Microsoft.AspNetCore.Http.Internal
{
public class RequestCookieCollection : IRequestCookieCollection
{
private const string EnableCookieNameDecoding = "Microsoft.AspNetCore.Http.EnableCookieNameDecoding";
private bool _enableCookieNameDecoding;

public static readonly RequestCookieCollection Empty = new RequestCookieCollection();
private static readonly string[] EmptyKeys = Array.Empty<string>();
private static readonly Enumerator EmptyEnumerator = new Enumerator();
Expand All @@ -21,14 +24,15 @@ public class RequestCookieCollection : IRequestCookieCollection

public RequestCookieCollection()
{
_enableCookieNameDecoding = AppContext.TryGetSwitch(EnableCookieNameDecoding, out var enabled) && enabled;
}

public RequestCookieCollection(Dictionary<string, string> store)
public RequestCookieCollection(Dictionary<string, string> store) : this()
{
Store = store;
}

public RequestCookieCollection(int capacity)
public RequestCookieCollection(int capacity) : this()
{
Store = new Dictionary<string, string>(capacity, StringComparer.OrdinalIgnoreCase);
}
Expand Down Expand Up @@ -57,6 +61,9 @@ public RequestCookieCollection(int capacity)
}

public static RequestCookieCollection Parse(IList<string> values)
=> ParseInternal(values, AppContext.TryGetSwitch(EnableCookieNameDecoding, out var enabled) && enabled);

internal static RequestCookieCollection ParseInternal(IList<string> values, bool enableCookieNameDecoding)
{
if (values.Count == 0)
{
Expand All @@ -76,7 +83,11 @@ public static RequestCookieCollection Parse(IList<string> values)
for (var i = 0; i < cookies.Count; i++)
{
var cookie = cookies[i];
var name = Uri.UnescapeDataString(cookie.Name.Value);
var name = cookie.Name.Value;
if (enableCookieNameDecoding)
{
name = Uri.UnescapeDataString(name);
}
var value = Uri.UnescapeDataString(cookie.Value.Value);
store[name] = value;
}
Expand Down Expand Up @@ -116,7 +127,8 @@ public bool ContainsKey(string key)
{
return false;
}
return Store.ContainsKey(key);
return Store.ContainsKey(key)
|| !_enableCookieNameDecoding && Store.ContainsKey(Uri.EscapeDataString(key));
}

public bool TryGetValue(string key, out string value)
Expand All @@ -126,7 +138,9 @@ public bool TryGetValue(string key, out string value)
value = null;
return false;
}
return Store.TryGetValue(key, out value);

return Store.TryGetValue(key, out value)
|| !_enableCookieNameDecoding && Store.TryGetValue(Uri.EscapeDataString(key), out value);
}

/// <summary>
Expand Down Expand Up @@ -229,4 +243,4 @@ public void Reset()
}
}
}
}
}
3 changes: 3 additions & 0 deletions src/Http/Http/src/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Http.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
43 changes: 23 additions & 20 deletions src/Http/Http/test/RequestCookiesCollectionTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// 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.Internal;
using Microsoft.Extensions.Primitives;
Expand All @@ -10,30 +11,32 @@ namespace Microsoft.AspNetCore.Http.Tests
{
public class RequestCookiesCollectionTests
{
public static TheoryData UnEscapesKeyValues_Data
[Theory]
[InlineData("key=value", "key", "value")]
[InlineData("__secure-key=value", "__secure-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 UnEscapesValues(string input, string expectedKey, string expectedValue)
{
get
{
// key, value, expected
return new TheoryData<string, string, string>
{
{ "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==" },
};
}
var cookies = RequestCookieCollection.Parse(new StringValues(input));

Assert.Equal(1, cookies.Count);
Assert.Equal(Uri.EscapeDataString(expectedKey), cookies.Keys.Single());
Assert.Equal(expectedValue, cookies[expectedKey]);
}

[Theory]
[MemberData(nameof(UnEscapesKeyValues_Data))]
public void UnEscapesKeyValues(
string input,
string expectedKey,
string expectedValue)
[InlineData("key=value", "key", "value")]
[InlineData("__secure-key=value", "__secure-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 AppContextSwitchUnEscapesKeyValues(string input, string expectedKey, string expectedValue)
{
var cookies = RequestCookieCollection.Parse(new StringValues(input));
var cookies = RequestCookieCollection.ParseInternal(new StringValues(input), enableCookieNameDecoding: true);

Assert.Equal(1, cookies.Count);
Assert.Equal(expectedKey, cookies.Keys.Single());
Expand Down

1 comment on commit 3e29d0b

@davidfowl
Copy link
Member

@davidfowl davidfowl commented on 3e29d0b Aug 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding app context switches in master? Was this from a merge?

Please sign in to comment.