Runtime: Behavioral Change Between SocketsHttpHandler and WinHttpHandler with HTTP method names

[martincostello]

Behavioral Change Between SocketsHttpHandler and WinHttpHandler with HTTP method names

_Moved from dotnet/core#1660_

A behaviour change between using WinHttpHandler (the 2.0 default) and SocketsHttpHandler (the new 2.1 default) with HttpClient when making HTTP requests with a user-specified HTTP method string that is lower-case that may cause HTTP requests to be rejected as invalid.

General

I help maintain an ASP.NET Core 2.0 application which we recently upgraded to ASP.NET Core 2.1. This application is hosted on AWS EC2 instances with IIS as a reverse proxy, with client requests being routed to AWS' Application Load Balancer (ALB).

In the end-to-end tests for the application, we have some test cases that perform some validation against the application as part of the deployment into AWS, one of which, when simplified, looks like this:

[Theory]
[InlineData("get")]
[InlineData("head")]
[InlineData("options")]
public async Task Validate_Application_Health(string httpMethod)
{
    var requestUri = new Uri("https://my-application-load-balancer");

    using (var httpClient = new HttpClient())
    {
        using (var request = new HttpRequestMessage(new HttpMethod(httpMethod), requestUri))
        {
            using (var response = await httpClient.SendAsync(message))
            {
                Assert.Equal(response.StatusCode, HttpStatusCode.OK);
            }
        }
    }
}

Upon upgrading the application to run on ASP.NET Core 2.1, when the application was first deployed into AWS and the tests run, they failed with HTTP 400 responses instead of the expected HTTP 200. The same test pointed at the deployed 2.1 application targeting 2.0 locally passed as expected, indicating the difference was somewhere in the tests client-side.

After various bits of trial and error, I eventually narrowed the issue down to the use of _lower case_ HTTP methods being passed as argument to the HttpMethod constructor. It appears that ALB rejects HTTP requests that are using lowercase HTTP methods, as changing the test case to send through _upper case_ strings when using 2.1 made them pass again (StackOverflow).

I guess something in the previous stack normalised the HTTP methods to upper case before sending them over the wire, but from looking at the code this appears to not be the case in the new 2.1 networking stack for HttpClient.

While this was easy to workaround and fix, it is a change in behaviour between 2.0 and 2.1 (at least on Windows) that as far as I'm aware isn't documented and is sort-of a breaking change, so I thought it best to report this as an issue, even if it does end up being closed as _By Design_ or _Won't Fix_.

Below is a repro console application the exhibits the difference in behaviour. On my machine, it outputs the following when the request URI is a valid AWS ALB:

WinHttpHandler GET -> OK
WinHttpHandler get -> OK
SocketsHttpHandler GET -> OK
SocketsHttpHandler get -> BadRequest
HttpClient GET -> OK
HttpClient get -> BadRequest

HttpMethodCase.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <LangVersion>latest</LangVersion>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp2.1</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="System.Net.Http.WinHttpHandler" Version="4.0.0" />
  </ItemGroup>
</Project>

Program.cs

using System;
using System.Net.Http;
using System.Threading.Tasks;

namespace HttpMethodCase
{
    internal static class Program
    {
        internal static async Task Main(string[] args)
        {
            var requestUri = new Uri("http://url-for-a-valid-aws-application-load-balancer/");

            using (var handler = new WinHttpHandler())
            {
                await SendAsync("GET", requestUri, handler);
                await SendAsync("get", requestUri, handler);
            }

            using (var handler = new SocketsHttpHandler())
            {
                await SendAsync("GET", requestUri, handler);
                await SendAsync("get", requestUri, handler);
            }

            await SendAsync("GET", requestUri);
            await SendAsync("get", requestUri);
        }

        private static async Task SendAsync(
            string httpMethod,
            Uri requestUri,
            HttpMessageHandler handler = null)
        {
            using (var httpClient = handler == null ? new HttpClient() : new HttpClient(handler, disposeHandler: false))
            {
                using (var request = new HttpRequestMessage(new HttpMethod(httpMethod), requestUri))
                {
                    using (var response = await httpClient.SendAsync(request))
                    {
                        await Console.Out.WriteLineAsync($"{handler?.GetType().Name ?? "HttpClient"} {httpMethod} -> {response.StatusCode}");
                    }
                }
            }
        }
    }
}

[stephentoub]

@davidsh, do you have thoughts on whether SocketsHttpHandler should verbs (known or otherwise) to be capitalized?

[davidsh]

It appears that ALB rejects HTTP requests that are using lowercase HTTP methods, as changing the test case to send through upper case strings when using 2.1 made them pass again (StackOverflow).

Historically, the RFC has referred to all methods in all caps, i.e. GET, HEAD, POST.

