Skip to content
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

Implement ZLibStream and fix SocketsHttpHandler deflate support #42717

Merged
merged 4 commits into from
Oct 9, 2020

Conversation

stephentoub
Copy link
Member

@stephentoub stephentoub commented Sep 25, 2020

  • 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

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.

Copy link
Member

@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
Member

carlossanlop commented Oct 6, 2020

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 prerelease. label Oct 7, 2020
@ghost ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https:/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
- Implements ZLibStream, exposes it in the ref, and add tests
- Fixes SocketsHttpHandler to use ZLibStream instead of DeflateStream
@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

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.
Projects
None yet
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