Aspnetcore: Consider adding a version of Response.Cookies.Append that does not encode

Created on 27 Feb 2018  路  9Comments  路  Source: dotnet/aspnetcore

I have run into the scenario where I'm receiving cookies properly encoded from a third party system and when calling Response.Cookies.Append it is doing a second round of encoding.

Could you consider adding another property to CookieOptions to skip the encoding? Basically:

//inside a controller
Response.Cookies.Append("KeyEncoded", "ValueEncoded", new CookieOptions { Raw = true });

This Raw property would mean: trust me, the key and the value are already encoded.

area-servers bug feature-HttpAbstractions

Most helpful comment

I think there are scenarios where we would want to have a raw cookie added to the headers regardless. Take as an example a cookie with a value like:

name=jim&dob=30

This will be encoded to:

name%3Djim%26age%3D123

While the encoding here is correct, there is nothing malicious with this value and browsers and servers have been able to handle the not-encoded version correctly. It is specially useful for backward compatibility with ASP.NET (not Core) where cookies were not encoded automatically by the framework and some of these cookies that needed to be encoded were not (whether by oversight or misinformation about the risks.)

Forcing the encoding will break consumers of the cookie. Before they could just get the cookie value, now they would need to run it through decodeURIComponent(cookieValue).

All 9 comments

Can you propose an API? AppendRaw maybe? /cc @Tratcher

Can you give some examples of the input and output you see?

AppendRaw works too. In fact, my current workaround is an extension method on IHeaderDictionary called AppendRawCookie.

I kind of prefer to have the Raw flag in the CookieOptions because it keeps the Append methods simple and keeps this potentially risky option a bit buried, steering users to use the safe option.

On the other hand, I鈥檓 not very familiar with other usages of CookieOptions and if this extra property would make sense there.

Workaround: use the SetCookieHeaderValue type to generate the cookie and Response.Headers.Append to add it. This is roughly how the current version is implemented.

Here is the example you requested:

A cookie value that is actually a piece of JSON

Third party system returns cookie value encoded as:

%7B%22Name%22%3A%22Jim%22%7D // decoded: {"Name":"Jim"}

After calling Response.Cookies.Append on the already encoded cookie what gets sent to the browser is:

%257B%2522Name%2522%253A%2522Jim%2522%257D

The workaround that we have right now looks like this:

public static void AppendRawCookie(this IHeaderDictionary header, string key, string value, CookieOptions options)
{
    if (header == null)
        throw new ArgumentNullException(nameof(header));
    if (key == null)
        throw new ArgumentNullException(nameof(key));
    if (options == null)
        throw new ArgumentNullException(nameof(options));


    if (value == null)
        value = string.Empty;

    var setCookieHeaderValue = new SetCookieHeaderValue(key, value)
    {
        Domain = options.Domain,
        Path = options.Path,
        Expires = options.Expires,
        Secure = options.Secure,
        HttpOnly = options.HttpOnly
    };

    var cookieValue = setCookieHeaderValue.ToString();
    header[HeaderNames.SetCookie] = StringValues.Concat(header[HeaderNames.SetCookie], cookieValue);
}

The double encoding is a bug we should fix, %7B should not be encoded double encoded as %257B. If we fixed that would you still need this new raw API?

I think there are scenarios where we would want to have a raw cookie added to the headers regardless. Take as an example a cookie with a value like:

name=jim&dob=30

This will be encoded to:

name%3Djim%26age%3D123

While the encoding here is correct, there is nothing malicious with this value and browsers and servers have been able to handle the not-encoded version correctly. It is specially useful for backward compatibility with ASP.NET (not Core) where cookies were not encoded automatically by the framework and some of these cookies that needed to be encoded were not (whether by oversight or misinformation about the risks.)

Forcing the encoding will break consumers of the cookie. Before they could just get the cookie value, now they would need to run it through decodeURIComponent(cookieValue).

It's possible to workaround this by appending to the Set-Cookie header itself. We have no plans to add this API at this time.

Was this page helpful?
0 / 5 - 0 ratings