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

Url.ToString gives back wrong result causing cookies to not be saved in the CookieJar #585

Closed
AeonLucid opened this issue Dec 12, 2020 · 2 comments
Labels

Comments

@AeonLucid
Copy link
Contributor

AeonLucid commented Dec 12, 2020

This issue causes cookies to be saved incorrectly. I tried to write a test to reproduce the issue but CookieCutter.IsValid calls Url.IsValid which calls Url.ToString() implicitly.

Here is a test which reproduces this issue.

[Test]
public async Task TestCookies()
{
    HttpTest.RespondWith("hi", cookies: new { x = "bar" });

    var client = new FlurlClient();
    var jar = new CookieJar();
    var response = await "https://cookies.com/"
        .WithClient(client)
        .WithCookies(jar)
        .SetQueryParams(new QueryParamCollection
        {
            {"a", "b c"},
            {"c", "d"},
        })
        .GetAsync();

    Assert.IsNotEmpty(response.Cookies);
    Assert.AreEqual("bar", response.Cookies.FirstOrDefault(c => c.Name == "x")?.Value);
    Assert.IsNotEmpty(jar);
    Assert.AreEqual("bar", response.Cookies.FirstOrDefault(c => c.Name == "x")?.Value);
}

It fails inside CookieCutter.IsValid where Url.IsValid is called. Passes this url into Url.IsValid.

image

Funny thing is, if you open the OriginUrl before Url.IsValid debug view before the method is called like so

image

Then the correct url is passed into Url.IsValid.

image

I think this is because it loads all the properties and calls EnsureParsed in the background.

@tmenier
Copy link
Owner

tmenier commented Dec 14, 2020

Yep, this is legit. And I learned something new. The root of the problem is in FlurlResponse.LoadCookies. It needs the URL of the original request in order to process the Set-Cookie headers and gets it via HttpResponseMessage.RequestMessage.RequestUri.ToString(). But apparently Uri.ToString() un-encodes query params, effectively giving you an invalid URI in this case because there's a space in it, even though it was property encoded when the request was sent! Switched from ToString() to AbsoluteUri and it works as expected. Thanks for reporting.

tmenier added a commit that referenced this issue Dec 14, 2020
@tmenier
Copy link
Owner

tmenier commented Dec 14, 2020

Fixed & released in 3.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants