dotnet / runtime

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

HttpClient: Impossible to send "Accept: a;charset=b" #21131

Closed zugzwangm closed 4 years ago

zugzwangm commented 7 years ago

I want to make a http request with a header "Accept" with value "a;charset=b". It is impossible to do that with HttpClient.

using (var client = new HttpClient())
{
    client.DefaultRequestHeaders.TryAddWithoutValidation("Accept", "a;charset=b");
    // ...
}

This will generate a request with header "Accept" with value "a; charset=b". Yes, this extra space is obviously valid, but the original value was also valid. And there are APIs out there that won't work with the extra space.

The only way to make it work is to hack it with reflection:

client.DefaultRequestHeaders.TryAddWithoutValidation("Accept", "foo");
foreach (var v in client.DefaultRequestHeaders.Accept)
{
    if (v.MediaType.Contains("foo"))
    {
        var field = v.GetType().GetTypeInfo().BaseType.GetField("_mediaType", BindingFlags.NonPublic | BindingFlags.Instance);
        field.SetValue(v, "a;charset=b"); // avoid MediaType's validation
        v.CharSet = "";                 
    }
}

Sugestion:

client.DefaultRequestHeaders.AddHeaderAndDontMessWithItDontChangeItDontParseItIWantTotalControlThankYouVeryMuch("Whatever header I want", "Whatever value I want");
geoffkizer commented 7 years ago

I don't understand why we don't send the original, unmodified header in cases like this. @davidsh @cipop do you guys know?

CIPop commented 7 years ago

@geoffkizer Some parser (WinHTTP?) appears to do normalization. Interestingly the RFC ABNF production doesn't include a space yet some of the RFC examples do.

@zugzwangm Do you see the same behavior in .Net Desktop?

And there are APIs out there that won't work with the extra space.

If any of the ones you know are public, could you give a few examples please?

davidsh commented 7 years ago

I don't understand why we don't send the original, unmodified header in cases like this. @davidsh @cipop do you guys know?

@geoffkizer In general, most HTTP stacks will try to normalize the headers when we send on the wire and that includes things like adjusting spaces as per the RFC rules of should and must things. The rule in networking is to be "strict in what we send" and "flexible in what we receive/parse". So, the tendency is to do things like this (add the space, etc.).

davidsh commented 7 years ago

Setting milestone

devandanger commented 6 years ago

I was more recently affected this, working with the vendor on their side to see if they can correct the logic to better conform with the RFS standard. Currently getting the error on their side 'Unable to extract version number from content type'.

Curious what the process of fixing this and it making it into the mono repo is. Any links which can better explain that flow would be great. Thanks!

DanRigby commented 6 years ago

It would appear that .NET Framework doesn't share this behavior, at least not when using HttpWebRequest, if this SO answer is correct: https://stackoverflow.com/a/40162071/53777

Some level of control over this normalization behavior would be helpful in working with non standards compliant host systems.

karelz commented 6 years ago

@DanRigby too many options affects usability and often makes the implementation more complex - so you pay by having more bugs, regressions and higher maintenance cost. I may be fine if we adjust behavior and if we are more benevolent. I am not huge fan of adding more knobs to control things.

JCKodel commented 6 years ago
using(var x = new HttpRequestMessage())
{
    x.Headers.TryAddWithoutValidation("Accept", "application/json;something-weird-here");
}

It would make sense that TryAddWithoutValidation would not validate and/or normalize the header. BTW, an example of this error can be seen in https://moip.com.br/en/ (an online payment gateway requires Accept: application/json;version=2.1. One space after ; it won't recognize the version 2.1 (and will send an old response)).

I know this is a Moip problem (how hard is to follow RFC rules, after all?), but, we, as the "most amazing programming technology, like ever"™ users, must have the option to satisfy those Java... people =P

karelz commented 6 years ago

@JCKodel thanks for the real-world example, that is very helpful. Do you happen to know what Moip uses under the hood? Is it problem in their code or in the server they use? If it is problem in the server, then we can expect more instances and the priority of this API would be higher.

JCKodel commented 6 years ago

