Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Can't reset the position of FrameRequestStream #1113

Closed
vic-yes opened this issue Sep 18, 2016 · 10 comments
Closed

Can't reset the position of FrameRequestStream #1113

vic-yes opened this issue Sep 18, 2016 · 10 comments

Comments

@vic-yes
Copy link

vic-yes commented Sep 18, 2016

I write a middleware to log the http request body, but after reading from Request.Body(FrameRequestStream), I can't read it again in other middleware.
Because the position of Body stream has been set to end.

Any ways to make me read it many times?

@Tratcher
Copy link
Member

This requires fully buffering the request. Kestrel will not do this for you, but there are some helpers available. https://github.com/aspnet/HttpAbstractions/blob/3e69df87f89601a5ede32c6765b736ab922b8fee/src/Microsoft.AspNetCore.Http/Internal/BufferingHelper.cs#L41

@davidfowl
Copy link
Member

That helper is "Internal" though.

@vic-yes
Copy link
Author

vic-yes commented Sep 19, 2016

Calling 'EnableRewind' method will change type of the 'Body' and it may affect the next execution of the middleware.

@vic-yes
Copy link
Author

vic-yes commented Sep 19, 2016

request.EnableRewind();
var body = request.body;
if (request.ContentLength != null && request.ContentLength > 0 && body != null)
{
byte[] buffer = new byte[request.ContentLength.Value];
body.Read(buffer, 0, (int)request.ContentLength.Value);
}

body.Read() will throw exception because the body has been disposed.

@davidfowl
Copy link
Member

You're hitting a different bug in kestrel that was fixed in 1.0.1 #1028.

Also, avoid synchronous reads on the body in general.

@benaadams
Copy link
Contributor

@563203132 you should upgrade to the released 1.0.1 patch https://blogs.msdn.microsoft.com/webdev/2016/09/13/asp-net-core-sept-2016-patch/

@benaadams
Copy link
Contributor

benaadams commented Sep 19, 2016

With 1.0.0 you have to restore the body at end to work around the bug:

var prebody = request.Body;
request.EnableRewind();
var body = request.Body;
if (request.ContentLength != null && request.ContentLength > 0)
{
    byte[] buffer = new byte[request.ContentLength.Value];
    await body.ReadAsync(buffer, 0, (int)request.ContentLength.Value);
}
// Add this
request.Body = prebody;

@davidfowl
Copy link
Member

I write a middleware to log the http request body

You might also want to consider just wrapping the request body and logging data as it's read rather than eagerly reading the thing into a byte[]. What happens if there's a large file upload on your site?

@vic-yes
Copy link
Author

vic-yes commented Sep 19, 2016

@benaadams I try your code and the 'body.ReadAsync' also throw exception because the body was disposed after calling the 'EnableRewind' method.

I upgrade to 1.0.1 and it's work.

@davidfowl I need to log the request at the begin of pipeline route.

Thank you very much.

@davidfowl
Copy link
Member

@davidfowl I need to log the request at the begin of pipeline route.

Beware of the memory concerns listed before. You don't want logging code to destroy your server performance.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants