Fix uncompressed scenarios with DisableBuffering #37022
Conversation
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1279589044 |
| InitializeCompressionHeaders(); | ||
| var compressionProvider = InitializeCompressionHeaders(); | ||
|
|
||
| if (_compressionProvider != null) |
There was a problem hiding this comment.
Does this leave a field that is never initialized now?
There was a problem hiding this comment.
This wasn't where it was initialized, only consumed. It's initialized over here:
aspnetcore/src/Middleware/ResponseCompression/src/ResponseCompressionBody.cs
Lines 266 to 272 in 59baa0d
The _compressionProvider field exists because there are two code paths that might cause it to be created, OnWrite and DisableBuffering, and we don't want to create it twice.
There was a problem hiding this comment.
But now the field is never assigned in OnWrite, so it will be created twice?
There was a problem hiding this comment.
Ah, I see - it ResolveCompressionProvider is called from InitialializeCompressionHeader, so it does end up being the same instance.
There was a problem hiding this comment.
It's assignment hasn't changed, that happens in ResolveCompressionProvider via:
- OnWrite()
- InitializeCompressionHeaders();
- ResolveCompressionProvider();
- InitializeCompressionHeaders();
The alternate code path is:
- DisableBuffering()
- ResolveCompressionProvider();
amcasey
added
area-middleware
5 participants
Contributes to #36960
The compression provider is created so long as the client's request allows compression. The compression stream should only be created if the response content-type is compressible.
DisableBuffering for uncompressed responses was regressed by #28464. If you called DisableBuffering then the compression stream would still get created but none of the compression headers would be set so the client wouldn't decompress.
The fix is to only create the compression stream if ShouldCompressResponse is true.
This should be backported to 6.0.