HttpClient sending fragment portion of uri on request

[SteveL-MSFT]

The code here was changed in dotnet/corefx#27360 to address a redirect issue with fragments. However, in the non-redirect case, the fragment portion of the URI is not sent on the request per section 9.5 of RFC 7231. Although many http sites ignore the fragment and return the document, some will instead return a 404.

This is causing a regression in the PowerShell webcmdlet compared to Windows PowerShell 5.1 (.NET Framework 4.x). PowerShell/PowerShell#7796

[davidsh]

@rmkerr

[rmkerr]

Hey @SteveL-MSFT, can you provide an isolated repro for this issue? We currently have tests for this behavior that pass, and it would be helpful to know what is different in your case.

[SteveL-MSFT]

@rmkerr tried to find a public web service that returned the raw URL of the request, but couldn't find one as they all seem to ignore the fragment and treat the URL as though the fragment wasn't supplied. However, you can repro this easily with our test weblistener. You can repro using these steps using PowerShell Core 6:

pwsh
git clone https://github.com/powershell/powershell
cd powershell
ipmo ./build.psm1
publish-pstesttools
start-weblistener
$url = get-weblistenerurl
$h = [System.Net.Http.HttpClient]::new()
$h.GetAsync($url)  # this works
$h.GetAsync("$url#fragment") # this fails with 404

[geoffkizer]

Right now, I can't remember why we decided in dotnet/corefx#27360 to send fragments. This does seem wrong per the RFC as quoted above.

Is WinHttpHandler sending fragments? Is that why we did it? @stephentoub do you recall?

[stephentoub]

I don't remember the details, other than a vague recollection that the rfc wasn't very explicit and the various other implementations were all over the map in what they choose to do and when, often inconsistently, so we just decided to always send.

[rmkerr]

I took another look at the spec, and I think we are probably not supposed to be sending the fragment. Take a look at the definition for http_URL in RFC 2616, which appears to ignore the fragment:

http_URL = "http:" "//" host [ ":" port ] [ abs_path [ "?" query ]]

There's also this section of the URI spec, which seems to say we should never be using the fragment to locate a resource:

the fragment identifier is not used in the scheme-specific
   processing of a URI; instead, the fragment identifier is separated
   from the rest of the URI prior to a dereference, and thus the
   identifying information within the fragment itself is dereferenced
   solely by the user agent, regardless of the URI scheme.

I think we should probably double check it by seeing how browsers behave, but if we do choose to drop the fragment there's a basis for that behavior in the spec.

[davidsh]

Take a look at the definition for http_URL in RFC 2616, which appears to ignore the fragment:

I'd recommended you not look at RFC 2616 since it has been replaced by the RFC 723x documents. So, you should check the updated RFC's to be sure.

[stephentoub]

If we decide to change it, it should be as easy as changing one line:
https://github.com/dotnet/corefx/blob/581627f713477fda8ac87be8dac74ff017d955cf/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs#L395
putting it back to the way it used to be:
https://github.com/dotnet/corefx/pull/27360/files#diff-300922d9e15c1d874df079b5746c8f10L281
plus updating the tests, of course (the currently validate the previously decided-upon behavior).

[rmkerr]

Thanks David, 7230 makes it a lot clearer. The fragment is actually included in the definition of http_URL there, and the issue is addressed in more detail. Section 5.1 "Identifying a Target Resource" specifies:

The target URI excludes the reference's fragment component, if any,
   since fragment identifiers are reserved for client-side processing

[geoffkizer]

plus updating the tests, of course (the currently validate the previously decided-upon behavior).

Which tests do you mean? The redirect tests? I think those should still be valid -- the RequestUri should still propagate the fragment on redirect, it just shouldn't send the fragment on the wire.

Or are there other tests you mean?

[stephentoub]

Which tests do you mean? The redirect tests?

Yes. I thought we were validating both the original request's sent uri and the redirect uri, but if we're only validating the latter, then it should be fine as-is.

[devcrafting]

In dotnet/corefx#32415, you can find a real server that return a 404 when fragment is sent.

If it can help, as I mentioned in the same issue, .NET Framework was ignoring fragment. I also tested with Postman, the same. I know that other (old) implémentations could be bad ones also, but still some references.

Thanks

[rmkerr]

I'm not actively working on this issue, but I want to summarize before I forget.

Our next steps should be to:
(1) Verify the expected behavior, so that if we make a change we make the correct one.
(2) Make the fix if necessary, as described in this comment.
(3) Update the tests, removing any checks that validate that the fragment is send on the wire.

[karelz]

Should be fairly straightforward, marking as "up for grabs" - @rmkerr can provide additional details if needed.