search

davidfowl / AspNetCoreDiagnosticScenarios

This repository has examples of broken patterns in ASP.NET Core applications
7.69k stars 740 forks source link

When buffering, still check if response HasStarted #23

Closed Tratcher closed 5 years ago

Tratcher commented 5 years ago

https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blame/d319a43874ca5d25c5e12dcbba8f8bc71a81e102/AspNetCoreGuidance.md#L187-L200

When buffering you should still check Response.HasStarted before setting response headers (like the prior example. Buffers can be Flushed to start the response early.

davidfowl commented 5 years ago

@Tratcher feel free to send a PR to update the document.

davidfowl commented 5 years ago

Fixed, removed the buffering example and added an OnStarting example.

doggy8088 commented 5 years ago

@davidfowl Can I read the bufferred stream in the next middleware?

davidfowl commented 5 years ago

Sure. If it rewound.

doggy8088 commented 5 years ago

@davidfowl For example, I have two middlewares. 

app.UseResponseBuffering();
app.UseMiddleware<Middleware1>();
app.UseMiddleware<Middleware2>();

I have something need to write to Response.Body in the Middleware2. I want to read the whole buffer in the Middleware1 when my code run after await _next(); in the Middleware1. How can I achieve this? The ResponseBuffering's bufferred stream is write only. I can't read it. Can you give me a hint how to do this?

doggy8088 commented 5 years ago

@davidfowl I setup a simple repo to describe my question. Would you take a look? https://github.com/doggy8088/buffer-middleware-test/blob/master/Startup.cs#L30-L31

davidfowl commented 5 years ago

You can't read the response body, the stream is write only. If you want to read the output then you should assign a MemoryStream.

doggy8088 commented 5 years ago

@davidfowl Where is the MemoryStream should I assign to?

doggy8088 commented 5 years ago

@davidfowl I made it. Here is my code. Is my implementation okay? 😃

Tratcher commented 5 years ago

Stream _original and MemoryStream _buffered should not be class variables, they should be local variables inside Use. Otherwise you'll get cross contamination between requests. Is that why you had the null check on line 41?

Side notes: