From 3d196b32cd009c7bbeee6dfbce0c8ab15f036434 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 15 Dec 2021 12:20:24 -0800 Subject: [PATCH 1/3] tests --- .../FunctionalTests/TrailingHeadersTest.cs | 30 +++++++++++++++---- .../FunctionalTests/SocketsHttpHandlerTest.cs | 30 +++++++++++++++---- 2 files changed, 48 insertions(+), 12 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 cef52465524dcf..5ac753c266ee0a 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/TrailingHeadersTest.cs @@ -67,8 +67,10 @@ public async Task Http2GetAsync_NoTrailingHeaders_EmptyCollection() } } - [ConditionalFact(nameof(TestsEnabled))] - public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() + [InlineData(false)] + [InlineData(true)] + [ConditionalTheory(nameof(TestsEnabled))] + public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted(bool responseHasContentLength) { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) using (HttpClient client = CreateHttpClient()) @@ -80,7 +82,14 @@ public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() int streamId = await connection.ReadRequestHeaderAsync(); // Response header. - await connection.SendDefaultResponseHeadersAsync(streamId); + if (responseHasContentLength) + { + await connection.SendResponseHeadersAsync(streamId, endStream: false, headers: new[] { new HttpHeaderData("Content-Length", DataBytes.Length.ToString()) }); + } + else + { + await connection.SendDefaultResponseHeadersAsync(streamId); + } // Response data, missing Trailers. await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes)); @@ -98,8 +107,10 @@ public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() } } - [ConditionalFact(nameof(TestsEnabled))] - public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Available() + [InlineData(false)] + [InlineData(true)] + [ConditionalTheory(nameof(TestsEnabled))] + public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Available(bool responseHasContentLength) { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) using (HttpClient client = CreateHttpClient()) @@ -111,7 +122,14 @@ public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Availab int streamId = await connection.ReadRequestHeaderAsync(); // Response header. - await connection.SendDefaultResponseHeadersAsync(streamId); + if (responseHasContentLength) + { + await connection.SendResponseHeadersAsync(streamId, endStream: false, headers: new[] { new HttpHeaderData("Content-Length", DataBytes.Length.ToString()) }); + } + else + { + await connection.SendDefaultResponseHeadersAsync(streamId); + } // Response data, missing Trailers. await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes)); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index 14f444b1935a60..bfd1e83bfad502 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -837,8 +837,10 @@ public async Task Http2GetAsync_NoTrailingHeaders_EmptyCollection() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] - public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() + [InlineData(false)] + [InlineData(true)] + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted(bool responseHasContentLength) { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) using (HttpClient client = CreateHttpClient()) @@ -850,7 +852,14 @@ public async Task Http2GetAsync_MissingTrailer_TrailingHeadersAccepted() int streamId = await connection.ReadRequestHeaderAsync(); // Response header. - await connection.SendDefaultResponseHeadersAsync(streamId); + if (responseHasContentLength) + { + await connection.SendResponseHeadersAsync(streamId, endStream: false, headers: new[] { new HttpHeaderData("Content-Length", DataBytes.Length.ToString()) }); + } + else + { + await connection.SendDefaultResponseHeadersAsync(streamId); + } // Response data, missing Trailers. await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes)); @@ -888,8 +897,10 @@ public async Task Http2GetAsync_TrailerHeaders_TrailingPseudoHeadersThrow() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] - public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Available() + [InlineData(false)] + [InlineData(true)] + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Available(bool responseHasContentLength) { using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer()) using (HttpClient client = CreateHttpClient()) @@ -901,7 +912,14 @@ public async Task Http2GetAsyncResponseHeadersReadOption_TrailingHeaders_Availab int streamId = await connection.ReadRequestHeaderAsync(); // Response header. - await connection.SendDefaultResponseHeadersAsync(streamId); + if (responseHasContentLength) + { + await connection.SendResponseHeadersAsync(streamId, endStream: false, headers: new[] { new HttpHeaderData("Content-Length", DataBytes.Length.ToString()) }); + } + else + { + await connection.SendDefaultResponseHeadersAsync(streamId); + } // Response data, missing Trailers. await connection.WriteFrameAsync(MakeDataFrame(streamId, DataBytes)); From eae9fbce14d5ea07e322ab0b07163aabecf979f4 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 15 Dec 2021 14:38:56 -0800 Subject: [PATCH 2/3] enable WINHTTP_OPTION_REQUIRE_STREAM_END on request handles --- .../Windows/WinHttp/Interop.winhttp_types.cs | 1 + .../src/System/Net/Http/WinHttpHandler.cs | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) 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 3344e34b0265e3..463de2572b2b3f 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 @@ -168,6 +168,7 @@ internal static partial class WinHttp public const uint WINHTTP_OPTION_TCP_KEEPALIVE = 152; public const uint WINHTTP_OPTION_STREAM_ERROR_CODE = 159; + public const uint WINHTTP_OPTION_REQUIRE_STREAM_END = 160; public enum WINHTTP_WEB_SOCKET_BUFFER_TYPE { diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs index 3b4c247b8b7032..684fde062774d4 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs @@ -1534,6 +1534,22 @@ private void SetRequestHandleHttp2Options(SafeWinHttpHandle requestHandle, Versi { if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "HTTP/2 option not supported"); } + + if (requestVersion == HttpVersion20 && WinHttpTrailersHelper.OsSupportsTrailers) + { + // Setting WINHTTP_OPTION_REQUIRE_STREAM_END to TRUE is needed for WinHttp to read trailing headers + // in case the response has Content-Lenght defined. + // According to the WinHttp team, the feature-detection logic in WinHttpTrailersHelper.OsSupportsTrailers + // should also indicate the support of WINHTTP_OPTION_REQUIRE_STREAM_END. + optionData = 1; + if (!Interop.WinHttp.WinHttpSetOption(requestHandle, Interop.WinHttp.WINHTTP_OPTION_REQUIRE_STREAM_END, ref optionData)) + { + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Info(this, "Failed to enable WINHTTP_OPTION_REQUIRE_STREAM_END error code: " + Marshal.GetLastWin32Error()); + } + } + } } private void SetWinHttpOption(SafeWinHttpHandle handle, uint option, ref uint optionData) From e6f111c9aa7acb067e0e1da29a46d3a019e3ee58 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 17 Dec 2021 12:07:07 -0800 Subject: [PATCH 3/3] set WINHTTP_OPTION_REQUIRE_STREAM_END on the session handle --- .../src/System/Net/Http/WinHttpHandler.cs | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs index 684fde062774d4..07133b2667c229 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs @@ -1120,6 +1120,7 @@ private void SetSessionHandleOptions(SafeWinHttpHandle sessionHandle) SetSessionHandleTimeoutOptions(sessionHandle); SetDisableHttp2StreamQueue(sessionHandle); SetTcpKeepalive(sessionHandle); + SetRequireStreamEnd(sessionHandle); } private unsafe void SetTcpKeepalive(SafeWinHttpHandle sessionHandle) @@ -1145,6 +1146,27 @@ private unsafe void SetTcpKeepalive(SafeWinHttpHandle sessionHandle) } } + private void SetRequireStreamEnd(SafeWinHttpHandle sessionHandle) + { + if (WinHttpTrailersHelper.OsSupportsTrailers) + { + // Setting WINHTTP_OPTION_REQUIRE_STREAM_END to TRUE is needed for WinHttp to read trailing headers + // in case the response has Content-Lenght defined. + // According to the WinHttp team, the feature-detection logic in WinHttpTrailersHelper.OsSupportsTrailers + // should also indicate the support of WINHTTP_OPTION_REQUIRE_STREAM_END. + // WINHTTP_OPTION_REQUIRE_STREAM_END doesn't have effect on HTTP 1.1 requests, therefore it's safe to set it on + // the session handle so it is inhereted by all request handles. + uint optionData = 1; + if (!Interop.WinHttp.WinHttpSetOption(sessionHandle, Interop.WinHttp.WINHTTP_OPTION_REQUIRE_STREAM_END, ref optionData)) + { + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Info(this, "Failed to enable WINHTTP_OPTION_REQUIRE_STREAM_END error code: " + Marshal.GetLastWin32Error()); + } + } + } + } + private void SetSessionHandleConnectionOptions(SafeWinHttpHandle sessionHandle) { uint optionData = (uint)_maxConnectionsPerServer; @@ -1534,22 +1556,6 @@ private void SetRequestHandleHttp2Options(SafeWinHttpHandle requestHandle, Versi { if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "HTTP/2 option not supported"); } - - if (requestVersion == HttpVersion20 && WinHttpTrailersHelper.OsSupportsTrailers) - { - // Setting WINHTTP_OPTION_REQUIRE_STREAM_END to TRUE is needed for WinHttp to read trailing headers - // in case the response has Content-Lenght defined. - // According to the WinHttp team, the feature-detection logic in WinHttpTrailersHelper.OsSupportsTrailers - // should also indicate the support of WINHTTP_OPTION_REQUIRE_STREAM_END. - optionData = 1; - if (!Interop.WinHttp.WinHttpSetOption(requestHandle, Interop.WinHttp.WINHTTP_OPTION_REQUIRE_STREAM_END, ref optionData)) - { - if (NetEventSource.Log.IsEnabled()) - { - NetEventSource.Info(this, "Failed to enable WINHTTP_OPTION_REQUIRE_STREAM_END error code: " + Marshal.GetLastWin32Error()); - } - } - } } private void SetWinHttpOption(SafeWinHttpHandle handle, uint option, ref uint optionData)