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

User-Agent from HttpClient.DefaultRequestHeaders is not set properly in FlurlClient #778

Closed
tranb3r opened this issue Dec 4, 2023 · 7 comments

Comments

@tranb3r
Copy link
Contributor

tranb3r commented Dec 4, 2023

User-Agent from HttpClient.DefaultRequestHeaders is not set properly in FlurlClient, when User-Agent is composed of multiple ProductInfo.

Example:

const string userAgent = "Mozilla/5.0 (X11; Linux i686; rv:109.0) Gecko/20100101 Firefox/120.0";
var httpClient = new HttpClient();
httpClient.DefaultRequestHeaders.Add("User-Agent", userAgent);
var flurlClient = new FlurlClient(httpClient);
foreach (var (name, value) in flurlClient.Headers)
{
    Console.WriteLine(name + ": " + value);
}

will output to console: User-Agent: Mozilla/5.0
instead of User-Agent: Mozilla/5.0 (X11; Linux i686; rv:109.0) Gecko/20100101 Firefox/120.0

So, an important part of the User-Agent is lost.
I guess the logic of headers initialization in FlurlClient constructor is incorrect.
What do you think? Am I missing something?

@tranb3r tranb3r added the bug label Dec 4, 2023
@tmenier
Copy link
Owner

tmenier commented Dec 4, 2023

Have you tried it with 4.0? It handles DefaultRequestHeaders more explicitly so I think it should work. If it's just the opposite (regression bug from 3 to 4), that might be the only case where I'd let it slow down the 4.0 train.

@tranb3r
Copy link
Contributor Author

tranb3r commented Dec 4, 2023

I've been using 4.0 for a verrrrry long time. So the problem is, I do not remember how Flurl managed Headers in 3.x.
Doing a quick test with Flurl.Http 3.2.4, I can see that Headers in FlurlClient are not initialized from httpClient.DefaultRequestHeaders. This seems to be new in 4.0.
So, is it possible in 3.x to properly set a User-Agent? And is there a solution in 4.0? I do not have answers right now but I'll let you know if I manage to figure it out.

@tranb3r
Copy link
Contributor Author

tranb3r commented Dec 4, 2023

Ok, here is some code that shows this could be considered a regression in 4.0:

            const string userAgent = "Mozilla/5.0 (X11; Linux i686; rv:109.0) Gecko/20100101 Firefox/120.0";
            var httpClient = new HttpClient();
            httpClient.DefaultRequestHeaders.Add("User-Agent", userAgent);
            var flurlClient = new FlurlClient(httpClient);
            var page = await flurlClient
                .Request("https://httpbingo.org/headers")
                .GetStringAsync();
            Console.WriteLine(page);

With 4.0:

{
  "headers": {
    "User-Agent": [
      "Mozilla/5.0"
    ]
  }
}

With 3.2.4:

{
  "headers": {
    "User-Agent": [
      "Mozilla/5.0 (X11; Linux i686; rv:109.0) Gecko/20100101 Firefox/120.0"
    ]
  }
}

But maybe there is a workaround when using 4.0.

@tranb3r
Copy link
Contributor Author

tranb3r commented Dec 5, 2023

Here is how I will workaround this issue for now:

        public static FlurlCLient FixUserAgent(this FlurlClient flurlClient)
        {
            var userAgent = flurlClient.HttpClient.DefaultRequestHeaders.UserAgent.ToString();
            flurlClient.Headers.AddOrReplace("User-Agent", userAgent);
            return flurlClient;
        }

But the issue is probably not specific to the User-Agent header.

@tmenier
Copy link
Owner

tmenier commented Dec 8, 2023

Thanks, I'll look into it ahead of the final release. So much to do...really hoped to ship 4.0 this week but not likely to happen.

tmenier pushed a commit that referenced this issue Dec 8, 2023
@tmenier tmenier added the 4.0 label Dec 8, 2023
@tmenier
Copy link
Owner

tmenier commented Dec 8, 2023

This is fixed in pre7 (if you didn't notice). I don't love the fix but MS doesn't make it easy. Thanks for providing the repro, it was perfect.

@tranb3r
Copy link
Contributor Author

tranb3r commented Dec 9, 2023

Thank you Todd for the fix!
I agree, MS implementation of user-agent split/join is a bit weird.

Also, I've tested pre7, it works for me.

@tmenier tmenier closed this as completed Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Released
Development

No branches or pull requests

3 participants
@tmenier @tranb3r and others