Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trouble reading stream from HttpContext.Response.Body in an ActionFilterAttribute #487

Closed
guardrex opened this issue Apr 27, 2015 · 28 comments
Labels

Comments

@guardrex
Copy link
Contributor

Might be an issue ... or more likely ... my noob 馃懚 status with MVC. I can post on SO if this definitely not an "issue."

I'm setting up an ActionFilterAttribute to do some OnActionExecuted post processing of content. I can write to the HttpContext.Response.Body with a StreamWriter ... that works just fine. I'm trying to get the content so I can work with it before writing it out, but attempts to use HttpContext.Response.Body constantly give me Stream was not readable. How do I read Response.Body here, or is the fact that I can't read the stream an issue?

This works fine and will shoot the text out as the output of the View() when the ActionResult has the filter annotation:

public class PostProcessFilterAttribute : ActionFilterAttribute {
    public override void OnActionExecuted(ActionExecutedContext filterContext) {
        var content = filterContext.HttpContext.Response;
        using (var responseWriter = new StreamWriter(content.Body, Encoding.UTF8)) {
            responseWriter.Write("This is some text to write!");
        }
        base.OnActionExecuted(filterContext);
    }
}

This attempt to read the Response.Body chokes:

public class PostProcessFilterAttribute : ActionFilterAttribute {
    public override void OnActionExecuted(ActionExecutedContext filterContext) {
        var content = filterContext.HttpContext.Response;
        // Next line throws "Stream was not readable"
        using (var responseReader = new StreamReader(content.Body)) {
            // I'd like to do some work with the stream here before shooting it back out
            using (var responseWriter = new StreamWriter(content.Body, Encoding.UTF8)) {
                responseWriter.Write(responseReader.ReadToEnd());
            }
        }
        base.OnActionExecuted(filterContext);
    }
}
@guardrex
Copy link
Contributor Author

There is one one SO answer that seems relevant (same exception). The post suggests something like:

var buffer = new MemoryStream();
var stream = filterContext.HttpContext.Response.Body;
filterContext.HttpContext.Response.Body = buffer;
buffer.Seek(0, SeekOrigin.Begin);
var reader = new StreamReader(buffer);
string responseBody = reader.ReadToEnd();                    
MemoryStream msNew = new MemoryStream();
using (StreamWriter wr = new StreamWriter(msNew)) {
    wr.WriteLine("This is my new content");
    wr.Flush();
    msNew.Seek(0, SeekOrigin.Begin);
    msNew.CopyTo(stream);
}

... but that is not working and resulting in a reset connection.

@benaadams
Copy link
Member

There is one one SO answer that seems relevant (same exception).

In the SO post they replace the response body stream (probably as the first stage in the pipeline) with a memory stream, then run the rest of the pipeline on it:

await next();

Then edit the response, before flushing it to the original response body stream:

await msNew.CopyToAsync(stream);

I imagine the original response body stream would be CanSeek == false and CanRead == false as it can flush to the network rather than trying to keep the entire response in memory.

@guardrex
Copy link
Contributor Author

Turns out that MVC handles the responses completely if it comes before the SO answer code in Configure. I'm looking for a post-processing option after Mvc has done it's bit.

Will someone on the team confirm ...

I imagine the original response body stream would be CanSeek == false and CanRead == false as it can flush to the network rather than trying to keep the entire response in memory.

... and does that mean that I will never be able to get the rendering stream in ActionFilterAttribute/OnActionExecuted?

... if things aren't going to work out for me that way, how about setting up an HttpResponse.Filter? ... (would that be an IFilter?) for my ActionFilterAttribute/OnActionExecuted?

@nil4
Copy link
Contributor

nil4 commented Apr 29, 2015

@guardrex: it appears you found the answer to your questions since you closed this issue. Could you please share the information you found?

@guardrex
Copy link
Contributor Author

@nil4 The ActionFilterAttribute - ActionExecutedContext doesn't expose the response stream that can be read as @benaadams suggested. I still think it's strange (because there are examples online showing in earlier frameworks/versions of MVC that it can be read and written), but that's the way they have it setup.