I'm looking thru the RFC's, though, because I vaguely recall that method names are supposed to be interpreted by ignoring case, i.e. "get" == "GET". But I could be wrong about that.

I think the safest thing to do, though, is to up case the standard method names before putting them on the wire. The HttpMethod class we have that already does that for the standard names, i.e. HttpMethod.Get.Method == "GET".

[stephentoub]

The HttpMethod class we have that already does that for the standard names, i.e. HttpMethod.Get.Method == "GET".

Yes and no. If you do new HttpMethod("get").Method, it returns "get", but the instance will compare with case insensitivity, e.g. new HttpMethod("get") == new HttpMethod("GET").

I think the safest thing to do, though, is to up case the standard method names before putting them on the wire.

Which ones? I looked to see what netfx and WinHttp do here. I tried:

• "get"
• "head"
• "post"
• "put"
• "delete"
• "trace"
• "options"
• "patch"
• "mkcol"

netfx only forces capitalization of:

• "get"
• "head"
• "post"
• "put"
• "mkcol"

(then from looking at the src, it also appears to capitalize "connect", but I didn't test that).

WinHTTP capitalizes:

• "get"
• "head"
• "post"
• "put"
• "delete"
• "trace"
• "options"

[stephentoub]

Since previous implementations differed and there's no one right answer, I guess I'll just standardize based on the verbs that have public static properties on HttpMethod.

[davidsh]

Actually, RFC 2616 does use the words "case sensitive" for the defined methods:

https://www.ietf.org/rfc/rfc2616.txt

5.1.1 Method
The Method token indicates the method to be performed on the resource identified by the Request-URI.
 The method is case-sensitive.
       Method         = "OPTIONS"                ; Section 9.2
                      | "GET"                    ; Section 9.3
                      | "HEAD"                   ; Section 9.4
                      | "POST"                   ; Section 9.5
                      | "PUT"                    ; Section 9.6
                      | "DELETE"                 ; Section 9.7
                      | "TRACE"                  ; Section 9.8
                      | "CONNECT"                ; Section 9.9
                      | extension-method
       extension-method = token

The updated RFC 7231 (replaces RFC 2616) also says it is case sensitive.

https://www.ietf.org/rfc/rfc7231.txt

4.1.  Overview

...

   HTTP was originally designed to be usable as an interface to
   distributed object systems.  The request method was envisioned as
   applying semantics to a target resource in much the same way as
   invoking a defined method on an identified object would apply
   semantics.  The method token is case-sensitive because it might be
   used as a gateway to object-based systems with case-sensitive method
   names.

[stephentoub]

Actually, RFC 2616 does use the words "case sensitive" for the defined methods:

Then presumably this is by design and we shouldn't do anything here? i.e. if someone specifies "get", we send "get"? Or for compat, we should still do so?

[martincostello]

Then presumably this is by design and we shouldn't do anything here? i.e. if someone specifies "get", we send "get"? Or for compat, we should still do so?

Yeah, that's what I was wondering.

When I was trying to get to the bottom of the failing test, I found that ALB was rejecting the use of get, but IIS itself when I hit an instance directly by its IP was fine with it.

Seems like some servers are more lenient that the RFC allows, so some clients might _technically_ be non-compliant but are unaware as their code is working happily.

[davidsh]

Then presumably this is by design and we shouldn't do anything here? i.e. if someone specifies "get", we send "get"? Or for compat, we should still do so?

It is a design requirement for the wire protocol that the methods are case sensitive. However, historically, we have "helped" developers and made sure that the standard defined HTTP methods are transmitted correctly regardless of how they set them into the APIs.

From a compat perspective, we probably should fix this.

[stephentoub]

Fixed in master.

[JimCline]

I don't know if this is related but I have been experiencing a behavior change
see https://stackoverflow.com/questions/50749042/httpclient-with-net-core-2-1-hangs

The code works fine under core2.0 on the home and corporate network. core2.1 works on the home network but not on the corporate network. I've tried using the WinHttpHandler and AppContext.SetSwitch("System.Net.Http.UseSocketsHttpHandler", false);

Still does not work on the corporate network.

[karelz]

@JimCline this is most likely unrelated problem. Your problem seems to be related to proxy (maybe dotnet/corefx#30166).
It would be best to file a new issue, collect Fiddler/Wireshark traces, so that we can narrow it down and root-cause. Then we will either dupe it against dotnet/corefx#30166 or it will track another problem.

[JimCline]

dotnet/corefx#30166 was the issue. I solved by using WinHttpHandler setting:
WindowsProxyUsePolicy = WindowsProxyUsePolicy.UseWinInetProxy

[stephentoub]

Fixed by PR dotnet/corefx#30370 in release/2.1 branch. Will be part of 2.1.3 release.