dotnet/systemweb-adapters

HttpCookie urlencodes cookie values

afshinm opened this issue · 5 comments

afshinm commented

Describe the bug

Create a new HttpCookie object in NET Standard / NET Core URL encodes the value of of cookie, whereas in NET Framework 4.7.2, this doesn't occur. This is probably the desired behavior but makes it difficult to share the same cookie between the two apps (NET Core and NET Framework)

To Reproduce

var cookie = new HttpCookie(cookieName, "a|b") { Secure = true, SameSite = SameSiteMode.None };
Console.WriteLine(cookie.Value); // prints a%7Cb

Exceptions (if any)

None

Further technical details

A solution is to manually create the Set-cookie header in the request payload but that's very error prone and hard to test.

Please include the following if applicable:

ASP.NET Framework Application:

  • Technologies and versions used (i.e. MVC/WebForms/etc): MVC
  • .NET Framework Version: 4.7.2
  • IIS Version: 10
  • Windows Version: 11

ASP.NET Core Application:

  • Targeted .NET version: .NET 6
joperezr commented

[Triage]: We looked at this during triage and this seems to be a bug in the implementation. https://github.com/dotnet/systemweb-adapters/blob/main/src/Microsoft.AspNetCore.SystemWebAdapters/HttpCookie.cs Seems like a good place to start for this investigation is checking the flags that may be causing the url encoding to happen here.

@afshinm would you be interested in taking a stab at a fix 😃?

afshinm commented

@joperezr Thanks for looking into this. Yes, more than happy to work on this.

Just to double check my understanding, do you agree that the cookie values shouldn't be URL encoded automatically? At least that's the behavior I'm seeing in Net Framework 4.7.2.

afshinm commented

Looking at this HttpCookie implementation https://github.com/microsoft/referencesource/blob/master/System.Web/HttpCookie.cs#L320 as a reference, I believe the problem is that we use the same property, _holder, to keep both string and collection values. Therefore, when the HttpCookie.Values property is called, the _holder data type changes and causes the URL encoding to happen.

afshinm commented

Just opened a PR to fix this. cc @joperezr @twsouthwick

  • ❤️2
twsouthwick commented

@joperezr Thanks for looking into this. Yes, more than happy to work on this.

Just to double check my understanding, do you agree that the cookie values shouldn't be URL encoded automatically? At least that's the behavior I'm seeing in Net Framework 4.7.2.

The goal of the adapters is to bring parity to what system.web was doing for code during migration. So, if we can make the adapters handle this similarly (i.e. not encode), sounds like it's in line with that goal

  • 👍1