I ended up going with a work-around to compress (GZip) output: I'm using IIS to compress the app's output. I couldn't set compression at the application level in IIS ... I had to set it at the Sites level, because Web Deploy will reset the Dynamic Content Compression if you only activate it at the app level; however, if set at the Sites level, the app will inherit the setting after deployment. [I reported this Web Deploy behavior on the IIS Forums, but apparently, this is normal behavior for Web Deploy.]

I would like to investigate using a global filter:

public static void RegisterGlobalFilters(GlobalFilterCollection filters)
{
    filters.Add(new GZipStream(Context.HttpContext.Response.Body, CompressionMode.Compress));
}

... but I don't know if the stream will be available for a filter either, and I haven't researched how to implement a global filter yet.

Actually, I don't really prefer either of these approaches. I would much prefer to decorate individual controller actions for compression with an ActionFilterAttribute. That would rock!

Anyway ... I'm going to re-open since I'm not the only one interested in this issue and would still like to get full control of that stream there. [Also note to @stephentoub ... since this might be a CoreFX thing.]

@guardrex guardrex reopened this Apr 29, 2015
@benaadams
Copy link
Member

@guardrex You'd need someone better informed for Attribute use in MVC6 to comment; but for what you are trying to do you can probably do this, if you insert it before app.UseMvc(); in your Startup class:

app.Use(async (context, next) =>
{
    // Set Gzip stream.
    context.Response.Headers.Item("Content-Encoding") = "gzip";
    // Wrap response body in Gzip stream.
    var body = context.Response.Body; 
    context.Response.Body = new GZipStream( body, CompressionMode.Compress);
    await next();
});

Though that will compress everything, which might not be what you want

@guardrex
Copy link
Contributor Author

@benaadams I was hoping for an attribute that could decorate individual controller actions for fine control, but I like your code sample. Being new to the pipeline, I might be asking a "dumb" question:

I'm confused about how the pipeline works here in that you say to put that app.Use(...) code before the app.UseMvc(). I thought that these ran in sequence. I would perhaps expect a filter to work that way, but it seems strange that the response would be compressed before Mvc does it's request/response work. I know the flip-side of this doesn't work ... when I put app.UseMvc() before an app.Use(...), Mvc handles the entire request/response ... like it doesn't call a next() at the end ... but maybe that's an option that I can set in the app.UseMvc() and I'm just not aware of how to do that.

@benaadams
Copy link
Member

I thought that these ran in sequence. I would perhaps expect a filter to work that way, but it seems strange that the response would be compressed before Mvc does it's request/response work.

What you are thinking is correct; in the example I'm wrapping the response stream in a gzip stream, which needs to happen before its written to, rather than after.

In the SO example you linked to; again they did the same; however they swapped the response stream for a memory stream, when you can then seek and read, and after await next(); is called the rest of the pipeline after has run so you then have result in the memory stream; which you can do what you want with; and then copy the result back to the original response stream.

The app.Use( ... ) items handle both before and after chosen by you where you call await next(); its quite a powerful system.

@guardrex
Copy link
Contributor Author

@benaadams I see ... and you can see how "linear" my thinking was with regard how streams process data. I simply didn't get it until now. Thanks for clearing that up for me.

I'm going to run one more round of testing later today and report back. I'll leave this open just a bit longer in case one of the team members wants to say if it's impossible by design that the ActionExecutedContext response stream is going to remain unreadable ... or maybe that wasn't what they intended.

@nil4
Copy link
Contributor

nil4 commented Apr 29, 2015

@guardrex: thank you for reopening the issue!

@benaadams
Copy link
Member

Well ideally you are right and you'd want to do it selectively by marking controllers with attributes; but alas I'm not sure how you'd do that and blend the two approaches; is probably a way.

@benaadams
Copy link
Member

@guardrex you could with an ActionFilter that runs before rather than after and wrap the stream in it

@guardrex
Copy link
Contributor Author

@benaadams The global case (and using IIS as I'm doing now) were truly work-around approaches. I really just wanted to use attributes and not have any global response processing (if that's what you meant by 'blending' there).

The reason this all came about was that there were some edge cases where gzipping the response would break the page I was rendering. Those were really "edge" cases. For example, I had a problem in one app gzipping some Google Maps markup. That issue isn't a problem right now. I just thought it would be cool if we could post-process responses via attributes and get a lot of fine control. It has very wide applications well beyond compression ... if we could get that response context body after the controller action runs, you could do anything under the Sun with it ... that would be cool.