@karelz : AFAIK, they use Java. This (https://github.com/backend-br/vagas/issues/122) is a job offering explaning some of the tech they use.

Calling their sandbox, the server used is Apache: image

The last time I saw an error message, it was from TomCat 8 (now they have a custom error page and this info is not disclosed anymore).

But I don't know if this is an server issue or a poorly written header reader (headers["Accept"].Split(";")[1] == "expectedvalue" will return false... I know... those poor Java souls can't really do a headers["Accept"].Split(";").Select(s => s.Trim).Last() =P

Nevertheless, we should be able to control exactly what we want to do, and I think TryAddWithoutValidation should mean that.

felipeOliveira commented 6 years ago

https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs

image

There is already support for leaving that space out, but no way to access it. TryAddWithoutValidation should pass true to the leadingSeparator argument.

karelz commented 6 years ago

@felipeOliveira the fact that something is technically possible is not a justification for adding new APIs. If we followed that, .NET API surface would be a GIANT mess today. That said, @JCKodel's information is something that can support API addition.

Also, please remove your reflection code snippet - we do not want encourage anyone to use private reflection. That is unsupported hackary that leads to compat problems down the road when the underlying implementation changes. Thanks! If anyone uses code like that, it should be with HUGE comment saying "UNSUPPORTED, IT MAY BREAK AT ANY TIME WITHOUT WARNING".

mustakimali commented 6 years ago

I hit the issue today. As I don't have any control over that non-conforming REST API I'm consuming - I have little option but to use the reflection method for now 😢

MarcoRossignoli commented 6 years ago

The rule in networking is to be "strict in what we send" and "flexible in what we receive/parse". So, the tendency is to do things like this (add the space, etc.).

Maybe an overload on TryAddWithoutValidation like skipNormalization

/cc @davidsh @karelz @geoffkizer

rmkerr commented 5 years ago

This actually appears to be fixed in .NET Core 2.1. Capturing the request the request from the original report in fiddler, it appears that no space was added:

GET http://microsoft.com/ HTTP/1.1
Accept: a;charset=b
Host: microsoft.com
Connection: Keep-Alive

I'm not sure of the exact change that fixed this issue, but it was probably related to the switch from WinHttpHandler to SocketsHttpHandler for our underlying HTTP implementation.

I'm going to close this issue as fixed. If I missed something and there is still an active issue here, feel free to reopen it :)

joseftw commented 5 years ago

@rmkerr This appear to still be an issue, see below for repro and fiddler response. I noticed this because of this question at SO: https://stackoverflow.com/questions/55833095/httpclient-httprequestmessage-adding-white-spaces-to-headers/56048114

.NET Core SDK (reflecting any global.json):
 Version:   3.0.100-preview5-011568
 Commit:    b487ff10aa

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17763
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.0.100-preview5-011568\

Host (useful for support):
  Version: 3.0.0-preview5-27626-15
  Commit:  61f30f5a23
class Program
    {
        static async Task Main(string[] args)
        {
            var httpClient = new HttpClient();
            var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost:5555");
            var acceptValue = "text/html,application/xml,application/json";
            request.Headers.Add("whatever", acceptValue);
            request.Headers.TryAddWithoutValidation("Accept", acceptValue);

            var result = await httpClient.SendAsync(request);
        }
    }

The above code produces the following result in Fiddler (notice the spacing in the Accept header but not in the whatever header):

GET http://localhost:5555/ HTTP/1.1
whatever: text/html,application/xml,application/json
Accept: text/html, application/xml, application/json
Host: localhost:5555
parxal commented 5 years ago

Hi @rmkerr this still happens in version 2.2 . Any ideia to override this? i am having a problem with a server that don't accept a white space after the ;

davidsh commented 5 years ago

Hi @rmkerr this still happens in version 2.2 . Any idea to override this? i am having a problem with a server that don't accept a white space after the ;

@rmkerr is not on .NET Core project anymore.

If you are still having a problem, please open a new issue. And please include enough information in order for us to reproduce the problem. Please include a run-able repro program. Thx.

parxal commented 5 years ago

@davidsh open it here > https://github.com/dotnet/corefx/issues/39260

GameHackerPM commented 4 years ago

It still exists! I tried with many versions of Net Framework (4.5, 4.6.1, 4.7.1, 4.8) and also Core Versions (2.0, 3.0, NetFramework 5.0) but still doesn't work! Please I need help for this <<<!