Middleware that rewrites content seemingly can't change ResponseStream ContentLength

[RickStrahl]

I have a Live Reload Middleware component that works fine in .NET Core 2.2 but has issues with truncated content in 3.0.

The middleware does a bunch of things, but the part that is problematic has to do with interaction with other content that in this case is output by the StaticFileMiddleware. Basically my middleware creates a new ResponseStream for all output, checks for an HTML response and if it finds it injects a block of script code into the HTML. It does this with a wrapper stream and by intercepting the WriteAsync operations (and overriding a bunch of other ops).

On the outbound pass (at the end of the pipeline) the middleware checks for HTML like this, and if it finds it changes the Response.ContentLength=null:

            // Make sure we force dynamic contentType since we're
            // rewriting the content - static content will set the header explicitly
            // and fail when it doesn't match
            if (_isHtmlResponse.Value)
                _context.Response.ContentLength = null;

The staticFile middleware has explicitly set the ContentLength and this code sets it to null to effectively let the Response calculate it. This has worked fine in .NET Core 2.x, but in 3.0 I see truncated content. Basically I see a small part of the injected content - basically the length of what otherwise would have been there in the original HTML document.

This bit of code should happen before any content has been written because it is part of the custom stream and fired before each write operation on the stream.

So the question are:

• Should I be able to reset the Stream length dynamically using the code I have
• If not what's the right way to Write a different ContentLength?
• I've also tried explicitly writing out the new calculated content length but that too is ignored

It's possible that

If you want to take a closer look, the runnable sample project is on GitHub, here's the relevant code .

The request in question is the static HTML file link which should get intercepted and injects the script.

[RickStrahl]

Upon further digging I actually see some nasty behavior here. It looks like ASP.NET is writing out the data and serving it to the browser correctly. The data is there, but the content length header is not updated when I rewrite it.

Here's a Fiddler trace:

The Content-Length is the original contentlength rather than the one I changed (should 1900+bytes). Even if I explicitly set the header like this:

if (_context.Response.ContentLength != null)            
    _context.Response.ContentLength += WebsocketScriptInjectionHelper.GetWebSocketClientJavaScript(_context).Length;     

I see the value going at 516 bytes, and coming out at 1900+ bytes, but the new value never shows up in the response.

Note again that no output has been sent yet when the content length is set.

[davidfowl]

Few questions:

• What server are you using?
• In 2.2 was the content length being set automatically? AFAIK we don't do none of the servers do that so maybe it was chunked?

[RickStrahl]

It's Kestrel in dev mode. But happens also with IIS Express...

Same code runs in 2.2.

I think the problem is that in 3.0 there's an implicit FlushAsync() happening that's touching the response and basically makes it so headers can't be written anymore.

if (!isContentLengthSet && _context.Response.ContentLength != null)
{
    try
    {
        _context.Response.Headers.Remove("Content-Length");
        _context.Response.Headers.ContentLength +=
            WebsocketScriptInjectionHelper.GetWebSocketClientJavaScript(_context).Length;
        isContentLengthSet = true;
    }
    catch (Exception ex) //   <!---  Headers are read-only because content has been written
    {                    
    }
} 

After some more poking around I also see that FlushAsync() is called before anything else.

Looking at the trace it comes from StartAsync() and I bet that causes the Response/Headers to get locked:

So I managed to now get this to work by adding this to FlushAsync():

public override Task FlushAsync(CancellationToken cancellationToken)
{
    if (!isContentLengthSet && IsHtmlResponse())
    {
        _context.Response.Headers.ContentLength = null;
        isContentLengthSet = true;
    }

    return _baseStream.FlushAsync(cancellationToken);
}

This seems to work now.

That's cool but seems a bit unwieldy. Why does FlushAsync() need to be called before any output is sent? And if that needs to happen to provide an 'initial' hook, maybe that shouldn't affect the Response.Header locking.

[RickStrahl]

Just double checked: 2.x does not call FlushAsync() (or Flush()) on the start of the request and that probably accounts for why I was able to change the content-length in those versions. The FlushAsync() is definitely what's affecting the headers locking.

[davidfowl]

That looks like correct analysis. In 2.2 that would be the case during the first call to WriteAsync (first write will flush headers if the content length wasn't set). So it worked on 2.2 because that logic was OK before the first write but then would actually fail if you touched that property again.

Instead, you should handle OnStarting and do your header checks in there. That will make the code less brittle.

@Tratcher why did we add an explicit StartAsync before in SendFileAsync

PS: Replacing the body is more complicated than it seems. We made it a bit better in 3.0 by exposing a new response feature that encompasses all of the possible ways to mutate the response body (see StreamResponseBodyFeature).

[RickStrahl]

Response.OnStarting() - sigh - yeah that does look much easier to deal with assuming the headers are set at that point and can still be changed. I'll do that for an update.

Yeah creating a full stream just to replace the response is a pain especially for the content type scenario. I'll check out StreamResponseBodyFeature - hmmm... got any links or examples? Search comes up with no hits.

Part of the problem is discovering all these myriads of APIs :-)

[RickStrahl]

I do wonder if Response should throw if the ContentLength property actually has no effect in the same as Response.Headers.Remove() access throws after Response content has been written. Without this you can end up with some nasty bugs that might go unnoticed. For example in this scenario the page totally redners fine because only the injected script is not there.

[Tratcher]

The alternative to OnStarting is to replace IHttpResponseBodyFeature. StreamResponseBodyFeature might be a good staring place. The response compression middleware is a good example: https://github.com/aspnet/AspNetCore/blob/2de6c732d1f410460a627cb75aca91ae2c741fd5/src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs#L54-L71

@davidfowl StreamResponseBodyFeature.SendFileAsync calls StartAsync so that you can derive and override just StartAsync for scenarios like this.

[Tratcher]

Setting any header after Start/Write/Flush should throw. We added some fast paths for ContentLength, I'll see if they bypass the HasStarted checks.