dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.25k stars 4.73k forks source link

CookieContainer doesn't replace Cookie with same name #84820

Open adjgam opened 1 year ago

adjgam commented 1 year ago

Description

CookieContainer doesn't replace cookie which added using CookieContainer.Add(Uri uri, Cookie cookie)

Reproduction Steps

  1. Add cookie to CookieContainer
  2. Do http/s request
  3. Receive cookie from web server with existing cookie name in CookieContainer

Expected behavior

New cookie received from web server must replace existing cookie with same name

Actual behavior

New cookie doesn't replace old cookie and we have two cookies with same name

Regression?

No response

Known Workarounds

No response

Configuration

Win 10 .Net 6.0.16 7.0.5 .Net Framework 4.7 4.8 4.8.1

Other information

static Task<HttpResponseMessage> HttpReq(CookieContainer cookieContainer)
{
    string url = "https://arduino.ua/";

    // Now create a client handler which uses that proxy
    var httpClientHandler = new HttpClientHandler
    {
        CookieContainer = cookieContainer,
        UseCookies = true
    };

    httpClientHandler.AllowAutoRedirect = true;

    var client = new HttpClient(
        handler: httpClientHandler,
        disposeHandler: true
        );

    client.Timeout = TimeSpan.FromSeconds(30);
    // add default headers
    client.DefaultRequestHeaders.TryAddWithoutValidation("Accept", "text/javascript, text/html, application/xml, text/xml, */*");
    client.DefaultRequestHeaders.TryAddWithoutValidation("User-Agent", "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/109.0");
    client.DefaultRequestHeaders.TryAddWithoutValidation("Accept-Encoding", "gzip, deflate, br");
    client.DefaultRequestHeaders.TryAddWithoutValidation("Accept-Language", "uk-UA,uk;q=0.8,en-US;q=0.5,en;q=0.3");

    var response = client.GetAsync(url);
    return response;

}

var cookieContainer = new CookieContainer();

Uri uri = new Uri("https://arduino.ua");
System.DateTime today = System.DateTime.Now;
System.DateTime Expires = today.AddYears(1);

Cookie cookie = new Cookie();
cookie.Expires = Expires;
cookie.HttpOnly = false;
cookie.Value = "nnnnn";
cookie.Name = "FBeID";
cookie.Domain = "arduino.ua";

cookieContainer.Add(uri, cookie);

var response = HttpReq(cookieContainer);
response.Result.Content.ReadAsStringAsync().Wait();

IEnumerable<Cookie> responseCookies = cookieContainer.GetCookies(uri).Cast<Cookie>();
foreach (Cookie new_cookie in responseCookies)
{
    Console.WriteLine(new_cookie.Name + "=" +new_cookie.Value);
}

request cookies: Cookie: FBeID=nnnnn

resonce cookies: Set-Cookie: PHPSESSID=eljpt8ef572ulpqn6bkv6io93h; expires=Thu, 18-Jan-2024 02:33:24 GMT; Max-Age=24105600; path=/ Set-Cookie: FBeID=1423878233

CookieContainer content: PHPSESSID=eljpt8ef572ulpqn6bkv6io93h FBeID=1423878233 FBeID=nnnnn

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

Issue Details
### Description CookieContainer doesn't replace cookie which added using `CookieContainer.Add(Uri uri, Cookie cookie)` ### Reproduction Steps 1. Add cookie to CookieContainer 2. Do http/s request 3. Receive cookie from web server with existing cookie name in CookieContainer ### Expected behavior New cookie received from web server must replace existing cookie with same name ### Actual behavior New cookie doesn't replace old cookie and we have two cookies with same name ### Regression? _No response_ ### Known Workarounds _No response_ ### Configuration Win 10 .Net 6.0.16 7.0.5 .Net Framework 4.7 4.8 4.8.1 ### Other information ``` static Task HttpReq(CookieContainer cookieContainer) { string url = "https://arduino.ua/"; // Now create a client handler which uses that proxy var httpClientHandler = new HttpClientHandler { CookieContainer = cookieContainer, UseCookies = true }; httpClientHandler.AllowAutoRedirect = true; var client = new HttpClient( handler: httpClientHandler, disposeHandler: true ); client.Timeout = TimeSpan.FromSeconds(30); // add default headers client.DefaultRequestHeaders.TryAddWithoutValidation("Accept", "text/javascript, text/html, application/xml, text/xml, */*"); client.DefaultRequestHeaders.TryAddWithoutValidation("User-Agent", "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/109.0"); client.DefaultRequestHeaders.TryAddWithoutValidation("Accept-Encoding", "gzip, deflate, br"); client.DefaultRequestHeaders.TryAddWithoutValidation("Accept-Language", "uk-UA,uk;q=0.8,en-US;q=0.5,en;q=0.3"); var response = client.GetAsync(url); return response; } var cookieContainer = new CookieContainer(); Uri uri = new Uri("https://arduino.ua"); System.DateTime today = System.DateTime.Now; System.DateTime Expires = today.AddYears(1); Cookie cookie = new Cookie(); cookie.Expires = Expires; cookie.HttpOnly = false; cookie.Value = "nnnnn"; cookie.Name = "FBeID"; cookie.Domain = "arduino.ua"; cookieContainer.Add(uri, cookie); var response = HttpReq(cookieContainer); response.Result.Content.ReadAsStringAsync().Wait(); IEnumerable responseCookies = cookieContainer.GetCookies(uri).Cast(); foreach (Cookie new_cookie in responseCookies) { Console.WriteLine(new_cookie.Name + "=" +new_cookie.Value); } ``` request cookies: Cookie: FBeID=nnnnn resonce cookies: Set-Cookie: PHPSESSID=eljpt8ef572ulpqn6bkv6io93h; expires=Thu, 18-Jan-2024 02:33:24 GMT; Max-Age=24105600; path=/ Set-Cookie: FBeID=1423878233 CookieContainer content: PHPSESSID=eljpt8ef572ulpqn6bkv6io93h FBeID=1423878233 FBeID=nnnnn
Author: adjgam
Assignees: -
Labels: `area-System.Net`, `untriaged`
Milestone: -
ManickaP commented 1 year ago

For some reason, the manually created cookie gets the DomainKey changed to contain a leading dot (the cookie gets cloned before inserting into the container). Then, the container has the cookies in different buckets and thus doesn't match them.

I'd say this belongs to the batch of cookie problems in #40328.

We also did some targeted fix for leading dot in #39781.

cc @CarnaViire @antonfirsov

ManickaP commented 1 year ago

Looking more into the history, seems like https://github.com/dotnet/runtime/pull/64038 didn't catch all possibilities. We might also use: https://github.com/dotnet/runtime/blob/d0a287a56fda74f610e2ada174e64512dc596130/src/libraries/Common/src/System/Net/CookieComparer.cs#L25-L30 As equality comparer for the hashtable: https://github.com/dotnet/runtime/blob/d0a287a56fda74f610e2ada174e64512dc596130/src/libraries/System.Net.Primitives/src/System/Net/CookieContainer.cs#L103

antonfirsov commented 1 year ago

A minimal repro to trigger this issue would be:

Uri uri = new Uri("https://arduino.ua");
CookieContainer container = new();
Cookie a = new()
{
    Domain= "arduino.ua",
    Name = "FBeID",
    Value = "0"
};
container.Add(uri, a);
container.SetCookies(uri, "FBeID=42");
Assert.Equal(1, container.Count);

For some reason, the manually created cookie gets the DomainKey changed to contain a leading dot

The manually created cookie has an explicitly set domain and CookieContainer is adding a leading dot when the domain is explicit. @adjgam a workaround to this issue is to remove the line setting the domain:

cookie.Domain = "arduino.ua";

According to RFC 6265 we should remove leading dots not add them, however I would be afraid to apply any naïve changes to the code without reviewing the bigger picture and the behavior of various browsers like for other cases described in #40328. Unfortunately, people may depend on the current behavior.

Moving to Future for now.