Ability to enable StreamCopyHttpContent when body is empty

[arminbashizade]

What should we add or change to make your life better?

(somehow similar to #618)
provide ability to enable copying content when the request body is originally empty, but will be populated later

Why is this important to you?

I have a transformer that adds a dynamic attribute (different for each request) to body for some routes, and I follow instructions outlined in #1601:

// deserialize, add attribute, serialize into bodyStr
var bytes = Encoding.UTF8.GetBytes(bodyStr);
context.HttpContext.Request.Body = new MemoryStream(bytes);
context.ProxyRequest.Content!.Headers.ContentLength = bytes.Length;

for example:

{
  "attr1": "value1"
}
// becomes
{
  "attr1": "value1",
  "attr2": "dynamic value"
}
////////////////
{}
// becomes
{
  "attr2": "dynamic value"
}
////////////////

// also becomes
{
  "attr2": "dynamic value"
}

this works fine when body is not empty, and by that I mean ContentLength == 0, however when the body is empty ProxyRequest.Content is set to null in HttpForwarder here:
https://github.com/microsoft/reverse-proxy/blob/b6f00851fc1032706c275b414244a95ef70d3a26/src/ReverseProxy/Forwarder/HttpForwarder.cs#L561-L584

and it's too late to set ContentLength or override IHttpRequestBodyDetectionFeature because transformers are applied after that:
https://github.com/microsoft/reverse-proxy/blob/b6f00851fc1032706c275b414244a95ef70d3a26/src/ReverseProxy/Forwarder/HttpForwarder.cs#L417-L423

I guess I can directly set context.ProxyRequest.Content in the specific case where body is empty (can you please confirm there's no risk here?) but I'd rather not adding an exception case. Is this something I can achieve with existing implementation?

[MihaZupan]

I guess I can directly set context.ProxyRequest.Content in the specific case where body is empty (can you please confirm there's no risk here?) but I'd rather not adding an exception case. Is this something I can achieve with existing implementation?

No, you should not replace the Content with your own, YARP has a custom implementation that it relies on.

Have you tried setting HttpContext.Request.Body in a middleware before calling into YARP?
E.g.

endpoints.MapReverseProxy(app =>
{
    // app.UseSessionAffinity();
    // app.UseLoadBalancing();
    // app.UsePassiveHealthChecks();

    app.Use(async (context, next) =>
    {
        context.Request.Body = ...;
        await next();
    });
});

[arminbashizade]

I understand that YARP has its own implementation, if I'm following the code correctly it's SetupRequestBodyCopy, but in this specific case, YARP will set content to null here:
https://github.com/microsoft/reverse-proxy/blob/b6f00851fc1032706c275b414244a95ef70d3a26/src/ReverseProxy/Forwarder/HttpForwarder.cs#L419-L420
because SetupRequestBodyCopy returns null, so is that still going to be an issue if I overwrite context.ProxyRequest.Content?

[MihaZupan]

so is that still going to be an issue if I overwrite context.ProxyRequest.Content?

Yes. YARP will assume that it's not sending any request content, so you will run into a bunch of issues:

• YARP relies on the custom implementation for telemetry and error detection.
• YARP may not wait for the request content upload to complete, which would lead to a bunch of thread-safety issues as we're telling ASP.NET that we're done with a request while we're still reading the body in the background.
  • For example
    https://github.com/microsoft/reverse-proxy/blob/b6f00851fc1032706c275b414244a95ef70d3a26/src/ReverseProxy/Forwarder/HttpForwarder.cs#L300-L307

[arminbashizade]

Thanks for providing extra context, I'll give the middleware a shot.

[danhulet]

I was able to create a middleware that can enable modifying the body

app.MapReverseProxy(x =>
{
    x.Use(async (context, next) =>
    {
        var canHaveBodyFeature = context.Features.Get<IHttpRequestBodyDetectionFeature>();
        if (HttpMethods.IsPost(context.Request.Method) && context.Request.ContentLength == 0 && canHaveBodyFeature?.CanHaveBody != true)
        {           
            context.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
        }
        await next();
    });
});

[arminbashizade]

@MihaZupan if you don't see any issues with the solution @danhulet provided, feel free to close this.

[Tratcher]

Ah, we modified the custom content object to delay capturing the request body stream for cases like this, but that only works if we thought there was a body to begin with. There's not an easy way to add a body to a request that didn't have one. We should spend a few minutes to see if there's a way to make that work.

[MihaZupan]

And if not, it'd be good to add a cheap check in YARP to scream if you change the content object, as you may be unlucky and see it "work" in simple tests.

[MihaZupan]

We added a check for request transforms modifying the request.Content in 2.3.0 - #2722.
#2867 is tracking whether we can improve the experience for request transforms so that you don't have to override the body detection feature.