From df5dcd18d15cdd7445ccb88bbe2cd19ce7e18f4e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 24 Dec 2020 22:48:57 +1300 Subject: [PATCH 01/22] WinHttpHandler: Read trailing headers --- .../Windows/WinHttp/Interop.winhttp_types.cs | 1 + .../Common/src/System/Net/SecurityProtocol.cs | 2 +- .../src/System.Net.Http.WinHttpHandler.csproj | 4 +- .../System/Net/Http/WinHttpResponseParser.cs | 78 +++++-- .../System/Net/Http/WinHttpResponseStream.cs | 51 ++++- ...ttp.WinHttpHandler.Functional.Tests.csproj | 7 +- .../FunctionalTests/TrailingHeadersTest.cs | 215 ++++++++++++++++++ .../UnitTests/WinHttpResponseStreamTest.cs | 2 +- 8 files changed, 328 insertions(+), 32 deletions(-) create mode 100644 src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs diff --git a/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs b/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs index aac3f8091a9134..788c31e92230fe 100644 --- a/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs +++ b/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs @@ -69,6 +69,7 @@ internal partial class WinHttp public const uint WINHTTP_QUERY_STATUS_TEXT = 20; public const uint WINHTTP_QUERY_RAW_HEADERS = 21; public const uint WINHTTP_QUERY_RAW_HEADERS_CRLF = 22; + public const uint WINHTTP_QUERY_FLAG_TRAILERS = 0x02000000; public const uint WINHTTP_QUERY_CONTENT_ENCODING = 29; public const uint WINHTTP_QUERY_SET_COOKIE = 43; public const uint WINHTTP_QUERY_CUSTOM = 65535; diff --git a/src/libraries/Common/src/System/Net/SecurityProtocol.cs b/src/libraries/Common/src/System/Net/SecurityProtocol.cs index f6de449ef3b8d5..a8ff9b3cbc6984 100644 --- a/src/libraries/Common/src/System/Net/SecurityProtocol.cs +++ b/src/libraries/Common/src/System/Net/SecurityProtocol.cs @@ -8,7 +8,7 @@ namespace System.Net internal static class SecurityProtocol { public const SslProtocols DefaultSecurityProtocols = -#if !NETSTANDARD2_0 && !NETFRAMEWORK +#if !NETSTANDARD2_0 && !NETSTANDARD2_1 && !NETFRAMEWORK SslProtocols.Tls13 | #endif SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12; diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj index 8e62c79985b8c0..0761a799f64100 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj @@ -1,7 +1,7 @@ - + true - netstandard2.0-windows;netstandard2.0;net461-windows + netstandard2.0-windows;netstandard2.0;netstandard2.1-windows;netstandard2.1;net461-windows true enable diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs index 1f9c88a019a91a..5196fc868a9732 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs @@ -29,7 +29,7 @@ public static HttpResponseMessage CreateResponseMessage( // Create a single buffer to use for all subsequent WinHttpQueryHeaders string interop calls. // This buffer is the length needed for WINHTTP_QUERY_RAW_HEADERS_CRLF, which includes the status line // and all headers separated by CRLF, so it should be large enough for any individual status line or header queries. - int bufferLength = GetResponseHeaderCharBufferLength(requestHandle, Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF); + int bufferLength = GetResponseHeaderCharBufferLength(requestHandle, Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF, isTrailingHeaders: false); char[] buffer = ArrayPool.Shared.Rent(bufferLength); try { @@ -58,7 +58,7 @@ public static HttpResponseMessage CreateResponseMessage( string.Empty; // Create response stream and wrap it in a StreamContent object. - var responseStream = new WinHttpResponseStream(requestHandle, state); + var responseStream = new WinHttpResponseStream(requestHandle, state, response); state.RequestHandle = null; // ownership successfully transfered to WinHttpResponseStram. Stream decompressedStream = responseStream; @@ -93,7 +93,7 @@ public static HttpResponseMessage CreateResponseMessage( response.RequestMessage = request; // Parse raw response headers and place them into response message. - ParseResponseHeaders(requestHandle, response, buffer, stripEncodingHeaders); + ParseResponseHeaders(requestHandle, Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF, response, buffer, stripEncodingHeaders, isTrailers: false); if (response.RequestMessage.Method != HttpMethod.Head) { @@ -223,7 +223,7 @@ private static unsafe int GetResponseHeader(SafeWinHttpHandle requestHandle, uin /// /// Returns the size of the char array buffer. /// - private static unsafe int GetResponseHeaderCharBufferLength(SafeWinHttpHandle requestHandle, uint infoLevel) + public static unsafe int GetResponseHeaderCharBufferLength(SafeWinHttpHandle requestHandle, uint infoLevel, bool isTrailingHeaders) { char* buffer = null; int bufferLength = 0; @@ -233,11 +233,21 @@ private static unsafe int GetResponseHeaderCharBufferLength(SafeWinHttpHandle re { int lastError = Marshal.GetLastWin32Error(); - Debug.Assert(lastError != Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND); + if (!isTrailingHeaders) + { + Debug.Assert(lastError != Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND); - if (lastError != Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER) + if (lastError != Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER) + { + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); + } + } + else { - throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); + if (!(lastError == Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER || lastError == Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND)) + { + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); + } } } @@ -286,24 +296,39 @@ private static string GetReasonPhrase(HttpStatusCode statusCode, char[] buffer, new string(buffer, 0, bufferLength); } - private static void ParseResponseHeaders( + private class HttpResponseTrailers : HttpHeaders + { + } + + public static void ParseResponseHeaders( SafeWinHttpHandle requestHandle, + uint infoLevel, HttpResponseMessage response, char[] buffer, - bool stripEncodingHeaders) + bool stripEncodingHeaders, + bool isTrailers) { HttpResponseHeaders responseHeaders = response.Headers; HttpContentHeaders contentHeaders = response.Content.Headers; +#if NETSTANDARD2_1 + HttpResponseHeaders responseTrailers = response.TrailingHeaders; +#else + HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); + response.RequestMessage.Properties["__ResponseTrailers"] = responseTrailers; +#endif int bufferLength = GetResponseHeader( requestHandle, - Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF, + infoLevel, buffer); var reader = new WinHttpResponseHeaderReader(buffer, 0, bufferLength); - // Skip the first line which contains status code, etc. information that we already parsed. - reader.ReadLine(); + if (!isTrailers) + { + // Skip the first line which contains status code, etc. information that we already parsed. + reader.ReadLine(); + } // Parse the array of headers and split them between Content headers and Response headers. string headerName; @@ -311,22 +336,29 @@ private static void ParseResponseHeaders( while (reader.ReadHeader(out headerName, out headerValue)) { - if (!responseHeaders.TryAddWithoutValidation(headerName, headerValue)) + if (!isTrailers) { - if (stripEncodingHeaders) + if (!responseHeaders.TryAddWithoutValidation(headerName, headerValue)) { - // Remove Content-Length and Content-Encoding headers if we are - // decompressing the response stream in the handler (due to - // WINHTTP not supporting it in a particular downlevel platform). - // This matches the behavior of WINHTTP when it does decompression itself. - if (string.Equals(HttpKnownHeaderNames.ContentLength, headerName, StringComparison.OrdinalIgnoreCase) || - string.Equals(HttpKnownHeaderNames.ContentEncoding, headerName, StringComparison.OrdinalIgnoreCase)) + if (stripEncodingHeaders) { - continue; + // Remove Content-Length and Content-Encoding headers if we are + // decompressing the response stream in the handler (due to + // WINHTTP not supporting it in a particular downlevel platform). + // This matches the behavior of WINHTTP when it does decompression itself. + if (string.Equals(HttpKnownHeaderNames.ContentLength, headerName, StringComparison.OrdinalIgnoreCase) || + string.Equals(HttpKnownHeaderNames.ContentEncoding, headerName, StringComparison.OrdinalIgnoreCase)) + { + continue; + } } - } - contentHeaders.TryAddWithoutValidation(headerName, headerValue); + contentHeaders.TryAddWithoutValidation(headerName, headerValue); + } + } + else + { + responseTrailers.TryAddWithoutValidation(headerName, headerValue); } } } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs index 812a56e7232cff..e8b4b30d375194 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs @@ -16,11 +16,14 @@ internal sealed class WinHttpResponseStream : Stream { private volatile bool _disposed; private readonly WinHttpRequestState _state; + private readonly HttpResponseMessage _responseMessage; private SafeWinHttpHandle _requestHandle; + private bool _readTrailingHeaders; - internal WinHttpResponseStream(SafeWinHttpHandle requestHandle, WinHttpRequestState state) + internal WinHttpResponseStream(SafeWinHttpHandle requestHandle, WinHttpRequestState state, HttpResponseMessage responseMessage) { _state = state; + _responseMessage = responseMessage; _requestHandle = requestHandle; } @@ -126,6 +129,7 @@ private async Task CopyToAsyncCore(Stream destination, byte[] buffer, Cancellati int bytesAvailable = await _state.LifecycleAwaitable; if (bytesAvailable == 0) { + ReadResponseTrailers(); break; } Debug.Assert(bytesAvailable > 0); @@ -142,12 +146,17 @@ private async Task CopyToAsyncCore(Stream destination, byte[] buffer, Cancellati int bytesRead = await _state.LifecycleAwaitable; if (bytesRead == 0) { + ReadResponseTrailers(); break; } Debug.Assert(bytesRead > 0); // Write that data out to the output stream +#if NETSTANDARD2_1 + await destination.WriteAsync(buffer.AsMemory(0, bytesRead), cancellationToken).ConfigureAwait(false); +#else await destination.WriteAsync(buffer, 0, bytesRead, cancellationToken).ConfigureAwait(false); +#endif } } finally @@ -240,7 +249,14 @@ private async Task ReadAsyncCore(byte[] buffer, int offset, int count, Canc } } - return await _state.LifecycleAwaitable; + int bytesRead = await _state.LifecycleAwaitable; + + if (bytesRead == 0) + { + ReadResponseTrailers(); + } + + return bytesRead; } finally { @@ -249,6 +265,37 @@ private async Task ReadAsyncCore(byte[] buffer, int offset, int count, Canc } } + private void ReadResponseTrailers() + { + // Only load response trailers if: + // 1. HTTP/2 or later + // 2. Response trailers not already loaded + if (_readTrailingHeaders || _responseMessage.Version < WinHttpHandler.HttpVersion20) + { + return; + } + + _readTrailingHeaders = true; + + var bufferLength = WinHttpResponseParser.GetResponseHeaderCharBufferLength( + _requestHandle, + Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF | Interop.WinHttp.WINHTTP_QUERY_FLAG_TRAILERS, + isTrailingHeaders: true); + + if (bufferLength != 0) + { + char[] trailersBuffer = ArrayPool.Shared.Rent(bufferLength); + try + { + WinHttpResponseParser.ParseResponseHeaders(_requestHandle, Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF | Interop.WinHttp.WINHTTP_QUERY_FLAG_TRAILERS, _responseMessage, trailersBuffer, stripEncodingHeaders: false, isTrailers: true); + } + finally + { + ArrayPool.Shared.Return(trailersBuffer); + } + } + } + public override int Read(byte[] buffer, int offset, int count) { return ReadAsync(buffer, offset, count, CancellationToken.None).GetAwaiter().GetResult(); diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj index 4bbd2f502e116a..481eca84a0eae4 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj @@ -14,11 +14,11 @@ - + + Link="Common\System\IO\DelegateStream.cs" /> + Link="Common\System\Net\Http\ResponseStreamTest.cs" /> + diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs new file mode 100644 index 00000000000000..4414a713956e30 --- /dev/null +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs @@ -0,0 +1,215 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Net.Http.Functional.Tests; +using System.Net.Http.Headers; +using System.Net.Test.Common; +using System.Text; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace System.Net.Http.WinHttpHandlerFunctional.Tests +{ + public class TrailingHeadersTest : HttpClientHandlerTestBase + { + public TrailingHeadersTest(ITestOutputHelper output) : base(output) + { } + + protected override Version UseVersion => new Version(2, 0); + + protected static byte[] DataBytes = Encoding.ASCII.GetBytes("data"); + + protected static readonly IList TrailingHeaders = new HttpHeaderData[] { + new HttpHeaderData("MyCoolTrailerHeader", "amazingtrailer"), + new HttpHeaderData("EmptyHeader", ""), + new HttpHeaderData("Accept-Encoding", "identity,gzip"), + new HttpHeaderData("Hello", "World") }; + + protected static Frame MakeDataFrame(int streamId, byte[] data, bool endStream = false) => + new DataFrame(data, (endStream ? FrameFlags.EndStream : FrameFlags.None), 0, streamId); + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public async Task Http2GetAsync_NoTrailingHeaders_EmptyCollection() + { + using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) + using (HttpClient client = CreateHttpClient()) + { + Task sendTask = client.GetAsync(server.Address); + + Http2LoopbackConnection connection = await server.EstablishConnectionAsync(); + + int streamId = await connection.ReadRequestHeaderAsync(); + + // Response header. + await connection.SendDefaultResponseHeadersAsync(streamId); + + // Response data. + await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes, endStream: true)); + + // Server doesn't send trailing header frame. + HttpResponseMessage response = await sendTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var trailingHeaders = GetTrailingHeaders(response); + Assert.NotNull(trailingHeaders); + Assert.Equal(0, trailingHeaders.Count()); + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() + { + using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) + using (HttpClient client = CreateHttpClient()) + { + Task sendTask = client.GetAsync(server.Address); + + Http2LoopbackConnection connection = await server.EstablishConnectionAsync(); + + int streamId = await connection.ReadRequestHeaderAsync(); + + // Response header. + await connection.SendDefaultResponseHeadersAsync(streamId); + + // Response data, missing Trailers. + await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes)); + + // Additional trailing header frame. + await connection.SendResponseHeadersAsync(streamId, isTrailingHeader: true, headers: TrailingHeaders, endStream: true); + + HttpResponseMessage response = await sendTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var trailingHeaders = GetTrailingHeaders(response); + Assert.Equal(TrailingHeaders.Count, trailingHeaders.Count()); + Assert.Contains("amazingtrailer", trailingHeaders.GetValues("MyCoolTrailerHeader")); + Assert.Contains("World", trailingHeaders.GetValues("Hello")); + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Available() + { + using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) + using (HttpClient client = CreateHttpClient()) + { + Task sendTask = client.GetAsync(server.Address, HttpCompletionOption.ResponseHeadersRead); + + Http2LoopbackConnection connection = await server.EstablishConnectionAsync(); + + int streamId = await connection.ReadRequestHeaderAsync(); + + // Response header. + await connection.SendDefaultResponseHeadersAsync(streamId); + + // Response data, missing Trailers. + await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes)); + + HttpResponseMessage response = await sendTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + // Pending read on the response content. + var trailingHeaders = GetTrailingHeaders(response); + Assert.True(trailingHeaders == null || trailingHeaders.Count() == 0); + + Stream stream = await response.Content.ReadAsStreamAsync(TestAsync); + Byte[] data = new Byte[100]; + await stream.ReadAsync(data, 0, data.Length); + + // Intermediate test - haven't reached stream EOF yet. + trailingHeaders = GetTrailingHeaders(response); + Assert.True(trailingHeaders == null || trailingHeaders.Count() == 0); + + // Finish data stream and write out trailing headers. + await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes)); + await connection.SendResponseHeadersAsync(streamId, endStream: true, isTrailingHeader: true, headers: TrailingHeaders); + + // Read data until EOF is reached + while (stream.Read(data, 0, data.Length) != 0) ; + + trailingHeaders = GetTrailingHeaders(response); + Assert.Equal(TrailingHeaders.Count, trailingHeaders.Count()); + Assert.Contains("amazingtrailer", trailingHeaders.GetValues("MyCoolTrailerHeader")); + Assert.Contains("World", trailingHeaders.GetValues("Hello")); + + // Read when already zero. Trailers shouldn't be changed. + stream.Read(data, 0, data.Length); + + trailingHeaders = GetTrailingHeaders(response); + Assert.Equal(TrailingHeaders.Count, trailingHeaders.Count()); + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public async Task Http2GetAsync_TrailerHeaders_TrailingHeaderNoBody() + { + using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) + using (HttpClient client = CreateHttpClient()) + { + Task sendTask = client.GetAsync(server.Address); + + Http2LoopbackConnection connection = await server.EstablishConnectionAsync(); + + int streamId = await connection.ReadRequestHeaderAsync(); + + // Response header. + await connection.SendDefaultResponseHeadersAsync(streamId); + await connection.SendResponseHeadersAsync(streamId, endStream: true, isTrailingHeader: true, headers: TrailingHeaders); + + HttpResponseMessage response = await sendTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var trailingHeaders = GetTrailingHeaders(response); + Assert.Equal(TrailingHeaders.Count, trailingHeaders.Count()); + Assert.Contains("amazingtrailer", trailingHeaders.GetValues("MyCoolTrailerHeader")); + Assert.Contains("World", trailingHeaders.GetValues("Hello")); + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public async Task Http2GetAsync_TrailingHeaders_NoData_EmptyResponseObserved() + { + using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) + using (HttpClient client = CreateHttpClient()) + { + Task sendTask = client.GetAsync(server.Address); + + Http2LoopbackConnection connection = await server.EstablishConnectionAsync(); + + int streamId = await connection.ReadRequestHeaderAsync(); + + // Response header. + await connection.SendDefaultResponseHeadersAsync(streamId); + + // No data. + + // Response trailing headers + await connection.SendResponseHeadersAsync(streamId, isTrailingHeader: true, headers: TrailingHeaders); + + HttpResponseMessage response = await sendTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(Array.Empty(), await response.Content.ReadAsByteArrayAsync()); + + var trailingHeaders = GetTrailingHeaders(response); + Assert.Contains("amazingtrailer", trailingHeaders.GetValues("MyCoolTrailerHeader")); + Assert.Contains("World", trailingHeaders.GetValues("Hello")); + } + } + + private HttpHeaders GetTrailingHeaders(HttpResponseMessage responseMessage) + { +#if !NET48 + return responseMessage.TrailingHeaders; +#else +#pragma warning disable CS0618 // Type or member is obsolete + responseMessage.RequestMessage.Properties.TryGetValue("__ResponseTrailers", out object trailers); +#pragma warning restore CS0618 // Type or member is obsolete + return (HttpHeaders)trailers; +#endif + } + } +} diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpResponseStreamTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpResponseStreamTest.cs index cac00483a083de..90900a0517e07a 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpResponseStreamTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpResponseStreamTest.cs @@ -376,7 +376,7 @@ internal Stream MakeResponseStream() handle.Context = state.ToIntPtr(); state.RequestHandle = handle; - return new WinHttpResponseStream(handle, state); + return new WinHttpResponseStream(handle, state, new HttpResponseMessage()); } } } From 051d910b6e95d4515866fa25153161b74eee4337 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 22 Feb 2021 19:44:23 +0100 Subject: [PATCH 02/22] add "remote server" test --- .../FunctionalTests/TrailingHeadersTest.cs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs index 4414a713956e30..d9827df40c0685 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs @@ -144,6 +144,39 @@ public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Availab } } + [Fact] + public async Task Http2GetAsyncResponseHeadersReadOption_RemoteServer_TrailingHeaders_Available() + { + Uri address = new Uri("https://localhost:5001/trailers.ashx"); + using (HttpClient client = CreateHttpClient()) + { + Task sendTask = client.GetAsync(address, HttpCompletionOption.ResponseHeadersRead); + + HttpResponseMessage response = await sendTask; + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + Stream stream = await response.Content.ReadAsStreamAsync(TestAsync); + byte[] data = new byte[100]; + await stream.ReadAsync(data, 0, data.Length); + + // Read data until EOF is reached + while (stream.Read(data, 0, data.Length) != 0) ; + + var trailingHeaders = GetTrailingHeaders(response); + + // "EmptyHeader" is missing with remote server, aspnet probably ignores it. + Assert.Equal(3, trailingHeaders.Count()); + Assert.Contains("amazingtrailer", trailingHeaders.GetValues("MyCoolTrailerHeader")); + Assert.Contains("World", trailingHeaders.GetValues("Hello")); + + // Read when already zero. Trailers shouldn't be changed. + stream.Read(data, 0, data.Length); + + trailingHeaders = GetTrailingHeaders(response); + Assert.Equal(3, trailingHeaders.Count()); + } + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] public async Task Http2GetAsync_TrailerHeaders_TrailingHeaderNoBody() { From 6f4449b126a2634275e87ba58e0d14ea7d647280 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 22 Feb 2021 19:48:45 +0100 Subject: [PATCH 03/22] PlatformDetection.SupportsAlpn = false on .NET Framework --- .../Common/tests/TestUtilities/System/PlatformDetection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs index f56404ff270243..6f531c9490ec86 100644 --- a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs +++ b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs @@ -132,7 +132,7 @@ public static bool IsNonZeroLowerBoundArraySupported // OSX - SecureTransport doesn't expose alpn APIs. TODO https://github.com/dotnet/runtime/issues/27727 public static bool IsOpenSslSupported => IsLinux || IsFreeBSD || Isillumos || IsSolaris; - public static bool SupportsAlpn => (IsWindows && !IsWindows7) || + public static bool SupportsAlpn => (IsWindows && !IsWindows7 && !IsNetFramework) || (IsOpenSslSupported && (OpenSslVersion.Major >= 1 && (OpenSslVersion.Minor >= 1 || OpenSslVersion.Build >= 2))); From ba83606a8cbdf98a4389a891fc6815a9073e55c0 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 22 Feb 2021 20:08:47 +0100 Subject: [PATCH 04/22] make tests conditional --- .../FunctionalTests/TrailingHeadersTest.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs index d9827df40c0685..eee76ac6faac50 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs @@ -19,6 +19,13 @@ public class TrailingHeadersTest : HttpClientHandlerTestBase public TrailingHeadersTest(ITestOutputHelper output) : base(output) { } + // Build number suggested by the WinHttp team. + // It can be reduced after the backport of WINHTTP_QUERY_FLAG_TRAILERS is finished, + // and the patches are rolled out to CI machines. + public static bool OsSupportsWinHttpTrailingHeaders => Environment.OSVersion.Version >= new Version(10, 0, 19622, 0); + + public static bool TestsEnabled => OsSupportsWinHttpTrailingHeaders && PlatformDetection.SupportsAlpn; + protected override Version UseVersion => new Version(2, 0); protected static byte[] DataBytes = Encoding.ASCII.GetBytes("data"); @@ -32,7 +39,7 @@ public TrailingHeadersTest(ITestOutputHelper output) : base(output) protected static Frame MakeDataFrame(int streamId, byte[] data, bool endStream = false) => new DataFrame(data, (endStream ? FrameFlags.EndStream : FrameFlags.None), 0, streamId); - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + [ConditionalFact(nameof(TestsEnabled))] public async Task Http2GetAsync_NoTrailingHeaders_EmptyCollection() { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) @@ -60,7 +67,7 @@ public async Task Http2GetAsync_NoTrailingHeaders_EmptyCollection() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + [ConditionalFact(nameof(TestsEnabled))] public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) @@ -91,7 +98,7 @@ public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + [ConditionalFact(nameof(TestsEnabled))] public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Available() { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) @@ -144,7 +151,7 @@ public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Availab } } - [Fact] + [ConditionalFact(nameof(OsSupportsWinHttpTrailingHeaders))] public async Task Http2GetAsyncResponseHeadersReadOption_RemoteServer_TrailingHeaders_Available() { Uri address = new Uri("https://localhost:5001/trailers.ashx"); @@ -177,7 +184,7 @@ public async Task Http2GetAsyncResponseHeadersReadOption_RemoteServer_TrailingHe } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + [ConditionalFact(nameof(TestsEnabled))] public async Task Http2GetAsync_TrailerHeaders_TrailingHeaderNoBody() { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) @@ -203,7 +210,7 @@ public async Task Http2GetAsync_TrailerHeaders_TrailingHeaderNoBody() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + [ConditionalFact(nameof(TestsEnabled))] public async Task Http2GetAsync_TrailingHeaders_NoData_EmptyResponseObserved() { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) From 4ea3699205ad3954bc41e67e99b703b8fccafd32 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 23 Feb 2021 18:55:34 +0100 Subject: [PATCH 05/22] WinHttpTrailersHelper --- .../Windows/WinHttp/Interop.winhttp_types.cs | 1 + .../src/System.Net.Http.WinHttpHandler.csproj | 3 +- .../System/Net/Http/WinHttpTrailersHelper.cs | 43 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs diff --git a/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs b/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs index 1f8f39b577b459..c8f49628123c5d 100644 --- a/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs +++ b/src/libraries/Common/src/Interop/Windows/WinHttp/Interop.winhttp_types.cs @@ -165,6 +165,7 @@ internal partial class WinHttp public const uint WINHTTP_OPTION_WEB_SOCKET_SEND_BUFFER_SIZE = 123; public const uint WINHTTP_OPTION_TCP_KEEPALIVE = 152; + public const uint WINHTTP_OPTION_STREAM_ERROR_CODE = 159; public enum WINHTTP_WEB_SOCKET_BUFFER_TYPE { diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj index 0761a799f64100..bb8420ef2f22d9 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj @@ -1,4 +1,4 @@ - + true netstandard2.0-windows;netstandard2.0;netstandard2.1-windows;netstandard2.1;net461-windows @@ -91,6 +91,7 @@ + diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs new file mode 100644 index 00000000000000..09f83531c61e97 --- /dev/null +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -0,0 +1,43 @@ +using System.Diagnostics; +using System.Runtime.InteropServices; +using SafeWinHttpHandle = Interop.WinHttp.SafeWinHttpHandle; + +namespace System.Net.Http +{ + public static class WinHttpTrailersHelper + { + private static Lazy s_trailersSupported = new Lazy(GetTrailersSupported); + public static bool AreTrailersSupported => s_trailersSupported.Value; + + private static bool GetTrailersSupported() + { + SafeWinHttpHandle sessionHandle = null; + + try + { + sessionHandle = Interop.WinHttp.WinHttpOpen( + IntPtr.Zero, + Interop.WinHttp.WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, + Interop.WinHttp.WINHTTP_NO_PROXY_NAME, + Interop.WinHttp.WINHTTP_NO_PROXY_BYPASS, + (int)Interop.WinHttp.WINHTTP_FLAG_ASYNC); + + if (sessionHandle.IsInvalid) return false; + uint buffer = 0; + uint bufferSize = sizeof(uint); + if (Interop.WinHttp.WinHttpQueryOption(sessionHandle, Interop.WinHttp.WINHTTP_OPTION_STREAM_ERROR_CODE, ref buffer, ref bufferSize)) + { + Debug.Fail("Querying WINHTTP_OPTION_STREAM_ERROR_CODE on a session handle should never succeed."); + return false; + } + + int lastError = Marshal.GetLastWin32Error(); + return lastError != Interop.WinHttp.WINHTTP_INVALID_STATUS_CALLBACK; + } + finally + { + sessionHandle.Dispose(); + } + } + } +} From 46df0f1b85833cab4b9a80c3354fddc6f0bbde5f Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 23 Feb 2021 19:31:55 +0100 Subject: [PATCH 06/22] add OS support check --- .../src/System/Net/Http/WinHttpResponseStream.cs | 7 ++++--- .../src/System/Net/Http/WinHttpTrailersHelper.cs | 9 ++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs index e8b4b30d375194..3fcbafc018de11 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs @@ -268,9 +268,10 @@ private async Task ReadAsyncCore(byte[] buffer, int offset, int count, Canc private void ReadResponseTrailers() { // Only load response trailers if: - // 1. HTTP/2 or later - // 2. Response trailers not already loaded - if (_readTrailingHeaders || _responseMessage.Version < WinHttpHandler.HttpVersion20) + // 1. WINHTTP_QUERY_FLAG_TRAILERS is supported by the OS + // 2. HTTP/2 or later + // 3. Response trailers not already loaded + if (!WinHttpTrailersHelper.OsSupportsTrailers || _responseMessage.Version < WinHttpHandler.HttpVersion20 || _readTrailingHeaders) { return; } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index 09f83531c61e97..18820d168e2326 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -1,13 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + using System.Diagnostics; using System.Runtime.InteropServices; using SafeWinHttpHandle = Interop.WinHttp.SafeWinHttpHandle; namespace System.Net.Http { - public static class WinHttpTrailersHelper + internal static class WinHttpTrailersHelper { private static Lazy s_trailersSupported = new Lazy(GetTrailersSupported); - public static bool AreTrailersSupported => s_trailersSupported.Value; + public static bool OsSupportsTrailers => s_trailersSupported.Value; private static bool GetTrailersSupported() { @@ -32,7 +35,7 @@ private static bool GetTrailersSupported() } int lastError = Marshal.GetLastWin32Error(); - return lastError != Interop.WinHttp.WINHTTP_INVALID_STATUS_CALLBACK; + return lastError != Interop.WinHttp.ERROR_INVALID_PARAMETER; } finally { From cc4f74d378720d9845cf2f11d2eda796a21c64bc Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 23 Feb 2021 20:36:10 +0100 Subject: [PATCH 07/22] refactor step 1 --- .../src/System/Net/Http/WinHttpResponseStream.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs index 3fcbafc018de11..f60b54c1ca7b4a 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs @@ -288,7 +288,7 @@ private void ReadResponseTrailers() char[] trailersBuffer = ArrayPool.Shared.Rent(bufferLength); try { - WinHttpResponseParser.ParseResponseHeaders(_requestHandle, Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF | Interop.WinHttp.WINHTTP_QUERY_FLAG_TRAILERS, _responseMessage, trailersBuffer, stripEncodingHeaders: false, isTrailers: true); + WinHttpResponseParser.ParseResponseTrailers(_requestHandle, _responseMessage, trailersBuffer); } finally { From 8285ee6679f58b1a7e16b13e2871eb3d0f00dcf3 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 23 Feb 2021 20:41:58 +0100 Subject: [PATCH 08/22] refactor step 2 --- .../System/Net/Http/WinHttpResponseParser.cs | 86 ++++++++++--------- 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs index 5196fc868a9732..00d7a4ef49398a 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs @@ -93,7 +93,7 @@ public static HttpResponseMessage CreateResponseMessage( response.RequestMessage = request; // Parse raw response headers and place them into response message. - ParseResponseHeaders(requestHandle, Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF, response, buffer, stripEncodingHeaders, isTrailers: false); + ParseResponseHeaders(requestHandle, response, buffer, stripEncodingHeaders); if (response.RequestMessage.Method != HttpMethod.Head) { @@ -300,66 +300,74 @@ private class HttpResponseTrailers : HttpHeaders { } - public static void ParseResponseHeaders( + private static void ParseResponseHeaders( SafeWinHttpHandle requestHandle, - uint infoLevel, HttpResponseMessage response, char[] buffer, - bool stripEncodingHeaders, - bool isTrailers) + bool stripEncodingHeaders) { HttpResponseHeaders responseHeaders = response.Headers; HttpContentHeaders contentHeaders = response.Content.Headers; -#if NETSTANDARD2_1 - HttpResponseHeaders responseTrailers = response.TrailingHeaders; -#else - HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); - response.RequestMessage.Properties["__ResponseTrailers"] = responseTrailers; -#endif int bufferLength = GetResponseHeader( requestHandle, - infoLevel, + Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF, buffer); var reader = new WinHttpResponseHeaderReader(buffer, 0, bufferLength); - if (!isTrailers) - { - // Skip the first line which contains status code, etc. information that we already parsed. - reader.ReadLine(); - } + // Skip the first line which contains status code, etc. information that we already parsed. + reader.ReadLine(); // Parse the array of headers and split them between Content headers and Response headers. - string headerName; - string headerValue; - - while (reader.ReadHeader(out headerName, out headerValue)) + while (reader.ReadHeader(out string headerName, out string headerValue)) { - if (!isTrailers) + if (!responseHeaders.TryAddWithoutValidation(headerName, headerValue)) { - if (!responseHeaders.TryAddWithoutValidation(headerName, headerValue)) + if (stripEncodingHeaders) { - if (stripEncodingHeaders) + // Remove Content-Length and Content-Encoding headers if we are + // decompressing the response stream in the handler (due to + // WINHTTP not supporting it in a particular downlevel platform). + // This matches the behavior of WINHTTP when it does decompression itself. + if (string.Equals(HttpKnownHeaderNames.ContentLength, headerName, StringComparison.OrdinalIgnoreCase) || + string.Equals(HttpKnownHeaderNames.ContentEncoding, headerName, StringComparison.OrdinalIgnoreCase)) { - // Remove Content-Length and Content-Encoding headers if we are - // decompressing the response stream in the handler (due to - // WINHTTP not supporting it in a particular downlevel platform). - // This matches the behavior of WINHTTP when it does decompression itself. - if (string.Equals(HttpKnownHeaderNames.ContentLength, headerName, StringComparison.OrdinalIgnoreCase) || - string.Equals(HttpKnownHeaderNames.ContentEncoding, headerName, StringComparison.OrdinalIgnoreCase)) - { - continue; - } + continue; } - - contentHeaders.TryAddWithoutValidation(headerName, headerValue); } + + contentHeaders.TryAddWithoutValidation(headerName, headerValue); } - else - { - responseTrailers.TryAddWithoutValidation(headerName, headerValue); - } + } + } + + public static void ParseResponseTrailers( + SafeWinHttpHandle requestHandle, + HttpResponseMessage response, + char[] buffer) + { + HttpResponseHeaders responseHeaders = response.Headers; + HttpContentHeaders contentHeaders = response.Content.Headers; + +#if NETSTANDARD2_1 + HttpResponseHeaders responseTrailers = response.TrailingHeaders; +#else + HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); + response.RequestMessage.Properties["__ResponseTrailers"] = responseTrailers; +#endif + + int bufferLength = GetResponseHeader( + requestHandle, + Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF | Interop.WinHttp.WINHTTP_QUERY_FLAG_TRAILERS, + buffer); + + var reader = new WinHttpResponseHeaderReader(buffer, 0, bufferLength); + + // Parse the array of headers and split them between Content headers and Response headers. + while (reader.ReadHeader(out string headerName, out string headerValue)) + { + responseTrailers.TryAddWithoutValidation(headerName, headerValue); } } From c5375e743af0d37c34c29f9c3c1f6da8cf6b9206 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 23 Feb 2021 21:00:55 +0100 Subject: [PATCH 09/22] refactor step 3 --- .../System/Net/Http/WinHttpResponseParser.cs | 14 +------------ .../System/Net/Http/WinHttpTrailersHelper.cs | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs index 00d7a4ef49398a..2fc6acfdde3f5d 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs @@ -296,10 +296,6 @@ private static string GetReasonPhrase(HttpStatusCode statusCode, char[] buffer, new string(buffer, 0, bufferLength); } - private class HttpResponseTrailers : HttpHeaders - { - } - private static void ParseResponseHeaders( SafeWinHttpHandle requestHandle, HttpResponseMessage response, @@ -347,15 +343,7 @@ public static void ParseResponseTrailers( HttpResponseMessage response, char[] buffer) { - HttpResponseHeaders responseHeaders = response.Headers; - HttpContentHeaders contentHeaders = response.Content.Headers; - -#if NETSTANDARD2_1 - HttpResponseHeaders responseTrailers = response.TrailingHeaders; -#else - HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); - response.RequestMessage.Properties["__ResponseTrailers"] = responseTrailers; -#endif + HttpHeaders responseTrailers = WinHttpTrailersHelper.GetResponseTrailers(response); int bufferLength = GetResponseHeader( requestHandle, diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index 18820d168e2326..bbfabe81812f17 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Net.Http.Headers; using System.Runtime.InteropServices; using SafeWinHttpHandle = Interop.WinHttp.SafeWinHttpHandle; @@ -12,6 +13,20 @@ internal static class WinHttpTrailersHelper private static Lazy s_trailersSupported = new Lazy(GetTrailersSupported); public static bool OsSupportsTrailers => s_trailersSupported.Value; + public static HttpHeaders GetResponseTrailers(HttpResponseMessage response) + { +#if NETSTANDARD2_1 + // We are (ab)using a property that became Obsolete on .NET 5 +#pragma warning disable CS0618 + return response.TrailingHeaders; +#pragma warning restore CS0618 +#else + HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); + response.RequestMessage.Properties["__ResponseTrailers"] = responseTrailers; + return responseTrailers; +#endif + } + private static bool GetTrailersSupported() { SafeWinHttpHandle sessionHandle = null; @@ -42,5 +57,11 @@ private static bool GetTrailersSupported() sessionHandle.Dispose(); } } + +#if !NETSTANDARD2_1 + private class HttpResponseTrailers : HttpHeaders + { + } +#endif } } From 6594f31bb344d4a9596ffe8f8ef13b64030fa860 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 24 Feb 2021 15:41:34 +0100 Subject: [PATCH 10/22] Add comments --- .../src/System/Net/Http/WinHttpTrailersHelper.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index bbfabe81812f17..c9fef88cc42d1c 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -10,6 +10,8 @@ namespace System.Net.Http { internal static class WinHttpTrailersHelper { + private const string RequestMessagePropertyName = "__ResponseTrailers"; + private static Lazy s_trailersSupported = new Lazy(GetTrailersSupported); public static bool OsSupportsTrailers => s_trailersSupported.Value; @@ -22,11 +24,14 @@ public static HttpHeaders GetResponseTrailers(HttpResponseMessage response) #pragma warning restore CS0618 #else HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); - response.RequestMessage.Properties["__ResponseTrailers"] = responseTrailers; + response.RequestMessage.Properties[RequestMessagePropertyName] = responseTrailers; return responseTrailers; #endif } + // There is no way to verify if WINHTTP_QUERY_FLAG_TRAILERS is supported by the OS without creating a request. + // Instead, the WinHTTP team recommended to check if WINHTTP_OPTION_STREAM_ERROR_CODE is recognized by the OS. + // Both features were introduced in Manganese and are planned to be backported to older Windows versions together. private static bool GetTrailersSupported() { SafeWinHttpHandle sessionHandle = null; @@ -50,6 +55,9 @@ private static bool GetTrailersSupported() } int lastError = Marshal.GetLastWin32Error(); + + // New Windows builds are expected to fail with ERROR_WINHTTP_INCORRECT_HANDLE_TYPE, + // when querying WINHTTP_OPTION_STREAM_ERROR_CODE on a session handle. return lastError != Interop.WinHttp.ERROR_INVALID_PARAMETER; } finally From 1370e6de70cd0caea810100af2f74f16e22b610f Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 24 Feb 2021 15:42:24 +0100 Subject: [PATCH 11/22] simpify GetResponseHeaderCharBufferLength --- .../src/System/Net/Http/WinHttpResponseParser.cs | 10 ++++++++-- .../src/System/Net/Http/WinHttpResponseStream.cs | 5 +---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs index 2fc6acfdde3f5d..dca40533ef1252 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs @@ -29,7 +29,7 @@ public static HttpResponseMessage CreateResponseMessage( // Create a single buffer to use for all subsequent WinHttpQueryHeaders string interop calls. // This buffer is the length needed for WINHTTP_QUERY_RAW_HEADERS_CRLF, which includes the status line // and all headers separated by CRLF, so it should be large enough for any individual status line or header queries. - int bufferLength = GetResponseHeaderCharBufferLength(requestHandle, Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF, isTrailingHeaders: false); + int bufferLength = GetResponseHeaderCharBufferLength(requestHandle, isTrailingHeaders: false); char[] buffer = ArrayPool.Shared.Rent(bufferLength); try { @@ -223,12 +223,18 @@ private static unsafe int GetResponseHeader(SafeWinHttpHandle requestHandle, uin /// /// Returns the size of the char array buffer. /// - public static unsafe int GetResponseHeaderCharBufferLength(SafeWinHttpHandle requestHandle, uint infoLevel, bool isTrailingHeaders) + public static unsafe int GetResponseHeaderCharBufferLength(SafeWinHttpHandle requestHandle, bool isTrailingHeaders) { char* buffer = null; int bufferLength = 0; uint index = 0; + uint infoLevel = Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF; + if (isTrailingHeaders) + { + infoLevel |= Interop.WinHttp.WINHTTP_QUERY_FLAG_TRAILERS; + } + if (!QueryHeaders(requestHandle, infoLevel, buffer, ref bufferLength, ref index)) { int lastError = Marshal.GetLastWin32Error(); diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs index f60b54c1ca7b4a..545ed679074da0 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs @@ -278,10 +278,7 @@ private void ReadResponseTrailers() _readTrailingHeaders = true; - var bufferLength = WinHttpResponseParser.GetResponseHeaderCharBufferLength( - _requestHandle, - Interop.WinHttp.WINHTTP_QUERY_RAW_HEADERS_CRLF | Interop.WinHttp.WINHTTP_QUERY_FLAG_TRAILERS, - isTrailingHeaders: true); + var bufferLength = WinHttpResponseParser.GetResponseHeaderCharBufferLength(_requestHandle, isTrailingHeaders: true); if (bufferLength != 0) { From 3cceba86f8831ce924e93e1dfae19e146150bae4 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 24 Feb 2021 15:44:32 +0100 Subject: [PATCH 12/22] fix build --- .../System/Net/Http/WinHttpTrailersHelper.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index c9fef88cc42d1c..0134511f2c2b67 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -10,22 +10,26 @@ namespace System.Net.Http { internal static class WinHttpTrailersHelper { +#if !NETSTANDARD2_1 private const string RequestMessagePropertyName = "__ResponseTrailers"; - + private class HttpResponseTrailers : HttpHeaders + { + } +#endif private static Lazy s_trailersSupported = new Lazy(GetTrailersSupported); public static bool OsSupportsTrailers => s_trailersSupported.Value; public static HttpHeaders GetResponseTrailers(HttpResponseMessage response) { #if NETSTANDARD2_1 - // We are (ab)using a property that became Obsolete on .NET 5 -#pragma warning disable CS0618 return response.TrailingHeaders; -#pragma warning restore CS0618 #else + // We are (ab)using a property that became Obsolete on .NET 5 +#pragma warning disable CS0618 HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); response.RequestMessage.Properties[RequestMessagePropertyName] = responseTrailers; return responseTrailers; +#pragma warning restore CS0618 #endif } @@ -65,11 +69,5 @@ private static bool GetTrailersSupported() sessionHandle.Dispose(); } } - -#if !NETSTANDARD2_1 - private class HttpResponseTrailers : HttpHeaders - { - } -#endif } } From e41eacb7d470262f8514fc93dd8d5936895f1c64 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 24 Feb 2021 15:51:21 +0100 Subject: [PATCH 13/22] Remove Http2GetAsyncResponseHeadersReadOption_RemoteServer_TrailingHeaders_Available --- .../FunctionalTests/TrailingHeadersTest.cs | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs index eee76ac6faac50..cef52465524dcf 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs @@ -151,39 +151,6 @@ public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Availab } } - [ConditionalFact(nameof(OsSupportsWinHttpTrailingHeaders))] - public async Task Http2GetAsyncResponseHeadersReadOption_RemoteServer_TrailingHeaders_Available() - { - Uri address = new Uri("https://localhost:5001/trailers.ashx"); - using (HttpClient client = CreateHttpClient()) - { - Task sendTask = client.GetAsync(address, HttpCompletionOption.ResponseHeadersRead); - - HttpResponseMessage response = await sendTask; - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - - Stream stream = await response.Content.ReadAsStreamAsync(TestAsync); - byte[] data = new byte[100]; - await stream.ReadAsync(data, 0, data.Length); - - // Read data until EOF is reached - while (stream.Read(data, 0, data.Length) != 0) ; - - var trailingHeaders = GetTrailingHeaders(response); - - // "EmptyHeader" is missing with remote server, aspnet probably ignores it. - Assert.Equal(3, trailingHeaders.Count()); - Assert.Contains("amazingtrailer", trailingHeaders.GetValues("MyCoolTrailerHeader")); - Assert.Contains("World", trailingHeaders.GetValues("Hello")); - - // Read when already zero. Trailers shouldn't be changed. - stream.Read(data, 0, data.Length); - - trailingHeaders = GetTrailingHeaders(response); - Assert.Equal(3, trailingHeaders.Count()); - } - } - [ConditionalFact(nameof(TestsEnabled))] public async Task Http2GetAsync_TrailerHeaders_TrailingHeaderNoBody() { From d48ae0aab3c049b81484e1d58baf83312f280385 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 24 Feb 2021 17:12:51 +0100 Subject: [PATCH 14/22] fix WinHttpHandler.Unit.Tests --- .../src/System/Net/Http/WinHttpTrailersHelper.cs | 7 ++----- .../System.Net.Http.WinHttpHandler.Unit.Tests.csproj | 5 ++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index 0134511f2c2b67..4908a5f7970d23 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -10,7 +10,7 @@ namespace System.Net.Http { internal static class WinHttpTrailersHelper { -#if !NETSTANDARD2_1 +#if !NETSTANDARD2_1 && !UNITTEST // The latter is required for the WinHttpHandler.Unit.Tests, which includes the source file. private const string RequestMessagePropertyName = "__ResponseTrailers"; private class HttpResponseTrailers : HttpHeaders { @@ -21,15 +21,12 @@ private class HttpResponseTrailers : HttpHeaders public static HttpHeaders GetResponseTrailers(HttpResponseMessage response) { -#if NETSTANDARD2_1 +#if NETSTANDARD2_1 || UNITTEST return response.TrailingHeaders; #else - // We are (ab)using a property that became Obsolete on .NET 5 -#pragma warning disable CS0618 HttpResponseTrailers responseTrailers = new HttpResponseTrailers(); response.RequestMessage.Properties[RequestMessagePropertyName] = responseTrailers; return responseTrailers; -#pragma warning restore CS0618 #endif } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj index 32e7c9c9e44aa6..52b02b402fb79f 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj @@ -1,9 +1,10 @@ - + $(NoWarn);0436 true ../../src/Resources/Strings.resx $(NetCoreAppCurrent)-windows + UNITTEST annotations @@ -84,6 +85,8 @@ Link="ProductionCode\WinHttpResponseStream.cs" /> + Date: Thu, 25 Feb 2021 14:59:05 +0100 Subject: [PATCH 15/22] fix unit tests --- .../tests/UnitTests/FakeInterop.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeInterop.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeInterop.cs index 596ce8212979b0..e26f1976418d6a 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeInterop.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeInterop.cs @@ -422,6 +422,12 @@ public static bool WinHttpQueryOption( ref uint buffer, ref uint bufferSize) { + if (option == WINHTTP_OPTION_STREAM_ERROR_CODE) + { + TestControl.LastWin32Error = (int)ERROR_INVALID_PARAMETER; + return false; + } + return true; } From 575cbd82f7a6023b6b4900628ceff749a4384f53 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 25 Feb 2021 14:59:56 +0100 Subject: [PATCH 16/22] add comments on RequestMessagePropertyName Co-authored-by: James Newton-King --- .../src/System/Net/Http/WinHttpTrailersHelper.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index 4908a5f7970d23..b175b14740f9a0 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -11,6 +11,8 @@ namespace System.Net.Http internal static class WinHttpTrailersHelper { #if !NETSTANDARD2_1 && !UNITTEST // The latter is required for the WinHttpHandler.Unit.Tests, which includes the source file. + // Trailer property name was chosen to be descriptive and be unlikely to collide with a user set property. + // Apps and libraries will use this key so it shouldn't change. private const string RequestMessagePropertyName = "__ResponseTrailers"; private class HttpResponseTrailers : HttpHeaders { From 098356a6946652b5bf40fc84b0a0f110ce921f9f Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 25 Feb 2021 15:06:36 +0100 Subject: [PATCH 17/22] simplify GetTrailersSupported --- .../System/Net/Http/WinHttpTrailersHelper.cs | 35 +++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index b175b14740f9a0..86fb9d9bb417eb 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -37,36 +37,27 @@ public static HttpHeaders GetResponseTrailers(HttpResponseMessage response) // Both features were introduced in Manganese and are planned to be backported to older Windows versions together. private static bool GetTrailersSupported() { - SafeWinHttpHandle sessionHandle = null; - - try - { - sessionHandle = Interop.WinHttp.WinHttpOpen( + using SafeWinHttpHandle sessionHandle = Interop.WinHttp.WinHttpOpen( IntPtr.Zero, Interop.WinHttp.WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, Interop.WinHttp.WINHTTP_NO_PROXY_NAME, Interop.WinHttp.WINHTTP_NO_PROXY_BYPASS, (int)Interop.WinHttp.WINHTTP_FLAG_ASYNC); - if (sessionHandle.IsInvalid) return false; - uint buffer = 0; - uint bufferSize = sizeof(uint); - if (Interop.WinHttp.WinHttpQueryOption(sessionHandle, Interop.WinHttp.WINHTTP_OPTION_STREAM_ERROR_CODE, ref buffer, ref bufferSize)) - { - Debug.Fail("Querying WINHTTP_OPTION_STREAM_ERROR_CODE on a session handle should never succeed."); - return false; - } - - int lastError = Marshal.GetLastWin32Error(); - - // New Windows builds are expected to fail with ERROR_WINHTTP_INCORRECT_HANDLE_TYPE, - // when querying WINHTTP_OPTION_STREAM_ERROR_CODE on a session handle. - return lastError != Interop.WinHttp.ERROR_INVALID_PARAMETER; - } - finally + if (sessionHandle.IsInvalid) return false; + uint buffer = 0; + uint bufferSize = sizeof(uint); + if (Interop.WinHttp.WinHttpQueryOption(sessionHandle, Interop.WinHttp.WINHTTP_OPTION_STREAM_ERROR_CODE, ref buffer, ref bufferSize)) { - sessionHandle.Dispose(); + Debug.Fail("Querying WINHTTP_OPTION_STREAM_ERROR_CODE on a session handle should never succeed."); + return false; } + + int lastError = Marshal.GetLastWin32Error(); + + // New Windows builds are expected to fail with ERROR_WINHTTP_INCORRECT_HANDLE_TYPE, + // when querying WINHTTP_OPTION_STREAM_ERROR_CODE on a session handle. + return lastError != Interop.WinHttp.ERROR_INVALID_PARAMETER; } } } From 01109e8bf074c55d8de6ba8cf9712e0a2afda006 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 25 Feb 2021 15:09:25 +0100 Subject: [PATCH 18/22] improve comments --- .../src/System/Net/Http/WinHttpResponseStream.cs | 2 +- .../src/System/Net/Http/WinHttpTrailersHelper.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs index 545ed679074da0..2922291faa01f8 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs @@ -269,7 +269,7 @@ private void ReadResponseTrailers() { // Only load response trailers if: // 1. WINHTTP_QUERY_FLAG_TRAILERS is supported by the OS - // 2. HTTP/2 or later + // 2. HTTP/2 or later (WINHTTP_QUERY_FLAG_TRAILERS does not work with HTTP/1.1) // 3. Response trailers not already loaded if (!WinHttpTrailersHelper.OsSupportsTrailers || _responseMessage.Version < WinHttpHandler.HttpVersion20 || _readTrailingHeaders) { diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs index 86fb9d9bb417eb..7be5a4c5a240d0 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTrailersHelper.cs @@ -10,7 +10,8 @@ namespace System.Net.Http { internal static class WinHttpTrailersHelper { -#if !NETSTANDARD2_1 && !UNITTEST // The latter is required for the WinHttpHandler.Unit.Tests, which includes the source file. + // UNITTEST is true when building against WinHttpHandler.Unit.Tests, which includes the source file. +#if !NETSTANDARD2_1 && !UNITTEST // Trailer property name was chosen to be descriptive and be unlikely to collide with a user set property. // Apps and libraries will use this key so it shouldn't change. private const string RequestMessagePropertyName = "__ResponseTrailers"; From cf13ba095c0b3c42b5ca09ec0a6dd5bf498563cc Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 25 Feb 2021 15:22:29 +0100 Subject: [PATCH 19/22] make code shorter in GetResponseHeaderCharBufferLength --- .../System/Net/Http/WinHttpResponseParser.cs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs index dca40533ef1252..9f324ada27a0f8 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs @@ -239,21 +239,12 @@ public static unsafe int GetResponseHeaderCharBufferLength(SafeWinHttpHandle req { int lastError = Marshal.GetLastWin32Error(); - if (!isTrailingHeaders) - { - Debug.Assert(lastError != Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND); + Debug.Assert(isTrailingHeaders || lastError != Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND); - if (lastError != Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER) - { - throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); - } - } - else + if (lastError != Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER && + (!isTrailingHeaders || lastError != Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND)) { - if (!(lastError == Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER || lastError == Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND)) - { - throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); - } + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); } } From 29969b46457ceff0e8df34990fecd854b8698ffe Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Fri, 26 Feb 2021 15:03:16 +0100 Subject: [PATCH 20/22] HttpClientHandlerTestBase.AllowAllCertificates should not be static --- .../tests/System/Net/Http/HttpClientHandlerTestBase.cs | 2 +- .../HttpClientHandlerTestBase.WinHttpHandler.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs index 26a084b969f200..8e6425c7c1dde0 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs @@ -40,7 +40,7 @@ protected HttpClient CreateHttpClient(HttpMessageHandler handler) => #endif }; - protected static HttpClient CreateHttpClient(string useVersionString) => + protected HttpClient CreateHttpClient(string useVersionString) => CreateHttpClient(CreateHttpClientHandler(useVersionString), useVersionString); protected static HttpClient CreateHttpClient(HttpMessageHandler handler, string useVersionString) => diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs index 5db27528550446..02e4d349374f87 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs @@ -10,9 +10,9 @@ public abstract partial class HttpClientHandlerTestBase : FileCleanupTestBase { protected static bool IsWinHttpHandler => true; - protected static bool AllowAllCertificates { get; set; } = true; + protected bool AllowAllCertificates { get; set; } = true; - protected static WinHttpClientHandler CreateHttpClientHandler(Version useVersion = null) + protected WinHttpClientHandler CreateHttpClientHandler(Version useVersion = null) { useVersion ??= HttpVersion.Version11; @@ -28,7 +28,7 @@ protected static WinHttpClientHandler CreateHttpClientHandler(Version useVersion protected WinHttpClientHandler CreateHttpClientHandler() => CreateHttpClientHandler(UseVersion); - protected static WinHttpClientHandler CreateHttpClientHandler(string useVersionString) => + protected WinHttpClientHandler CreateHttpClientHandler(string useVersionString) => CreateHttpClientHandler(Version.Parse(useVersionString)); protected static HttpRequestMessage CreateRequest(HttpMethod method, Uri uri, Version version, bool exactVersion = false) => From 0332b0e3d8a904218b0fdda135b15980ad738434 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Fri, 26 Feb 2021 17:25:55 +0100 Subject: [PATCH 21/22] revert previous attempt --- .../tests/System/Net/Http/HttpClientHandlerTestBase.cs | 2 +- .../HttpClientHandlerTestBase.WinHttpHandler.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs index 8e6425c7c1dde0..26a084b969f200 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs @@ -40,7 +40,7 @@ protected HttpClient CreateHttpClient(HttpMessageHandler handler) => #endif }; - protected HttpClient CreateHttpClient(string useVersionString) => + protected static HttpClient CreateHttpClient(string useVersionString) => CreateHttpClient(CreateHttpClientHandler(useVersionString), useVersionString); protected static HttpClient CreateHttpClient(HttpMessageHandler handler, string useVersionString) => diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs index 02e4d349374f87..5db27528550446 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs @@ -10,9 +10,9 @@ public abstract partial class HttpClientHandlerTestBase : FileCleanupTestBase { protected static bool IsWinHttpHandler => true; - protected bool AllowAllCertificates { get; set; } = true; + protected static bool AllowAllCertificates { get; set; } = true; - protected WinHttpClientHandler CreateHttpClientHandler(Version useVersion = null) + protected static WinHttpClientHandler CreateHttpClientHandler(Version useVersion = null) { useVersion ??= HttpVersion.Version11; @@ -28,7 +28,7 @@ protected WinHttpClientHandler CreateHttpClientHandler(Version useVersion = null protected WinHttpClientHandler CreateHttpClientHandler() => CreateHttpClientHandler(UseVersion); - protected WinHttpClientHandler CreateHttpClientHandler(string useVersionString) => + protected static WinHttpClientHandler CreateHttpClientHandler(string useVersionString) => CreateHttpClientHandler(Version.Parse(useVersionString)); protected static HttpRequestMessage CreateRequest(HttpMethod method, Uri uri, Version version, bool exactVersion = false) => From 05b85ab1e5236c2344893df647404adf333b30c8 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 1 Mar 2021 12:25:42 +0100 Subject: [PATCH 22/22] AllowAllCertificates: use parameter instead of static property --- .../HttpClientHandlerTest.ServerCertificates.cs | 14 +++++++++++--- .../HttpClientHandlerTestBase.WinHttpHandler.cs | 6 ++---- .../tests/FunctionalTests/PlatformHandlerTest.cs | 2 +- ...HttpClientHandlerTestBase.SocketsHttpHandler.cs | 4 ++-- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ServerCertificates.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ServerCertificates.cs index 973a10967a452e..64e0017d18fdd7 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ServerCertificates.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ServerCertificates.cs @@ -30,6 +30,10 @@ public abstract partial class HttpClientHandler_ServerCertificates_Test : HttpCl public HttpClientHandler_ServerCertificates_Test(ITestOutputHelper output) : base(output) { } + // This enables customizing ServerCertificateCustomValidationCallback in WinHttpHandler variants: + protected bool AllowAllHttp2Certificates { get; set; } = true; + protected new HttpClientHandler CreateHttpClientHandler() => CreateHttpClientHandler(UseVersion, allowAllHttp2Certificates: AllowAllHttp2Certificates); + [Fact] public void Ctor_ExpectedDefaultValues() { @@ -451,15 +455,19 @@ public void HttpClientUsesSslCertEnvironmentVariables() File.WriteAllText(sslCertFile, ""); psi.Environment.Add("SSL_CERT_FILE", sslCertFile); - RemoteExecutor.Invoke(async (useVersionString) => + RemoteExecutor.Invoke(async (useVersionString, allowAllHttp2CertificatesString) => { const string Url = "https://www.microsoft.com"; - using (HttpClient client = CreateHttpClient(useVersionString)) + HttpClientHandler handler = CreateHttpClientHandler( + Version.Parse(useVersionString), + allowAllHttp2Certificates: bool.Parse(allowAllHttp2CertificatesString)); + + using (HttpClient client = CreateHttpClient(handler, useVersionString)) { await Assert.ThrowsAsync(() => client.GetAsync(Url)); } - }, UseVersion.ToString(), new RemoteInvokeOptions { StartInfo = psi }).Dispose(); + }, UseVersion.ToString(), AllowAllHttp2Certificates.ToString(), new RemoteInvokeOptions { StartInfo = psi }).Dispose(); } } } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs index 5db27528550446..9c8289ec789348 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs @@ -10,15 +10,13 @@ public abstract partial class HttpClientHandlerTestBase : FileCleanupTestBase { protected static bool IsWinHttpHandler => true; - protected static bool AllowAllCertificates { get; set; } = true; - - protected static WinHttpClientHandler CreateHttpClientHandler(Version useVersion = null) + protected static WinHttpClientHandler CreateHttpClientHandler(Version useVersion = null, bool allowAllHttp2Certificates = true) { useVersion ??= HttpVersion.Version11; WinHttpClientHandler handler = new WinHttpClientHandler(useVersion); - if (useVersion >= HttpVersion20.Value && AllowAllCertificates) + if (useVersion >= HttpVersion20.Value && allowAllHttp2Certificates) { handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/PlatformHandlerTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/PlatformHandlerTest.cs index 058bfaee00fc45..6a53fbc9c0d42f 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/PlatformHandlerTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/PlatformHandlerTest.cs @@ -277,7 +277,7 @@ public sealed class PlatformHandler_HttpClientHandler_ServerCertificates_Http2_T protected override Version UseVersion => HttpVersion20.Value; public PlatformHandler_HttpClientHandler_ServerCertificates_Http2_Test(ITestOutputHelper output) : base(output) { - AllowAllCertificates = false; + AllowAllHttp2Certificates = false; } } diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs index d7c377cde7ad16..3ea69bb51bb033 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs @@ -18,13 +18,13 @@ public abstract partial class HttpClientHandlerTestBase : FileCleanupTestBase public static bool IsMsQuicSupported => QuicImplementationProviders.MsQuic.IsSupported; - protected static HttpClientHandler CreateHttpClientHandler(Version useVersion = null, QuicImplementationProvider quicImplementationProvider = null) + protected static HttpClientHandler CreateHttpClientHandler(Version useVersion = null, QuicImplementationProvider quicImplementationProvider = null, bool allowAllHttp2Certificates = true) { useVersion ??= HttpVersion.Version11; HttpClientHandler handler = (PlatformDetection.SupportsAlpn && useVersion != HttpVersion.Version30) ? new HttpClientHandler() : new VersionHttpClientHandler(useVersion); - if (useVersion >= HttpVersion.Version20) + if (useVersion >= HttpVersion.Version20 && allowAllHttp2Certificates) { handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; }