Skip to content

Implement ZLibStream and fix SocketsHttpHandler deflate support#42717

Merged
stephentoub merged 4 commits intodotnet:masterfrom
stephentoub:zlibstream
Oct 9, 2020
Merged

Implement ZLibStream and fix SocketsHttpHandler deflate support#42717
stephentoub merged 4 commits intodotnet:masterfrom
stephentoub:zlibstream

Conversation

@stephentoub
Copy link
Member

@stephentoub stephentoub commented Sep 25, 2020 •
edited
Loading

  • Implements ZLibStream, exposes it in the ref, and add tests
  • Fixes SocketsHttpHandler to use ZLibStream instead of DeflateStream to make HttpClient correctly support "deflate"

Fixes #2236
Fixes #38022
(This completes the commit from #2236 (comment).)

The tests require the updated runtime-assets test data from dotnet/runtime-assets#87.

cc: @ericstj @carlossanlop @geoffkizer

@Dotnet-GitSync-Bot
Copy link
Collaborator

Dotnet-GitSync-Bot commented Sep 25, 2020

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Sep 25, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@stephentoub stephentoub added this to the 6.0.0 milestone Sep 25, 2020
@runfoapp runfoapp bot mentioned this pull request Sep 28, 2020
Closed
carlossanlop
carlossanlop approved these changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left some minor documentation suggestions to consider.

@carlossanlop
Copy link
Contributor

carlossanlop commented Oct 6, 2020 •
edited
Loading

One of the CI failures is:

F:\workspace\_work\1\s\src\libraries\Common\tests\System\Net\Http\HttpClientHandlerTest.Decompression.cs(49,55): error CS0246:
The type or namespace name 'ZLibStream' could not be found (are you missing a using directive or an assembly reference?) 
[F:\workspace\_work\1\s\src\libraries\System.Net.Http.WinHttpHandler\tests\FunctionalTests\System.Net.Http.WinHttpHandler.Functional.Tests.csproj]

There are also some System.Net.Http failures, not sure if related to the changes here:
https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-42717-merge-fafaa6790ef2475e97/System.Net.Http.WinHttpHandler.Functional.Tests/console.4fe9d08c.log?sv=2019-07-07&se=2020-10-21T03%3A06%3A13Z&sr=c&sp=rl&sig=D1Z9SAXIL1Cy89j83v3WxACNT1ufGuVAMvFTLlVbw%2F0%3D

For example (the spanish error message says "operacion anulada" which means "aborted operation"):

System.Net.Http.HttpRequestException : Error while copying content to a stream.
      ---- System.IO.IOException : The read operation failed, see inner exception.
      -------- System.Net.Http.WinHttpException : Error -2147467260 calling WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, 'Operaci˘n anulada'.
      Stack Trace:
        /_/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs(343,0): at System.Net.Http.HttpClient.GetByteArrayAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
        /_/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs(89,0): at System.Net.Http.Functional.Tests.HttpClientHandler_Decompression_Test.<>c__DisplayClass4_0.<<DecompressedResponse_MethodSpecified_DecompressedContentReturned>b__0>d.MoveNext()
        --- End of stack trace from previous location ---
        /_/src/libraries/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs(96,0): at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks)
        /_/src/libraries/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs(124,0): at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks)
        /_/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs(88,0): at System.Net.Test.Common.LoopbackServer.<>c__DisplayClass13_0.<<CreateClientAndServerAsync>b__0>d.MoveNext()
        --- End of stack trace from previous location ---
        /_/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs(72,0): at System.Net.Test.Common.LoopbackServer.CreateServerAsync(Func`2 funcAsync, Options options)
        /_/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs(83,0): at System.Net.Http.Functional.Tests.HttpClientHandler_Decompression_Test.DecompressedResponse_MethodSpecified_DecompressedContentReturned(String encodingName, Func`2 compress, DecompressionMethods methods)
        --- End of stack trace from previous location ---
        ----- Inner Stack Trace -----
        /_/src/libraries/Common/src/System/Threading/Tasks/RendezvousAwaitable.cs(63,0): at System.Threading.Tasks.RendezvousAwaitable`1.GetResult()
        /_/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs(126,0): at System.Net.Http.WinHttpResponseStream.CopyToAsyncCore(Stream destination, Byte[] buffer, CancellationToken cancellationToken)
        /_/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs(339,0): at System.Net.Http.HttpClient.GetByteArrayAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
        ----- Inner Stack Trace -----
        /_/src/coreclr/src/System.Private.CoreLib/src/System/Environment.CoreCLR.cs(96,0): at System.Environment.get_StackTrace()
        /_/src/libraries/Common/src/System/Runtime/ExceptionServices/ExceptionStackTrace.cs(23,0): at System.Runtime.ExceptionServices.ExceptionStackTrace.AddCurrentStack(Exception exception)
        /_/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpException.cs(58,0): at System.Net.Http.WinHttpException.CreateExceptionUsingError(Int32 error, String nameOfCalledFunction)
        /_/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs(332,0): at System.Net.Http.WinHttpRequestCallback.OnRequestError(WinHttpRequestState state, WINHTTP_ASYNC_RESULT asyncResult)
        /_/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs(103,0): at System.Net.Http.WinHttpRequestCallback.RequestCallback(IntPtr handle, WinHttpRequestState state, UInt32 internetStatus, IntPtr statusInformation, UInt32 statusInformationLength)
        /_/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs(46,0): at System.Net.Http.WinHttpRequestCallback.WinHttpCallback(IntPtr handle, IntPtr context, UInt32 internetStatus, IntPtr statusInformation, UInt32 statusInformationLength)
        --- End of stack trace from AddCurrentStack ---

@stephentoub stephentoub added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Oct 7, 2020
@ghost ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 7, 2020
@stephentoub stephentoub force-pushed the zlibstream branch 2 times, most recently from cc8d4d6 to bc948e8 Compare October 8, 2020 21:20
stephentoub and others added 4 commits October 8, 2020 21:54
@stephentoub
Implement ZLibStream and fix SocketsHttpHandler deflate support
fc2cbea 
- Implements ZLibStream, exposes it in the ref, and add tests
- Fixes SocketsHttpHandler to use ZLibStream instead of DeflateStream
@stephentoub
Add comment about deflate content encoding
b72c7c7
@stephentoub @carlossanlop
Apply suggestions from code review
87c2fe4 
Co-authored-by: Carlos Sanchez <[email protected]>
@stephentoub
Fix netfx build
cb7442a
@stephentoub stephentoub merged commit d3beb60 into dotnet:master Oct 9, 2020
@stephentoub stephentoub deleted the zlibstream branch October 9, 2020 10:12
This was referenced Oct 12, 2020
Closed
Closed
@JamesNK JamesNK mentioned this pull request Nov 9, 2020
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@stephentoub stephentoub removed breaking-change Issue or PR that represents a breaking API or functional change over a previous release. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet labels Sep 30, 2021
@karelz
Copy link
Member

karelz commented Oct 4, 2021

@stephentoub you removed the breaking change labels ... is that because you implemented the detection that accepts both?
And there is no impact on server it seems ...

@stephentoub
Copy link
Member Author

stephentoub commented Oct 4, 2021

is that because you implemented the detection that accepts both?

Correct.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ericstj ericstj ericstj approved these changes

+2 more reviewers

@geoffkizer geoffkizer geoffkizer approved these changes

@carlossanlop carlossanlop carlossanlop approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@stephentoub stephentoub

Labels

area-System.Net.Http new-api-needs-documentation

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

HttpClient doesn't decompress "deflate" correctly Add support for zlib data format (RFC 1950)

6 participants

@stephentoub @Dotnet-GitSync-Bot @carlossanlop @karelz @ericstj @geoffkizer