Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Does not work with proxy due to missing Content-Length #27

Open
mcm-ham opened this issue Jan 20, 2021 · 3 comments
Open

Does not work with proxy due to missing Content-Length #27

mcm-ham opened this issue Jan 20, 2021 · 3 comments

Comments

@mcm-ham
Copy link

mcm-ham commented Jan 20, 2021

I'm finding requests inside batch are missing request body once forwarded by reverse proxies such as ProxyKit or YARP are missing. Looking into the source code I discovered this was due to missing ContentLength:
https://github.com/ProxyKit/ProxyKit/blob/master/src/ProxyKit/HttpContextExtensions.cs#L59

After I manually added ContentLength header it now correctly works. However, I didn't see this being needed in the examples provided here:
https://devblogs.microsoft.com/aspnet/introducing-batch-support-in-web-api-and-web-api-odata/

@Tornhoof
Copy link
Owner

Tornhoof commented Jan 20, 2021

Hello,
thank you for your interest in httpbatchhandler.

The middleware is not setting the content-length header, this is done by the actual server implementation (above the middleware). In the tests the content-length is correctly set in the response.
As for the test website, kestrel correctly uses Transfer-Encoding: chunked, which is the expected behaviour if Kestrel does not the length of the response (which it doesn't without buffering).

example:

HTTP/1.1 200 OK
Date: Wed, 20 Jan 2021 07:10:09 GMT
Content-Type: multipart/batch; boundary="8101b18f-02c6-42a2-92bb-e64bec026c8c"
Server: Kestrel
Transfer-Encoding: chunked

Your problem appears to be somewhere else, for example something stripping the transfer-encoding or changing the behavior, do you run yarp/proxykit in the same process as the middleware? If yes, I wouldn't be suprised if you run into problems, as they all change request handling quite a bit.

@mcm-ham
Copy link
Author

mcm-ham commented Jan 20, 2021

thank you for your interest in httpbatchhandler.

Thanks for your work on HttpBatchHandler, we're finding it useful.

In the tests the content-length is correctly set in the response.

I'm referring to the individual requests after batch middleware i.e. if I modify the test website to add this:

app.Use(async (context, next) => {
    _logger.LogWarning($"Before Content-Length: {context.Request.Headers[HeaderNames.ContentLength]}");
    await next();
});
app.UseBatchMiddleware();
app.Use(async (context, next) => {
    _logger.LogWarning($"After Content-Length: {context.Request.Headers[HeaderNames.ContentLength]}");
    await next();
});

And then make a batch request like this:

var req = new HttpMessageContent(
    new HttpRequestMessage(HttpMethod.Post, "http://localhost:5123/api/values") { Content = new StringContent("5") }
);
await new HttpClient().SendAsync(new HttpRequestMessage(HttpMethod.Post, "http://localhost:5123/api/batch")
{
    Content = new MultipartContent("mixed", "batch_" + Guid.NewGuid().ToString()) { req, req }
});

You can see from the log output that the batch request has content-length but once split into 2 individual requests they no longer have content-length header.

warn: HttpBatchHandler.Website.Startup[0]
      Before Content-Length: 432
warn: HttpBatchHandler.Website.Startup[0]
      After Content-Length:
warn: HttpBatchHandler.Website.Startup[0]
      After Content-Length:

@Tornhoof
Copy link
Owner

Ah okay, now I understand, you mean Content-Length of individual requests after the batch fan out. HttpBatchHandler does not change the headers for those requests in any way, so if your original batched request contains no content-length then HttpBatchHandler does not add them. So yeah, your solution to add them individually should work fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants