-
Notifications
You must be signed in to change notification settings - Fork 67
Add request feature to support buffered/bufferless streams #170
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| return _bufferedStream; | ||
| } | ||
|
|
||
| throw new InvalidOperationException("InputStream must be prebuffered"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could remove this requirement and provide an API similar to what is described in #164 for HttpRequest.GetInputStreamAsync() that would do the awaiting there. This then could block and wait with the recommendation people move to the async version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, this blocks on framework so is not a real change. Non-blocking APIs are the now available GetBufferedStream and GetBufferlessStream
src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IHttpRequestAdapterFeature.cs
Outdated
Show resolved
Hide resolved
|
|
||
| return body; | ||
| } | ||
| set => _other.Body = value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should Mode be updated if the stream is replaced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_bufferedStream as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought about it, but wasn't sure how necessary it would be. Pushed a PR to reset in that case
Fixes #150