This repository has been archived by the owner. It is now read-only.

When HttpResponse.Body is replaced, the replacement is used for future requests #940

Closed
jodavis opened this Issue Jun 23, 2016 · 4 comments

Comments

Projects
None yet
6 participants
@jodavis
Copy link

jodavis commented Jun 23, 2016

This is a contrived example to illustrate the point:

app.Use(next => context =>
{
    if (context.Response.Body is MemoryStream)
    {
        throw new Exception("Wrong stream type in context.Response.Body");
    }

    context.Response.Body = new MemoryStream();

    return context.Response.WriteAsync("Hello, world!");
});

Run the application and browse to it. The first request returns nothing (content was written to the MemoryStream). Subsequent requests will hit the Exception.

The real-world case is when the Body stream is decorated for a given request. The decorator stream will be re-used for a subsequent request, which could result in re-decorating the stream, and ultimately produce a long chain of decorators.

It appears that Kestrel is trying to be efficient by re-using its underlying Stream instances. But the reusable instances are being replaced, with unintended consequences for future requests.

@halter73

This comment has been minimized.

Copy link
Member

halter73 commented Jun 24, 2016

Nice catch. These lines should be moved outside the if condition so the streams get reset between every request.

@halter73 halter73 added the bug label Jun 24, 2016

@halter73 halter73 added this to the 1.0.1 milestone Jun 24, 2016

@benaadams

This comment has been minimized.

Copy link
Contributor

benaadams commented Jun 25, 2016

@halter73 good follow though; couldn't find what situation InitializeStreams wasn't called

@davidfowl

This comment has been minimized.

Copy link
Member

davidfowl commented Jun 26, 2016

Eeek 😄

@cesarbs cesarbs self-assigned this Jun 27, 2016

@muratg muratg added the 1 - Ready label Jun 28, 2016

@halter73

This comment has been minimized.

Copy link
Member

halter73 commented Jun 28, 2016

@jodavis As a workaround until we get a new release out, you can store the original Body before replacing it and set it back before the middleware exits. Body is the same stream for all requests on a given connection when it's not replaced.

@cesarbs cesarbs added 2 - Working and removed 1 - Ready labels Jul 5, 2016

cesarbs added a commit that referenced this issue Jul 6, 2016

cesarbs added a commit that referenced this issue Jul 6, 2016

@cesarbs cesarbs closed this Jul 6, 2016

@cesarbs cesarbs added 3 - Done and removed 2 - Working labels Jul 6, 2016

xabikos added a commit to xabikos/aspnet-webpack that referenced this issue Jul 24, 2016

#23 Upgrade the version of Webpack project in order to publish it and…
… add a workaround for correctly replacing the response because of a bug in kestrel aspnet/KestrelHttpServer#940
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.