you could with an ActionFilter that runs before rather than after and wrap the stream in it

Yes, I hope the stream is readable in that context. I can't remember if I tried that or not. I'll give it a shot this evening and report back.

@guardrex
Copy link
Contributor Author

@benaadams With this:

public void Configure(IApplicationBuilder app) {
    app.UseErrorPage();
    app.UseStaticFiles();
    app.Use(async (context, next) => {
        context.Response.Headers.Append("Content-Encoding", "gzip");
        context.Response.Body = new System.IO.Compression.GZipStream(context.Response.Body, System.IO.Compression.CompressionMode.Compress);
        await next();
    });
    app.UseMvc();
}

... no response ... a 200 comes back but with no response body (Fiddler confirms there is no response body). It's as though the next() is not passing to Mvc.

In attempting to apply the ActionFilter before the action to wrap the stream ...

public override void OnActionExecuting(ActionExecutingContext filterContext) {...}

... I still get a Stream was not readable exception.

I think I'll be ok with IIS Compression, but not being able to read the stream in the ActionFilterAttribute is still a general issue until I hear that it's by design.

@benaadams
Copy link
Member

Try flushing the stream at end

public void Configure(IApplicationBuilder app) {
    app.UseErrorPage();
    app.UseStaticFiles();
    app.Use(async (context, next) => {
        context.Response.Headers.Append("Content-Encoding", "gzip");
        context.Response.Body = new System.IO.Compression.GZipStream(context.Response.Body, System.IO.Compression.CompressionMode.Compress);
        await next();
        await context.Response.Body.FlushAsync();
    });
    app.UseMvc();
}````

@guardrex
Copy link
Contributor Author

@benaadams Same response ... 200 OK but with no content.

HTTP/1.1 200 OK
Cache-Control: public,max-age=2592000
Content-Type: text/html; charset=utf-8
Vary: Accept-Encoding
Server: Microsoft-HTTPAPI/2.0
Date: Thu, 30 Apr 2015 03:40:35 GMT
Content-Length: 0

@rynowak
Copy link
Member

rynowak commented Apr 30, 2015

Sorry for just noticing this now. A few points.

/cc @Tratcher in case he wants to elaborate.

1). If you're on IIS why not just use dynamic compression at the server level? https://technet.microsoft.com/en-us/library/cc753681%28v=ws.10%29.aspx

2). Our default streams are not intended to be read. By default we do some buffering in these streams, but we don't buffer the whole response because that would use an awful lot of memory. Once the amount of data we're willing to buffer has been written, it goes out on the wire and we don't store it any more. There are some techniques to enable buffering, and one of them is to replace the stream.

3). If you want to mess with the response stream for an MVC action (or all MVC actions), the right filter stage is a result filter (IResultFilter or IAsyncResultFilter).

Thing of it this way, calling View() in your controller is creating an IActionResult, which will be executed by MVC later, it's not actually executing the view inline, and that's why messing with the stream in that phase doesn't do what you want.

Result Filters actually surround the execution of the view code.

4). If you want to mess with the response stream for your whole application and don't want to/can't use gzip support provided by your server, then middleware is a good choice. (Compare to filters, which allow you to scope to Controller/Action).

If you still having issues with the middleware approach used above:

  • have you tried setting a break point in the action in question to make sure it's actually hit?
  • have you removed your action filter code (which wouldn't do what you want)?

@guardrex
Copy link
Contributor Author

@rynowak Thank you ... that's very helpful. I didn't understand how the streams were being handled.

Yes, I seem to favor IIS Dynamic Content Compression. I have that setup in IIS now, and it is working fine for me. That's probably what I'll stick with. I'm not happy that Web Deploy turns off Dynamic Content Compression for an app that has it set at the app level in IIS. I must set it at the Sites level to get it to stick each time I deploy that app, but of course that also has the effect of applying the compression to all sites in IIS. I asked the Web Deploy folks in the IIS Forums, and they said it's supposed to work that way but didn't tell me the reason. A related issue is Web Deploy removing manually-set Application Settings on a deploy ... also very annoying, since I was hoping to leverage .UseEnvironmentSettings() and have those manual IIS app settings override the ones I have in the app (so the behavior of an Azure-VM/IIS-hosted app doesn't match an Azure-Web Apps-hosted app, where I think the Azure app settings stick on a deployment with Web Deploy). These are somewhat unrelated here, but I mention them in passing in case you agree with me and want to help direct these issues to the right person/people there.

