-
Notifications
You must be signed in to change notification settings - Fork 25.1k
Update request-response.md #35289
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
Merged
Merged
Update request-response.md #35289
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
2d87bdc
Update request-response.md
Rick-Anderson b7bb02b
Apply suggestions from code review
Rick-Anderson 71c4c2d
response body
Rick-Anderson eb0c4d3
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 08043e2
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 5645802
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 66fcdba
Update request-response.md
Rick-Anderson 09bd0e0
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 9034fe1
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 8170607
Update request-response.md
Rick-Anderson b2579ac
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 7ef7ded
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson de437dc
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 22024ee
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 111f09a
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson 33868db
Update aspnetcore/fundamentals/middleware/request-response.md
Rick-Anderson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 understand that it is not possible to write a custom compression middleware with PipeWriter. Is that correct?
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.
That's what I tried to do here (line 28 change).
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.
@gfoidl @martincostello ???
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.
It's possible, but very awkward compared to Streams. W/ Streams it's quite easy, as the decorator pattern can be used to wrap a stream, which wraps a streams, etc.
W/ PipeWriter more custom code would be needed. E.g. an intermediate Pipeline that reads what the user's PipeWriter writes (and flushes), then perfom the compression and write (and flush) it the response body's PipeReader.
Also for compression .NET provides no easy to use infrastructure for this use case.
So as said it's possible, but w/ quite an effort. So I see no reason why one should use PipeWriter for this, when it's quite easy to accomplish with Streams.
Note: The Stream in the response is quite a simple adapter for the PipeWriter (at least in Kestrel), so there shouldn't be any perf-difference, especially when the data is sent over the network. So use either
BodyorBodyWriterdepending on the use case and on how it can be done more easily.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 think this is the piece that I was missing. The pipe and stream are different APIs for the same channel of data? For example, if my endpoint middleware writes to pipe and my compression middleware wraps the stream, my data will be compressed?
Could we explicit this part in this document 🙏
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.
Yep.
Underlying is the network socket, from which data is written / read. In oder to make these operations easier to use and abstracted away (because instead of network socket it could also be named pipes, unix domain sockets, etc. -- effort was done in ASP.NET Core with so called "project bedrock") there's pipes and streams, which allow "unified" access higher up in the stack.
W/o knowing your code I'm not able to say yes or no.
Interleaving PipeWriter and BodyStream is dangerous, as none of these is thread-safe. They're not intended to be used concurrent or mixed in any way.
So it really depends when you write / flush the pipe and what you'll do with the stream.
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.
Let's say my endpoint handler is just writing a protobuf object to the pipe
and I use the standard response compression middleware
I'm expecting the compression middleware to decorate the stream and not touch the pipe. I'm wondering if my response will be compressed.