Somewhat similar to https://github.com/dotnet/aspnetcore/issues/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.
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);
Content-Length is null ✔️ and Transfer-Encoding header is present. ✔️ 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.
dotnet --infoPS 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]
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)
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.
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.
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.
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.
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.
https://github.com/dotnet/aspnetcore/blob/4b88074e308c9ddf6ba6f513c30942f4aa7d3ccf/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs#L33
https://github.com/dotnet/aspnetcore/blob/4b88074e308c9ddf6ba6f513c30942f4aa7d3ccf/src/Security/Authentication/Negotiate/test/Negotiate.Test/NegotiateHandlerTests.cs#L438-L454
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 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.
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. https://github.com/dotnet/AspNetCore.Docs/issues/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
Yes was aware of HTTP/2 issue there, am going to address that in next version, cheers.
The simplest solution for a proxy is to always assume there's a request body.
Azure App Service doesn't like GET requests with Content-Length; people were reporting Bad Gateway https://github.com/ProxyKit/ProxyKit/issues/179 so it appears need to do the check (at least for HTTP/1.1).
Thanks for the feedback 👍
Closed via new docs.
After struggling with a similar issue (Kestrel HTTP/2 and YARP) I'm convinced that Kestrel should provide some hint for the HTTP/2 body scenario and we should mimic that here too.
https://github.com/dotnet/aspnetcore/issues/24175
Fixed by #24984.
Most helpful comment
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)