I wasn't aware of the IResultFilter/IAsyncResultFilter ... I'm new to MVC and online examples for compression didn't mention those interfaces. I'll give that a shot.

The answer to your last two questions is 'yes ... sort of' and 'yes'. I know the action is hit normally, because when I comment out the app.Use(...) that @benaadams and I were discussing that the app behaves normally and I get a response body ... the normal view. When I have that code active, I get a 200 OK with no response body. btw-- The reason I can't set a breakpoint at the moment is that I'm using Brackets, and I'm not very good with it. I just installed VS 2015 RC last night, so I'll play with that bit more shortly in a real IDE. And, yes, I did remove the prior action filter code when testing the middleware approach.

@EvanMulawski
Copy link

Any update on this? I have tried IAsyncResultFilter and creating a middleware, but it only works for the first request/response. Subsequent responses are empty or fail with "The base stream is not writable." It looks like the stream for the response body is being reused for other requests.

using (var memoryStream = new MemoryStream())
{
    var stream = context.Response.Body;
    context.Response.Body = memoryStream;
    await _next();

    if (acceptEncoding.Contains("gzip"))
    {
        using (var compressedStream = new GZipStream(stream, CompressionLevel.Optimal))
        {
            context.Response.Headers.Add("Content-Encoding", new[] { "gzip" });
            memoryStream.Seek(0, SeekOrigin.Begin);
            await memoryStream.CopyToAsync(compressedStream);
        }
    }
    else if (acceptEncoding.Contains("deflate"))
    {
        using (var compressedStream = new DeflateStream(stream, CompressionLevel.Optimal))
        {
            context.Response.Headers.Add("Content-Encoding", new[] { "gzip" });
            memoryStream.Seek(0, SeekOrigin.Begin);
            await memoryStream.CopyToAsync(compressedStream);
        }
    }
}

@benaadams
Copy link
Member

@evan-mulawski its a Kestrel bug that has been fixed post RTM aspnet/KestrelHttpServer#940

Workaround is to reassign the stream back to the original at the end of the using block:

context.Response.Body = stream;

@benaadams
Copy link
Member

Fix for reference is: aspnet/KestrelHttpServer#955

@EvanMulawski
Copy link

Thanks for the quick reply! That workaround did the trick.

@ghost
Copy link

ghost commented Jun 18, 2018

@EvanMulawski Hi, it is really working for you ? In my own middleware i can read the body request but the response is still not readable.

@davidfowl
Copy link
Member

@sorcer1 you can't read the response. It's a write only stream. If you want to read what was written then you need to assign a new stream (like a memory stream or a custom stream) and read after responses are written.

@ghost
Copy link

ghost commented Jun 19, 2018

I've tried but the body reponse was empty now and in the log there's only invalid characters. The response exceptes was json data.

public async Task Invoke(HttpContext context)
     {
         using (var memoryStream = new MemoryStream())
         {
             var stream = context.Response.Body;
             context.Response.Body = memoryStream;

             await next(context);

             var responseBody = await new StreamReader(memoryStream).ReadToEndAsync();
             logger?.LogDebug($"Response: {responseBody}");

             context.Response.Body = stream;
         }
     }

@Tratcher
Copy link
Member

A) you forgot to rewind the memory stream before reading it. Call Seek.
B) You can't just assign the stream back to Body at the end, you must CopyToAsync to get the data across to the right stream.

@ghost
Copy link

ghost commented Jun 19, 2018

Ok for the async copy, i have my response now. But not in the logger ! :(

@santoshpatro
Copy link

I am facing similar kind of issue.I have posted the question at https://stackoverflow.com/questions/53364132/system-argumentexception-hresult-0x80070057-message-stream-is-now-writable-param

Can any one help me here to fix this issue?

ryanbrandenburg pushed a commit that referenced this issue Nov 22, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

8 participants