When streaming content to TestServer, `Transfer-Encoding` header is missing.

[damianh]

Describe the bug

Somewhat similar to #13273 but this is client -> server.

According to https://tools.ietf.org/html/rfc2616#section-4.3:

The presence of a message-body in a request is signaled by the inclusion of a Content-Length or Transfer-Encoding header field in the request's message-headers.

When streaming content to an AspNetCore application via TestSever and it's client without a Content-Length header, the Transfer-Encoding header is missing.

To Reproduce

Given the following contrived code where a client pushes some data to the server and where the Content-Length cannot be determined up-front:

var client = testServer.CreateClient();
var random = new Random();
var request = new HttpRequestMessage(HttpMethod.Post, "/")
{
    Content = new PushStreamContent(async (stream, httpContext, transPortContext) =>
    {
        var buffer = new byte[100];
        for (int i = 0; i < random.Next(1, 100); i++)
        {
            await stream.WriteAsync(buffer);
        }
        await stream.FlushAsync();
        stream.Dispose();
    })
};
var response = await client.SendAsync(request);

1. When run against Kestrel, Content-Length is null ✔️ and Transfer-Encoding header is present. ✔️
2. When run against TestServer, Content-Length is null ✔️ but Transfer-Envoding header is not present. ❌

Notwithstanding the potential complexity of simulating Transfer-Encoding chunked in TestServer and it's custom HttpMessageHandler, it would appear the behaviour is not inline with the RFC?

Side note: I think I'd prefer to see TestServer deprecated in favour of an in-memory kestrel.

Further technical details

• ASP.NET Core version : 2.1.x and 3.1.x
• Include the output of dotnet --info

PS C:\dev> dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   5.0.100-preview.3.20216.6
 Commit:    9f62a32109

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18363
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.100-preview.3.20216.6\

Host (useful for support):
  Version: 5.0.0-preview.3.20214.6
  Commit:  b037784658

.NET SDKs installed:
  2.1.801 [C:\Program Files\dotnet\sdk]
  3.1.102 [C:\Program Files\dotnet\sdk]
  3.1.201 [C:\Program Files\dotnet\sdk]
  3.1.300-preview-015135 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.3.20216.6 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.0-preview.3.20215.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0-preview.3.20214.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0-preview.3.20214.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

[davidfowl]

Side note: I think I'd prefer to see TestServer deprecated in favour of an in-memory kestrel.

Yea it might be time to start seriously considering this. It requires the SocketsHttpHandler to support other transports (which is the plan in .net 5)

[Tratcher]

This mirrors #18463 (expecting content-length).

@damianh what's the impact to your application?

Content-Length and Transfer-Encoding are protocol level concerns that the application shouldn't normally depend on. This is even more true in HTTP/2 where the Transfer-Encoding header is no longer used. If the test scenario cares about a specific header it can be set by the client today.

Side note: I think I'd prefer to see TestServer deprecated in favour of an in-memory kestrel.

Providing a Kestrel alternative might be useful, but I have no expectation TestServer would be deprecated. TestServer operates at a very different level of abstraction from Kestrel and there are many test use-cases that kestrel couldn't replicate like flowing exceptions directly back to the client, or letting the client manipulate server side data structures.

[davidfowl]

Providing a Kestrel alternative might be useful, but I have no expectation TestServer would be deprecated. TestServer operates at a very different level of abstraction from Kestrel and there are many test use-cases that kestrel couldn't replicate like flowing exceptions directly back to the client, or letting the client manipulate server side data structures.

I dunno, I'm not sure how many people use it that way 😄. Thats' not how it works when they run anyways.

[Tratcher]

I dunno, I'm not sure how many people use it that way 😄. Thats' not how it works when they run anyways.

Many of the middleware tests are written that way, that's what TestServer was designed for.

The use of TestServer in MVC's WebApplicationFactory is a different story, that one might be better suited for Kestrel.

[davidfowl]

Many of the middleware tests are written that way, that's what TestServer was designed for.

Which feature specifically? Exceptions seems like the most useful one.

[Tratcher]

Yes direct exceptions is certainly the more common case, but the ability to tweak the HttpContext is also very useful for middleware that rely on Features.

aspnetcore/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs

Line 33 in 4b88074

var ex = await Assert.ThrowsAsync<NotSupportedException>(() => SendAsync(server, "/Anonymous1", connection: null));

aspnetcore/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs

Lines 438 to 454 in 4b88074

	private static Task<HttpContext> SendAsync(TestServer server, string path, TestConnection connection, string authorizationHeader = null, bool http2 = false)
	{
	return server.SendAsync(context =>
	{
	context.Request.Protocol = http2 ? "HTTP/2" : "HTTP/1.1";
	context.Request.Path = path;
	if (!string.IsNullOrEmpty(authorizationHeader))
	{
	context.Request.Headers[HeaderNames.Authorization] = authorizationHeader;
	}
	if (connection != null)
	{
	context.Features.Set<IConnectionItemsFeature>(connection);
	context.Features.Set<IConnectionCompleteFeature>(connection);
	}
	});
	}

[damianh]

Thanks for the response folks. @Tratcher I believe we've engaged on this topic a couple of times already and I understand your position on TestServer's intention. I think I've previously mentioned that the docs for Test Server could be clearer about it's intention and, importantly, it's limitations. At the moment I (and others) seem to be using it fine until suddenly it's not fine with subtle pitfalls like this (in this case I was trying to reproduce an an issue reported in ProxyKit and TestServer was giving me a false result). It seems the impression in-the-wild regarding TestSever is that is should be complaint with HTTP spec https://twitter.com/ben_a_adams/status/1232315585976864768

What's clear to me is that I'm looking for a in-memory webserver that fully acts as a web server. In other words, I actually don't want exceptions to flow through to the client. I'll take a 500 and have another way to get at the exception information if I need it. Am predominantly looking for an in-mem one to not deal with port issues locally, in CI, with parallel tests etc.

As that doesn't exist, I think my course of action is to just never use TestServer and just stick with Kestrel - as far as I can tell, I can still manipulate server side data structures via ConfigureTestServices etc, so it'll be fine.

[damianh]

@damianh what's the impact to your application?

Just to answer this, I had a bug in ProxyKit regarding detecting the presence of a body and ensuring it was forwarded (or not). I developed it against TestServer which gave me the wrong impression of something working the way it shouldn't.

[Tratcher]

Docs are a valid concern. TestServer didn't have its own doc until last week. The new doc convers some of these scenarios, but it could mention Content-Length and Transfer-Encoding. dotnet/AspNetCore.Docs#18255
https://docs.microsoft.com/en-us/aspnet/core/test/middleware?view=aspnetcore-3.1

I had a bug in ProxyKit regarding detecting the presence of a body and ensuring it was forwarded (or not)

Your fix doesn't work on HTTP/2, Content-Length is optional and Transfer-Encoding is prohibited. The simplest solution for a proxy is to always assume there's a request body.
https://github.com/ProxyKit/ProxyKit/pull/227/files#diff-7ae9aa92fe8fba004ba2b80d2ee04852R58