Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CookieContainer doesn't replace Cookie with same name #84820

Open
adjgam opened this issue Apr 14, 2023 · 4 comments · May be fixed by #112604
Open

CookieContainer doesn't replace Cookie with same name #84820

adjgam opened this issue Apr 14, 2023 · 4 comments · May be fixed by #112604
Assignees
Labels
area-System.Net bug in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@adjgam
Copy link

adjgam commented Apr 14, 2023

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 ghost added the untriaged New issue has not been triaged by the area owner label Apr 14, 2023
@ghost
Copy link

ghost commented Apr 14, 2023

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<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

Author: adjgam
Assignees: -
Labels:

area-System.Net, untriaged

Milestone: -

@ManickaP
Copy link
Member

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
Copy link
Member

Looking more into the history, seems like #64038 didn't catch all possibilities. We might also use:

internal static bool EqualDomains(ReadOnlySpan<char> left, ReadOnlySpan<char> right)
{
if (left.Length != 0 && left[0] == '.') left = left.Slice(1);
if (right.Length != 0 && right[0] == '.') right = right.Slice(1);
return left.Equals(right, StringComparison.OrdinalIgnoreCase);

As equality comparer for the hashtable:
private readonly Hashtable m_domainTable = new Hashtable(); // Do not rename (binary serialization)

@antonfirsov
Copy link
Member

antonfirsov commented Apr 14, 2023

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.

@antonfirsov antonfirsov removed the untriaged New issue has not been triaged by the area owner label Apr 14, 2023
@antonfirsov antonfirsov added this to the Future milestone Apr 14, 2023
@MihaZupan MihaZupan added the bug label Apr 15, 2023
@antonfirsov antonfirsov self-assigned this Feb 13, 2025
@antonfirsov antonfirsov linked a pull request Feb 15, 2025 that will close this issue
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net bug in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants