Skip to content

Buffer input stream for System.Web adapters#447

Merged
twsouthwick merged 16 commits into
mainfrom
tasou/buffer-input-stream
Mar 18, 2022
Merged

Buffer input stream for System.Web adapters#447
twsouthwick merged 16 commits into
mainfrom
tasou/buffer-input-stream

Conversation

@twsouthwick
Copy link
Copy Markdown
Contributor

@twsouthwick twsouthwick commented Mar 8, 2022

This sets up the input stream to be buffered and reads it all into memory to mimic the behavior of System.Web. This can be turned on per endpoint via the SystemWebAdapterAttribute metadata entry.

A WebApi endpoint has also been added to test similar behavior between the two frameworks that can be expanded as more features are brought online.

@twsouthwick twsouthwick force-pushed the tasou/buffer-input-stream branch 2 times, most recently from e4939fd to 572030c Compare March 8, 2022 22:26
@twsouthwick
Copy link
Copy Markdown
Contributor Author

@davidfowl @Tratcher I haven't done much with endpoint metadata, but this looks like a good way to allow configurability per endpoint for System.Web compatibility. I'm assuming "last one wins" is a decent enough way to manage it and I don't need to deal with merging multiple layers of metadata (i.e. through the GetOrderedMetadata<> call).

As always, feedback is appreciated!

@twsouthwick twsouthwick force-pushed the tasou/buffer-input-stream branch from 868bd5a to 10599f0 Compare March 10, 2022 17:51
Comment thread src/SystemWebAdapters/src/Internal/SystemWebAdapterMiddleware.cs Outdated
Comment thread src/SystemWebAdapters/src/SystemWebAdaptersExtensions.cs Outdated
twsouthwick and others added 2 commits March 10, 2022 11:57
Co-authored-by: David Fowler <davidfowl@gmail.com>
Comment thread src/SystemWebAdapters/src/HttpRequest.cs Outdated
Comment thread src/SystemWebAdapters/src/Internal/SystemWebAdapterMiddleware.cs Outdated
Comment thread src/SystemWebAdapters/src/Internal/SystemWebAdapterMiddleware.cs Outdated
@twsouthwick twsouthwick requested a review from Tratcher March 15, 2022 00:53
@twsouthwick twsouthwick force-pushed the tasou/buffer-input-stream branch from e19b8cc to 14ff346 Compare March 15, 2022 00:54
Comment thread src/SystemWebAdapters/src/Middleware/BufferRequestStreamMiddleware.cs Outdated
Comment thread src/SystemWebAdapters/src/Middleware/BufferRequestStreamMiddleware.cs Outdated
…ware.cs

Co-authored-by: David Fowler <davidfowl@gmail.com>
Comment thread src/SystemWebAdapters/src/BufferRequestStreamAttribute.cs Outdated
Comment thread src/SystemWebAdapters/src/Middleware/BufferRequestStreamMiddleware.cs Outdated
Comment thread src/SystemWebAdapters/src/SystemWebAdaptersExtensions.cs Outdated
Comment thread src/SystemWebAdapters/src/Metadata/IBufferRequestStreamMetadata.cs Outdated
Comment thread src/SystemWebAdapters/src/Middleware/BufferRequestStreamMiddleware.cs Outdated
Comment thread src/SystemWebAdapters/src/Adapters/PreBufferRequestStreamMiddleware.cs Outdated
Comment thread src/SystemWebAdapters/src/Adapters/SystemWebAdaptersExtensions.cs Outdated
@twsouthwick
Copy link
Copy Markdown
Contributor Author

@Tratcher I had to put back the services.AddSingleton<PreBufferRequestStreamMiddleware>(). Without that, I get an error about it not being registered. Maybe there's a different pattern you're thinking of?

@Tratcher
Copy link
Copy Markdown
Member

@Tratcher I had to put back the services.AddSingleton<PreBufferRequestStreamMiddleware>(). Without that, I get an error about it not being registered. Maybe there's a different pattern you're thinking of?

That might be because you're using IMiddleware, we don't usually use that. Here's the more common pattern:
https://github.com/dotnet/aspnetcore/blob/5fa80eb167ac3ae2c03590b135379fc3e3621175/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs#L43-L86

@twsouthwick twsouthwick merged commit f0c444c into main Mar 18, 2022
@twsouthwick twsouthwick deleted the tasou/buffer-input-stream branch March 18, 2022 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants