From 06569153251c915bcdf2519d8a68a5456058c63f Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 4 Jun 2025 17:59:08 +0200 Subject: [PATCH 01/14] Validate headers for latin 1 and exclude \0 --- .../src/Resources/Strings.resx | 3 ++ .../src/System/Net/Http/WinHttpHandler.cs | 36 +++++++++++++++++-- .../Http/Headers/AuthenticationHeaderValue.cs | 2 +- .../Net/Http/Headers/GenericHeaderParser.cs | 2 +- .../System/Net/Http/Headers/HttpHeaders.cs | 6 ++-- .../Net/Http/Headers/NameValueHeaderValue.cs | 2 +- .../System/Net/Http/HttpResponseMessage.cs | 2 +- .../src/System/Net/Http/HttpRuleParser.cs | 6 ++-- 8 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx b/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx index 82e7445d67e742..ac1f180faeab00 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx @@ -135,4 +135,7 @@ Request version value must be one of 1.0, 1.1, 2.0, or 3.0. + + {0} headers must be valid Latin-1 characters. + 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 fd61e8fab619fc..fb332351ff573c 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 @@ -714,6 +714,20 @@ private static WinHttpChunkMode GetChunkedModeForSend(HttpRequestMessage request return chunkedMode; } +#if NETFRAMEWORK + private static bool ContainsOnlyValidLatin1(string headerString) + { + foreach (char c in headerString) + { + if (c <= 0 || c > 255 || c == '\r' || c == '\n') + { + return false; + } + } + return true; + } +#endif + private static void AddRequestHeaders( SafeWinHttpHandle requestHandle, HttpRequestMessage requestMessage, @@ -744,7 +758,16 @@ private static void AddRequestHeaders( } // Serialize general request headers. - requestHeadersBuffer.AppendLine(requestMessage.Headers.ToString()); + string requestHeaders = requestMessage.Headers.ToString(); +#if NETFRAMEWORK + if (!ContainsOnlyValidLatin1(requestHeaders)) +#else + if (requestHeaders.ContainsAnyExceptInRange((char)1, (char)255)) +#endif + { + throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpRequestMessage))); + } + requestHeadersBuffer.AppendLine(requestHeaders); // Serialize entity-body (content) headers. if (requestMessage.Content != null) @@ -759,7 +782,16 @@ private static void AddRequestHeaders( requestMessage.Content.Headers.ContentLength = contentLength; } - requestHeadersBuffer.AppendLine(requestMessage.Content.Headers.ToString()); + string contentHeaders = requestMessage.Content.Headers.ToString(); +#if NETFRAMEWORK + if (!ContainsOnlyValidLatin1(requestHeaders)) +#else + if (requestHeaders.ContainsAnyExceptInRange((char)1, (char)255)) +#endif + { + throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpContent))); + } + requestHeadersBuffer.AppendLine(contentHeaders); } // Add request headers to WinHTTP request handle. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs index 930cb8f8408587..e8291087b406a9 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs @@ -119,7 +119,7 @@ internal static int GetAuthenticationLength(string? input, int startIndex, out o parsedValue = null; - if (string.IsNullOrEmpty(input) || (startIndex >= input.Length) || HttpRuleParser.ContainsNewLine(input, startIndex)) + if (string.IsNullOrEmpty(input) || (startIndex >= input.Length) || HttpRuleParser.ContainsNewLineOrNull(input, startIndex)) { return 0; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs index 9e93c595f377e3..6706eb10064533 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs @@ -119,7 +119,7 @@ private static int ParseMultipleEntityTags(string value, int startIndex, out obj /// private static int ParseWithoutValidation(string value, int startIndex, out object? parsedValue) { - if (HttpRuleParser.ContainsNewLine(value, startIndex)) + if (HttpRuleParser.ContainsNewLineOrNull(value, startIndex)) { parsedValue = null; return 0; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs index ab0df44d9d566d..76f3f926243180 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs @@ -806,7 +806,7 @@ private static void ParseSingleRawHeaderValue(HeaderStoreItemInfo info, HeaderDe Debug.Assert(Monitor.IsEntered(info)); if (descriptor.Parser == null) { - if (HttpRuleParser.ContainsNewLine(rawValue)) + if (HttpRuleParser.ContainsNewLineOrNull(rawValue)) { if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(null, SR.Format(SR.net_http_log_headers_no_newlines, descriptor.Name, rawValue)); AddInvalidValue(info, rawValue); @@ -1024,7 +1024,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte if (descriptor.Parser == null) { // If we don't have a parser for the header, we consider the value valid if it doesn't contains - // newline characters. We add the values as "parsed value". Note that we allow empty values. + // newline or \0 characters. We add the values as "parsed value". Note that we allow empty values. CheckContainsNewLine(value); AddParsedValue(info, value ?? string.Empty); return; @@ -1134,7 +1134,7 @@ internal static void CheckContainsNewLine(string? value) return; } - if (HttpRuleParser.ContainsNewLine(value)) + if (HttpRuleParser.ContainsNewLineOrNull(value)) { throw new FormatException(SR.net_http_headers_no_newlines); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs index 6920a881828fcc..694fdd287bee4d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs @@ -371,7 +371,7 @@ private static void CheckValueFormat(string? value) ThrowFormatException(value); } } - else if (HttpRuleParser.ContainsNewLine(value)) + else if (HttpRuleParser.ContainsNewLineOrNull(value)) { ThrowFormatException(value); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs index a24dc5d457d040..9957aa41d62089 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs @@ -90,7 +90,7 @@ public string? ReasonPhrase } set { - if ((value != null) && HttpRuleParser.ContainsNewLine(value)) + if ((value != null) && HttpRuleParser.ContainsNewLineOrNull(value)) { throw new FormatException(SR.net_http_reasonphrase_format_error); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs index c64cdc02c8da9a..8ca876f0b7e56b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs @@ -86,8 +86,10 @@ internal static int GetWhitespaceLength(string input, int startIndex) return input.Length - startIndex; } - internal static bool ContainsNewLine(string value, int startIndex = 0) => - value.AsSpan(startIndex).ContainsAny('\r', '\n'); + // See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5: + // "Field values containing CR, LF, or NUL characters are invalid and dangerous" + internal static bool ContainsNewLineOrNull(string value, int startIndex = 0) => + value.AsSpan(startIndex).ContainsAny('\r', '\n', '\0'); internal static int GetNumberLength(string input, int startIndex, bool allowDecimal) { From 47ebafadd9b8d2c64594f8a83892015f210926ad Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 5 Jun 2025 13:15:54 +0200 Subject: [PATCH 02/14] Fix and tests --- .../src/System/Net/Http/WinHttpHandler.cs | 14 +-- .../tests/UnitTests/WinHttpHandlerTest.cs | 90 ++++++++++++++++++- 2 files changed, 92 insertions(+), 12 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 fb332351ff573c..d1417aeb84c5e3 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 @@ -714,11 +714,12 @@ private static WinHttpChunkMode GetChunkedModeForSend(HttpRequestMessage request return chunkedMode; } -#if NETFRAMEWORK private static bool ContainsOnlyValidLatin1(string headerString) { foreach (char c in headerString) { + // See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5: + // "Field values containing CR, LF, or NUL characters are invalid and dangerous" if (c <= 0 || c > 255 || c == '\r' || c == '\n') { return false; @@ -726,7 +727,6 @@ private static bool ContainsOnlyValidLatin1(string headerString) } return true; } -#endif private static void AddRequestHeaders( SafeWinHttpHandle requestHandle, @@ -759,11 +759,7 @@ private static void AddRequestHeaders( // Serialize general request headers. string requestHeaders = requestMessage.Headers.ToString(); -#if NETFRAMEWORK if (!ContainsOnlyValidLatin1(requestHeaders)) -#else - if (requestHeaders.ContainsAnyExceptInRange((char)1, (char)255)) -#endif { throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpRequestMessage))); } @@ -783,11 +779,7 @@ private static void AddRequestHeaders( } string contentHeaders = requestMessage.Content.Headers.ToString(); -#if NETFRAMEWORK - if (!ContainsOnlyValidLatin1(requestHeaders)) -#else - if (requestHeaders.ContainsAnyExceptInRange((char)1, (char)255)) -#endif + if (!ContainsOnlyValidLatin1(contentHeaders)) { throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpContent))); } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs index 1945a41ede7fd1..8da34616bd1ad0 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs @@ -121,7 +121,7 @@ public void TcpKeepalive_WhenDisabled_DoesntSetOptions() SendRequestHelper.Send( handler, - () => handler.TcpKeepAliveEnabled = false ); + () => handler.TcpKeepAliveEnabled = false); Assert.Null(APICallHistory.WinHttpOptionTcpKeepAlive); } @@ -837,6 +837,94 @@ public void SendAsync_MultipleCallsWithDispose_NoHandleLeaksManuallyVerifiedUsin } } + [Theory] + [InlineData('\r', HeaderType.Request)] + [InlineData('\n', HeaderType.Request)] + [InlineData('\0', HeaderType.Request)] + [InlineData('\r', HeaderType.Content)] + [InlineData('\n', HeaderType.Content)] + [InlineData('\0', HeaderType.Content)] + [InlineData('\r', HeaderType.Cookie)] + [InlineData('\n', HeaderType.Cookie)] + [InlineData('\0', HeaderType.Cookie)] + public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpRequestException(char dangerousChar, HeaderType headerType) + { + var handler = new WinHttpHandler(); + using (var client = new HttpClient(handler)) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + switch (headerType) + { + case HeaderType.Request: + request.Headers.Add("Custom-Header", $"HeaderValue{dangerousChar}WithControlChar"); + break; + case HeaderType.Content: + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{dangerousChar}WithControlChar"); + break; + case HeaderType.Cookie: + handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; + handler.CookieContainer = new CookieContainer(); + handler.CookieContainer.Add(new Uri(TestServer.FakeServerEndpoint), new Cookie("CustomCookie", $"Value{dangerousChar}WithControlChar")); + break; + } + + var ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + var fex = Assert.IsType(ex.InnerException); + Assert.Contains("Latin-1", fex.Message); + } + } + + [Theory] + [InlineData('\u00A9', HeaderType.Request)] + [InlineData('\u00FF', HeaderType.Request)] + [InlineData('\u0001', HeaderType.Request)] + [InlineData('\u00A9', HeaderType.Content)] + [InlineData('\u00FF', HeaderType.Content)] + [InlineData('\u0001', HeaderType.Content)] + [InlineData('\u00A9', HeaderType.Cookie)] + [InlineData('\u00FF', HeaderType.Cookie)] + [InlineData('\u0001', HeaderType.Cookie)] + public async Task SendAsync_RequestWithLatin1HeaderValue_Succeeds(char safeChar, HeaderType headerType) + { + var handler = new WinHttpHandler(); + using (var client = new HttpClient(handler)) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + switch (headerType) + { + case HeaderType.Request: + request.Headers.Add("Custom-Header", $"HeaderValue{safeChar}WithSafeChar"); + break; + case HeaderType.Content: + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{safeChar}WithSafeChar"); + break; + case HeaderType.Cookie: + handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; + handler.CookieContainer = new CookieContainer(); + handler.CookieContainer.Add(new Uri(TestServer.FakeServerEndpoint), new Cookie("CustomCookie", $"Value{safeChar}WithSafeChar")); + break; + } + + using (HttpResponseMessage response = await client.SendAsync(request)) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + } + + public enum HeaderType + { + Request, + Content, + Cookie + } + // Commented out as the test relies on finalizer for cleanup and only has value as written // when run on its own and manual analysis is done of logs. //[Fact] From 91b123ade8f2a2110f1f3ae2a60800a388dac206 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 5 Jun 2025 17:29:57 +0200 Subject: [PATCH 03/14] More tests / fixes, WIP --- .../src/System/Net/Http/WinHttpHandler.cs | 6 ++---- .../tests/UnitTests/WinHttpHandlerTest.cs | 19 ++++++++++++++----- .../src/Resources/Strings.resx | 4 ++-- .../Http/Headers/AuthenticationHeaderValue.cs | 2 +- .../System/Net/Http/Headers/HttpHeaders.cs | 6 +++--- .../Net/Http/Headers/HttpRequestHeaders.cs | 4 ++-- .../UnitTests/Headers/HttpHeadersTest.cs | 2 ++ 7 files changed, 26 insertions(+), 17 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 d1417aeb84c5e3..f9951e807b38c0 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 @@ -718,9 +718,7 @@ private static bool ContainsOnlyValidLatin1(string headerString) { foreach (char c in headerString) { - // See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5: - // "Field values containing CR, LF, or NUL characters are invalid and dangerous" - if (c <= 0 || c > 255 || c == '\r' || c == '\n') + if (c <= 0 || c > 255) { return false; } @@ -1591,7 +1589,7 @@ private static void HandleAsyncException(WinHttpRequestState state, Exception ex // If the exception was due to the cancellation token being canceled, throw cancellation exception. state.Tcs.TrySetCanceled(state.CancellationToken); } - else if (ex is WinHttpException || ex is IOException || ex is InvalidOperationException) + else if (ex is WinHttpException || ex is IOException || ex is InvalidOperationException || ex is FormatException) { // Wrap expected exceptions as HttpRequestExceptions since this is considered an error during // execution. All other exception types, including ArgumentExceptions and ProtocolViolationExceptions diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs index 8da34616bd1ad0..61360c5cd32658 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs @@ -841,20 +841,23 @@ public void SendAsync_MultipleCallsWithDispose_NoHandleLeaksManuallyVerifiedUsin [InlineData('\r', HeaderType.Request)] [InlineData('\n', HeaderType.Request)] [InlineData('\0', HeaderType.Request)] + [InlineData('\u0100', HeaderType.Request)] [InlineData('\r', HeaderType.Content)] [InlineData('\n', HeaderType.Content)] [InlineData('\0', HeaderType.Content)] + [InlineData('\u0100', HeaderType.Content)] [InlineData('\r', HeaderType.Cookie)] [InlineData('\n', HeaderType.Cookie)] [InlineData('\0', HeaderType.Cookie)] + [InlineData('\u0100', HeaderType.Cookie)] public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpRequestException(char dangerousChar, HeaderType headerType) { var handler = new WinHttpHandler(); - using (var client = new HttpClient(handler)) - { - TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); - var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + try + { switch (headerType) { case HeaderType.Request: @@ -870,7 +873,13 @@ public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpReq handler.CookieContainer.Add(new Uri(TestServer.FakeServerEndpoint), new Cookie("CustomCookie", $"Value{dangerousChar}WithControlChar")); break; } - + } + catch (FormatException fex) when (fex.Message.Contains("New-line or NUL")) + { + return; + } + using (var client = new HttpClient(handler)) + { var ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); var fex = Assert.IsType(ex.InnerException); Assert.Contains("Latin-1", fex.Message); diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 8b1e61d8cda685..c9a0c293576ec7 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -150,8 +150,8 @@ Invalid range. At least one of the two parameters must not be null. - - New-line characters are not allowed in header values. + + New-line or NUL characters are not allowed in header values. Cannot write more bytes to the buffer than the configured maximum buffer size: {0}. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs index e8291087b406a9..129f62bc68dddf 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs @@ -38,7 +38,7 @@ public AuthenticationHeaderValue(string scheme) public AuthenticationHeaderValue(string scheme, string? parameter) { HeaderUtilities.CheckValidToken(scheme); - HttpHeaders.CheckContainsNewLine(parameter); + HttpHeaders.ContainsNewLineOrNull(parameter); _scheme = scheme; _parameter = parameter; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs index 76f3f926243180..06054c3cb2af0a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs @@ -1025,7 +1025,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte { // If we don't have a parser for the header, we consider the value valid if it doesn't contains // newline or \0 characters. We add the values as "parsed value". Note that we allow empty values. - CheckContainsNewLine(value); + ContainsNewLineOrNull(value); AddParsedValue(info, value ?? string.Empty); return; } @@ -1127,7 +1127,7 @@ internal bool TryGetHeaderDescriptor(string name, out HeaderDescriptor descripto return false; } - internal static void CheckContainsNewLine(string? value) + internal static void ContainsNewLineOrNull(string? value) { if (value == null) { @@ -1136,7 +1136,7 @@ internal static void CheckContainsNewLine(string? value) if (HttpRuleParser.ContainsNewLineOrNull(value)) { - throw new FormatException(SR.net_http_headers_no_newlines); + throw new FormatException(SR.net_http_headers_no_newlines_no_nul); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs index a77f5ca17f6114..9ae2a8b1c9f009 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs @@ -97,7 +97,7 @@ public string? From value = null; } - CheckContainsNewLine(value); + ContainsNewLineOrNull(value); SetOrRemoveParsedValue(KnownHeaders.From.Descriptor, value); } @@ -167,7 +167,7 @@ public string? Protocol get => _specialCollectionsSlots is null ? null : (string?)_specialCollectionsSlots[ProtocolSlot]; set { - CheckContainsNewLine(value); + ContainsNewLineOrNull(value); _specialCollectionsSlots ??= new object[NumCollectionsSlots]; _specialCollectionsSlots[ProtocolSlot] = value; } diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs index 411a6d9f70b3b1..44495c9593c66c 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs @@ -2562,6 +2562,8 @@ public void TryAddInvalidHeader_ShouldThrowFormatException() [InlineData("invalid\r")] [InlineData("\r\n")] [InlineData("invalid\r\n")] + [InlineData("\0")] + [InlineData("invalid\0")] public void TryGetValues_InvalidValuesContainingNewLines_ShouldNotRemoveInvalidValueAndShouldReturnRequestedValue(string value) { const string Name = "custom"; From fa8776aa1afdac538cbf13154febc28401f22a87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Mon, 9 Jun 2025 15:26:00 +0200 Subject: [PATCH 04/14] Post rebase fix --- .../src/System/Net/Http/Headers/AltSvcHeaderParser.cs | 2 +- .../src/System/Net/Http/Headers/CookieHeaderParser.cs | 2 +- .../System.Net.Http/src/System/Net/Http/HttpRuleParser.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AltSvcHeaderParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AltSvcHeaderParser.cs index 180a25131c7582..25d32b1620c374 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AltSvcHeaderParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AltSvcHeaderParser.cs @@ -279,7 +279,7 @@ private static bool TryReadUnknownPercentEncodedAlpnProtocolName(ReadOnlySpan diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CookieHeaderParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CookieHeaderParser.cs index cc9dcbc7b5321d..75101b933d1208 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CookieHeaderParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CookieHeaderParser.cs @@ -22,7 +22,7 @@ public override bool TryParseValue(string? value, object? storeValue, ref int in Debug.Assert(index == 0); // Some headers support empty/null values. This one doesn't. - if (string.IsNullOrEmpty(value) || HttpRuleParser.ContainsNewLine(value)) + if (string.IsNullOrEmpty(value) || HttpRuleParser.ContainsNewLineOrNull(value)) { parsedValue = null; return false; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs index 8ca876f0b7e56b..0ff91456586c5b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs @@ -320,7 +320,7 @@ private static bool IsValidHostName(ReadOnlySpan host) return false; } - Debug.Assert(!ContainsNewLine(host.ToString())); + Debug.Assert(!ContainsNewLineOrNull(host.ToString())); return true; } } From f3e5438b13d35c355cdfecfc6847cfacea8d4c15 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 9 Jun 2025 17:30:59 +0200 Subject: [PATCH 05/14] Moved and fixed tests, fixed cookies --- .../System/Net/Http/HttpClientHandlerTest.cs | 124 ++++++++++++++++++ .../src/System/Net/Http/WinHttpHandler.cs | 4 + .../tests/UnitTests/WinHttpHandlerTest.cs | 97 -------------- .../src/System/Net/Cookie.cs | 4 +- 4 files changed, 130 insertions(+), 99 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 582e4b8a1fd434..4b4253fab1d072 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2150,5 +2150,129 @@ public async Task SendAsync_InvalidRequestUri_Throws() request = new HttpRequestMessage(HttpMethod.Get, new Uri("foo://foo.bar")); await Assert.ThrowsAsync(() => invoker.SendAsync(request, CancellationToken.None)); } + + [Theory] + [InlineData('\r', HeaderType.Request)] + [InlineData('\n', HeaderType.Request)] + [InlineData('\0', HeaderType.Request)] + [InlineData('\u0100', HeaderType.Request)] + [InlineData('\r', HeaderType.Content)] + [InlineData('\n', HeaderType.Content)] + [InlineData('\0', HeaderType.Content)] + [InlineData('\u0100', HeaderType.Content)] + [InlineData('\r', HeaderType.Cookie)] + [InlineData('\n', HeaderType.Cookie)] + [InlineData('\0', HeaderType.Cookie)] + [InlineData('\u0100', HeaderType.Cookie)] + public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpRequestException(char dangerousChar, HeaderType headerType) + { + string uri = "https://example.com"; // URI doesn't matter, the request should never leave the client + var handler = CreateHttpClientHandler(); + + var request = new HttpRequestMessage(HttpMethod.Get, uri); + request.Version = UseVersion; + try + { + switch (headerType) + { + case HeaderType.Request: + request.Headers.Add("Custom-Header", $"HeaderValue{dangerousChar}WithControlChar"); + break; + case HeaderType.Content: + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{dangerousChar}WithControlChar"); + break; + case HeaderType.Cookie: +#if WINHTTPHANDLER_TEST + handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; +#endif + handler.CookieContainer = new CookieContainer(); + handler.CookieContainer.Add(new Uri(uri), new Cookie("CustomCookie", $"Value{dangerousChar}WithControlChar")); + break; + } + } + catch (FormatException fex) when (fex.Message.Contains("New-line or NUL") && dangerousChar != '\u0100') + { + return; + } + catch (CookieException) when (dangerousChar != '\u0100') + { + return; + } + + using (var client = new HttpClient(handler)) + { + var ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + if (IsWinHttpHandler) + { + var fex = Assert.IsType(ex.InnerException); + Assert.Contains("Latin-1", fex.Message); + } + else + { + var message = UseVersion == HttpVersion30 ? ex.InnerException.Message : ex.Message; + Assert.Contains("ASCII", message); + } + } + } + + [Theory] + [InlineData('\u00A9', HeaderType.Request)] + [InlineData('\u00FF', HeaderType.Request)] + [InlineData('\u0001', HeaderType.Request)] + [InlineData('\u00A9', HeaderType.Content)] + [InlineData('\u00FF', HeaderType.Content)] + [InlineData('\u0001', HeaderType.Content)] + [InlineData('\u00A9', HeaderType.Cookie)] + [InlineData('\u00FF', HeaderType.Cookie)] + [InlineData('\u0001', HeaderType.Cookie)] + public async Task SendAsync_RequestWithLatin1HeaderValue_Succeeds(char safeChar, HeaderType headerType) + { + if (!IsWinHttpHandler && safeChar > 0x7F) + { + return; // SocketsHttpHandler doesn't support Latin-1 characters in headers without setting header encoding. + } + await LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + var handler = CreateHttpClientHandler(); + using (var client = new HttpClient(handler)) + { + var request = new HttpRequestMessage(HttpMethod.Get, uri); + request.Version = UseVersion; + switch (headerType) + { + case HeaderType.Request: + request.Headers.Add("Custom-Header", $"HeaderValue{safeChar}WithSafeChar"); + break; + case HeaderType.Content: + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{safeChar}WithSafeChar"); + break; + case HeaderType.Cookie: +#if WINHTTPHANDLER_TEST + handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; +#endif + handler.CookieContainer = new CookieContainer(); + handler.CookieContainer.Add(uri, new Cookie("CustomCookie", $"Value{safeChar}WithSafeChar")); + break; + } + + using (HttpResponseMessage response = await client.SendAsync(request)) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + }, async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + }); + } + + public enum HeaderType + { + Request, + Content, + Cookie + } } } 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 f9951e807b38c0..55096ba867f301 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 @@ -751,6 +751,10 @@ private static void AddRequestHeaders( string? cookieHeader = WinHttpCookieContainerAdapter.GetCookieHeader(requestMessage.RequestUri, cookies); if (!string.IsNullOrEmpty(cookieHeader)) { + if (!ContainsOnlyValidLatin1(cookieHeader)) + { + throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpRequestMessage))); + } requestHeadersBuffer.AppendLine(cookieHeader); } } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs index 61360c5cd32658..2505480230e59e 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs @@ -837,103 +837,6 @@ public void SendAsync_MultipleCallsWithDispose_NoHandleLeaksManuallyVerifiedUsin } } - [Theory] - [InlineData('\r', HeaderType.Request)] - [InlineData('\n', HeaderType.Request)] - [InlineData('\0', HeaderType.Request)] - [InlineData('\u0100', HeaderType.Request)] - [InlineData('\r', HeaderType.Content)] - [InlineData('\n', HeaderType.Content)] - [InlineData('\0', HeaderType.Content)] - [InlineData('\u0100', HeaderType.Content)] - [InlineData('\r', HeaderType.Cookie)] - [InlineData('\n', HeaderType.Cookie)] - [InlineData('\0', HeaderType.Cookie)] - [InlineData('\u0100', HeaderType.Cookie)] - public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpRequestException(char dangerousChar, HeaderType headerType) - { - var handler = new WinHttpHandler(); - TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); - - var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); - try - { - switch (headerType) - { - case HeaderType.Request: - request.Headers.Add("Custom-Header", $"HeaderValue{dangerousChar}WithControlChar"); - break; - case HeaderType.Content: - request.Content = new StringContent("test content"); - request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{dangerousChar}WithControlChar"); - break; - case HeaderType.Cookie: - handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; - handler.CookieContainer = new CookieContainer(); - handler.CookieContainer.Add(new Uri(TestServer.FakeServerEndpoint), new Cookie("CustomCookie", $"Value{dangerousChar}WithControlChar")); - break; - } - } - catch (FormatException fex) when (fex.Message.Contains("New-line or NUL")) - { - return; - } - using (var client = new HttpClient(handler)) - { - var ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); - var fex = Assert.IsType(ex.InnerException); - Assert.Contains("Latin-1", fex.Message); - } - } - - [Theory] - [InlineData('\u00A9', HeaderType.Request)] - [InlineData('\u00FF', HeaderType.Request)] - [InlineData('\u0001', HeaderType.Request)] - [InlineData('\u00A9', HeaderType.Content)] - [InlineData('\u00FF', HeaderType.Content)] - [InlineData('\u0001', HeaderType.Content)] - [InlineData('\u00A9', HeaderType.Cookie)] - [InlineData('\u00FF', HeaderType.Cookie)] - [InlineData('\u0001', HeaderType.Cookie)] - public async Task SendAsync_RequestWithLatin1HeaderValue_Succeeds(char safeChar, HeaderType headerType) - { - var handler = new WinHttpHandler(); - using (var client = new HttpClient(handler)) - { - TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); - - var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); - switch (headerType) - { - case HeaderType.Request: - request.Headers.Add("Custom-Header", $"HeaderValue{safeChar}WithSafeChar"); - break; - case HeaderType.Content: - request.Content = new StringContent("test content"); - request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{safeChar}WithSafeChar"); - break; - case HeaderType.Cookie: - handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; - handler.CookieContainer = new CookieContainer(); - handler.CookieContainer.Add(new Uri(TestServer.FakeServerEndpoint), new Cookie("CustomCookie", $"Value{safeChar}WithSafeChar")); - break; - } - - using (HttpResponseMessage response = await client.SendAsync(request)) - { - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - } - } - } - - public enum HeaderType - { - Request, - Content, - Cookie - } - // Commented out as the test relies on finalizer for cleanup and only has value as written // when run on its own and manual analysis is done of logs. //[Fact] diff --git a/src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs b/src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs index d4341314bb1163..4f55b3d462d6ab 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs @@ -42,7 +42,7 @@ public sealed class Cookie internal static readonly char[] PortSplitDelimiters = new char[] { ' ', ',', '\"' }; // Space (' ') should be reserved as well per RFCs, but major web browsers support it and some web sites use it - so we support it too - private static readonly SearchValues s_reservedToNameChars = SearchValues.Create("\t\r\n=;,"); + private static readonly SearchValues s_reservedToNameChars = SearchValues.Create("\t\0\r\n=;,"); private static readonly SearchValues s_domainChars = SearchValues.Create("-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz"); @@ -384,7 +384,7 @@ internal void VerifyAndSetDefaults(CookieVariant variant, Uri uri) } // Check the value - if (m_value == null || + if (m_value == null || m_value.ContainsAny('\r', '\n', '\0') || (!(m_value.Length > 2 && m_value.StartsWith('\"') && m_value.EndsWith('\"')) && m_value.AsSpan().ContainsAny(';', ','))) { throw new CookieException(SR.Format(SR.net_cookie_attribute, "Value", m_value ?? "")); From e0fe847b4d9f10e45bfe8abb5ccb728d4e309559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Fri, 13 Jun 2025 16:56:02 +0200 Subject: [PATCH 06/14] Fixed windows part --- .../System/Net/Http/HttpClientHandlerTest.cs | 8 ++-- .../src/Resources/Strings.resx | 2 +- .../Net/Http/WinHttpCookieContainerAdapter.cs | 10 ++++- .../src/System/Net/Http/WinHttpHandler.cs | 42 ++++++------------- 4 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 4b4253fab1d072..b55a0c21e975e4 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2202,15 +2202,17 @@ public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpReq using (var client = new HttpClient(handler)) { - var ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + var ex = await Assert.ThrowsAnyAsync(() => client.SendAsync(request)); + _output.WriteLine(ex.ToString()); if (IsWinHttpHandler) { - var fex = Assert.IsType(ex.InnerException); + var fex = Assert.IsType(ex); Assert.Contains("Latin-1", fex.Message); } else { - var message = UseVersion == HttpVersion30 ? ex.InnerException.Message : ex.Message; + var hrex = Assert.IsType(ex); + var message = UseVersion == HttpVersion30 ? hrex.InnerException.Message : hrex.Message; Assert.Contains("ASCII", message); } } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx b/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx index ac1f180faeab00..1746057dc5aecd 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx @@ -136,6 +136,6 @@ Request version value must be one of 1.0, 1.1, 2.0, or 3.0. - {0} headers must be valid Latin-1 characters. + Request headers must be valid Latin-1 characters. diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs index 739995f5e732c6..b464672d8bb466 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs @@ -80,7 +80,15 @@ public static void ResetCookieRequestHeaders(WinHttpRequestState state, Uri redi (uint)cookieHeader.Length, Interop.WinHttp.WINHTTP_ADDREQ_FLAG_ADD)) { - WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpAddRequestHeaders)); + int lastError = Marshal.GetLastWin32Error(); + if (lastError == Interop.WinHttp.ERROR_INVALID_PARAMETER) + { + throw new FormatException(SR.net_http_invalid_header_value); + } + else + { + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpAddRequestHeaders)); + } } } } 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 55096ba867f301..ce072519f4cad2 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 @@ -714,18 +714,6 @@ private static WinHttpChunkMode GetChunkedModeForSend(HttpRequestMessage request return chunkedMode; } - private static bool ContainsOnlyValidLatin1(string headerString) - { - foreach (char c in headerString) - { - if (c <= 0 || c > 255) - { - return false; - } - } - return true; - } - private static void AddRequestHeaders( SafeWinHttpHandle requestHandle, HttpRequestMessage requestMessage, @@ -751,21 +739,12 @@ private static void AddRequestHeaders( string? cookieHeader = WinHttpCookieContainerAdapter.GetCookieHeader(requestMessage.RequestUri, cookies); if (!string.IsNullOrEmpty(cookieHeader)) { - if (!ContainsOnlyValidLatin1(cookieHeader)) - { - throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpRequestMessage))); - } requestHeadersBuffer.AppendLine(cookieHeader); } } // Serialize general request headers. - string requestHeaders = requestMessage.Headers.ToString(); - if (!ContainsOnlyValidLatin1(requestHeaders)) - { - throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpRequestMessage))); - } - requestHeadersBuffer.AppendLine(requestHeaders); + requestHeadersBuffer.AppendLine(requestMessage.Headers.ToString()); // Serialize entity-body (content) headers. if (requestMessage.Content != null) @@ -780,12 +759,7 @@ private static void AddRequestHeaders( requestMessage.Content.Headers.ContentLength = contentLength; } - string contentHeaders = requestMessage.Content.Headers.ToString(); - if (!ContainsOnlyValidLatin1(contentHeaders)) - { - throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpContent))); - } - requestHeadersBuffer.AppendLine(contentHeaders); + requestHeadersBuffer.AppendLine(requestMessage.Content.Headers.ToString()); } // Add request headers to WinHTTP request handle. @@ -795,7 +769,15 @@ private static void AddRequestHeaders( (uint)requestHeadersBuffer.Length, Interop.WinHttp.WINHTTP_ADDREQ_FLAG_ADD)) { - WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpAddRequestHeaders)); + int lastError = Marshal.GetLastWin32Error(); + if (lastError == Interop.WinHttp.ERROR_INVALID_PARAMETER) + { + throw new FormatException(SR.net_http_invalid_header_value); + } + else + { + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpAddRequestHeaders)); + } } } @@ -1593,7 +1575,7 @@ private static void HandleAsyncException(WinHttpRequestState state, Exception ex // If the exception was due to the cancellation token being canceled, throw cancellation exception. state.Tcs.TrySetCanceled(state.CancellationToken); } - else if (ex is WinHttpException || ex is IOException || ex is InvalidOperationException || ex is FormatException) + else if (ex is WinHttpException || ex is IOException || ex is InvalidOperationException) { // Wrap expected exceptions as HttpRequestExceptions since this is considered an error during // execution. All other exception types, including ArgumentExceptions and ProtocolViolationExceptions From 0c0826359fa3bfefb993ca78efdd14b96a303b22 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 16 Jun 2025 17:11:26 +0200 Subject: [PATCH 07/14] Feedback --- .../Fuzzing/DotnetFuzzing/Fuzzers/HttpHeadersFuzzer.cs | 4 ++-- src/libraries/System.Net.Http/src/Resources/Strings.resx | 2 +- .../src/System/Net/Http/Headers/AuthenticationHeaderValue.cs | 2 +- .../src/System/Net/Http/Headers/HttpHeaders.cs | 4 ++-- .../src/System/Net/Http/Headers/HttpRequestHeaders.cs | 4 ++-- .../System.Net.Http/src/System/Net/Http/HttpRuleParser.cs | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/HttpHeadersFuzzer.cs b/src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/HttpHeadersFuzzer.cs index b7d4586b91f255..09456461f6f568 100644 --- a/src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/HttpHeadersFuzzer.cs +++ b/src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/HttpHeadersFuzzer.cs @@ -78,7 +78,7 @@ static void Test(HttpHeaders headers, string name, string value) { foreach (string headerValue in values) { - Assert.False(headerValue.ContainsAny('\r', '\n')); + Assert.False(headerValue.ContainsAny('\r', '\n', '\0')); } } @@ -86,7 +86,7 @@ static void Test(HttpHeaders headers, string name, string value) { foreach (string headerValue in values) { - Assert.False(headerValue.ContainsAny('\r', '\n')); + Assert.False(headerValue.ContainsAny('\r', '\n', '\0')); } } } diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index c9a0c293576ec7..09d04283e353b9 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -124,7 +124,7 @@ The format of the HTTP method is invalid. - The reason phrase must not contain new-line characters. + The reason phrase must not contain new-line or NUL characters. The number of elements is greater than the available space from arrayIndex to the end of the destination array. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs index 129f62bc68dddf..d83514a6da2a7a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs @@ -38,7 +38,7 @@ public AuthenticationHeaderValue(string scheme) public AuthenticationHeaderValue(string scheme, string? parameter) { HeaderUtilities.CheckValidToken(scheme); - HttpHeaders.ContainsNewLineOrNull(parameter); + HttpHeaders.CheckContainsNewLineOrNull(parameter); _scheme = scheme; _parameter = parameter; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs index 06054c3cb2af0a..35494cbb8535e6 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs @@ -1025,7 +1025,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte { // If we don't have a parser for the header, we consider the value valid if it doesn't contains // newline or \0 characters. We add the values as "parsed value". Note that we allow empty values. - ContainsNewLineOrNull(value); + CheckContainsNewLineOrNull(value); AddParsedValue(info, value ?? string.Empty); return; } @@ -1127,7 +1127,7 @@ internal bool TryGetHeaderDescriptor(string name, out HeaderDescriptor descripto return false; } - internal static void ContainsNewLineOrNull(string? value) + internal static void CheckContainsNewLineOrNull(string? value) { if (value == null) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs index 9ae2a8b1c9f009..2c554b005e012a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs @@ -97,7 +97,7 @@ public string? From value = null; } - ContainsNewLineOrNull(value); + CheckContainsNewLineOrNull(value); SetOrRemoveParsedValue(KnownHeaders.From.Descriptor, value); } @@ -167,7 +167,7 @@ public string? Protocol get => _specialCollectionsSlots is null ? null : (string?)_specialCollectionsSlots[ProtocolSlot]; set { - ContainsNewLineOrNull(value); + CheckContainsNewLineOrNull(value); _specialCollectionsSlots ??= new object[NumCollectionsSlots]; _specialCollectionsSlots[ProtocolSlot] = value; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs index 0ff91456586c5b..63e17c5fa4f643 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs @@ -200,7 +200,7 @@ internal static HttpParseResult GetQuotedPairLength(string input, int startIndex // Quoted-char has 2 characters. Check whether there are 2 chars left ('\' + char) // If so, check whether the character is in the range 0-127 and not a new line. Otherwise, it's an invalid value. - if ((startIndex + 2 > input.Length) || (input[startIndex + 1] is > (char)127 or '\r' or '\n')) + if ((startIndex + 2 > input.Length) || (input[startIndex + 1] is > (char)127 or '\r' or '\n' or '\0')) { return HttpParseResult.InvalidFormat; } @@ -252,7 +252,7 @@ private static HttpParseResult GetExpressionLength(string input, int startIndex, char c = input[current]; - if (c == '\r' || c == '\n') + if (c == '\r' || c == '\n' || c == '\0') { return HttpParseResult.InvalidFormat; } From d01b666b811c514f47a5640ecd1546de27fb7617 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 16 Jun 2025 17:17:16 +0200 Subject: [PATCH 08/14] Fix tests - do not run on browser --- .../Common/tests/System/Net/Http/HttpClientHandlerTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index b55a0c21e975e4..185092a407ad0a 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2151,7 +2151,7 @@ public async Task SendAsync_InvalidRequestUri_Throws() await Assert.ThrowsAsync(() => invoker.SendAsync(request, CancellationToken.None)); } - [Theory] + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] [InlineData('\r', HeaderType.Request)] [InlineData('\n', HeaderType.Request)] [InlineData('\0', HeaderType.Request)] @@ -2218,7 +2218,7 @@ public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpReq } } - [Theory] + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] [InlineData('\u00A9', HeaderType.Request)] [InlineData('\u00FF', HeaderType.Request)] [InlineData('\u0001', HeaderType.Request)] From 790b9fd92d54e509ace4cdbecf4f1358ef9c31bd Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 16 Jun 2025 17:20:21 +0200 Subject: [PATCH 09/14] Add cookie specific tests --- .../tests/FunctionalTests/CookieContainerTest.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieContainerTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieContainerTest.cs index da3bd1f6e909f4..26735bd56d2876 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieContainerTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieContainerTest.cs @@ -139,6 +139,9 @@ public static IEnumerable SetCookiesInvalidData() yield return new object[] { u5, "=value" }; // No name yield return new object[] { u5, "$=value" }; // Invalid name yield return new object[] { new Uri("http://url.com"), "na\tme=value; domain=.domain.com" }; // Invalid name + yield return new object[] { u5, "name=val\rue" }; // Invalid value + yield return new object[] { u5, "name=val\nue" }; // Invalid value + yield return new object[] { u5, "name=val\0ue" }; // Invalid value yield return new object[] { new Uri("http://url.com"), "name=value; domain=.domain.com" }; // Domain not the same yield return new object[] { new Uri("http://url.com:90"), "name=value; port=\"80\"" }; // Port not the same yield return new object[] { new Uri("http://url.com"), "name=value; domain=" }; // Empty domain From 23a6fb26c79cc11e448423bc7d05eb330bfb99fe Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 17 Jun 2025 17:01:40 +0200 Subject: [PATCH 10/14] Assert header value in possitive tests --- .../System/Net/Http/HttpClientHandlerTest.cs | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 185092a407ad0a..5ebd4ff4b8e9a4 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2234,6 +2234,7 @@ public async Task SendAsync_RequestWithLatin1HeaderValue_Succeeds(char safeChar, { return; // SocketsHttpHandler doesn't support Latin-1 characters in headers without setting header encoding. } + var headerValue = $"HeaderValue{safeChar}WithSafeChar"; await LoopbackServerFactory.CreateClientAndServerAsync(async uri => { var handler = CreateHttpClientHandler(); @@ -2244,18 +2245,18 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => switch (headerType) { case HeaderType.Request: - request.Headers.Add("Custom-Header", $"HeaderValue{safeChar}WithSafeChar"); + request.Headers.Add("Custom-Header", headerValue); break; case HeaderType.Content: request.Content = new StringContent("test content"); - request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{safeChar}WithSafeChar"); + request.Content.Headers.Add("Custom-Content-Header", headerValue); break; case HeaderType.Cookie: #if WINHTTPHANDLER_TEST handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; #endif handler.CookieContainer = new CookieContainer(); - handler.CookieContainer.Add(uri, new Cookie("CustomCookie", $"Value{safeChar}WithSafeChar")); + handler.CookieContainer.Add(uri, new Cookie("CustomCookie", headerValue)); break; } @@ -2266,7 +2267,19 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => } }, async server => { - await server.AcceptConnectionSendResponseAndCloseAsync(); + var data = await server.AcceptConnectionSendResponseAndCloseAsync(); + switch (headerType) + { + case HeaderType.Request: + Assert.Equal(headerValue, data.GetSingleHeaderValue("Custom-Header")); + break; + case HeaderType.Content: + Assert.Equal(headerValue, data.GetSingleHeaderValue("Custom-Content-Header")); + break; + case HeaderType.Cookie: + Assert.Equal($"CustomCookie={headerValue}", data.GetSingleHeaderValue("cookie")); + break; + } }); } From 6586e9593c0ef04d290f25c2ca74c7304571cc84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Wed, 18 Jun 2025 18:18:00 +0200 Subject: [PATCH 11/14] Expanded and fixed tests on Windows. --- .../System/Net/Http/HttpClientHandlerTest.cs | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 5ebd4ff4b8e9a4..1e3bfcd310a4b0 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2156,14 +2156,20 @@ public async Task SendAsync_InvalidRequestUri_Throws() [InlineData('\n', HeaderType.Request)] [InlineData('\0', HeaderType.Request)] [InlineData('\u0100', HeaderType.Request)] + [InlineData('\u0080', HeaderType.Request)] + [InlineData('\u009F', HeaderType.Request)] [InlineData('\r', HeaderType.Content)] [InlineData('\n', HeaderType.Content)] [InlineData('\0', HeaderType.Content)] [InlineData('\u0100', HeaderType.Content)] + [InlineData('\u0080', HeaderType.Content)] + [InlineData('\u009F', HeaderType.Content)] [InlineData('\r', HeaderType.Cookie)] [InlineData('\n', HeaderType.Cookie)] [InlineData('\0', HeaderType.Cookie)] [InlineData('\u0100', HeaderType.Cookie)] + [InlineData('\u0080', HeaderType.Cookie)] + [InlineData('\u009F', HeaderType.Cookie)] public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpRequestException(char dangerousChar, HeaderType headerType) { string uri = "https://example.com"; // URI doesn't matter, the request should never leave the client @@ -2219,15 +2225,24 @@ public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpReq } [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] + [InlineData('\u0001', HeaderType.Request)] + [InlineData('\u0007', HeaderType.Request)] + [InlineData('\u007F', HeaderType.Request)] + [InlineData('\u00A0', HeaderType.Request)] [InlineData('\u00A9', HeaderType.Request)] [InlineData('\u00FF', HeaderType.Request)] - [InlineData('\u0001', HeaderType.Request)] + [InlineData('\u0001', HeaderType.Content)] + [InlineData('\u0007', HeaderType.Content)] + [InlineData('\u007F', HeaderType.Content)] + [InlineData('\u00A0', HeaderType.Content)] [InlineData('\u00A9', HeaderType.Content)] [InlineData('\u00FF', HeaderType.Content)] - [InlineData('\u0001', HeaderType.Content)] + [InlineData('\u0001', HeaderType.Cookie)] + [InlineData('\u0007', HeaderType.Cookie)] + [InlineData('\u007F', HeaderType.Cookie)] + [InlineData('\u00A0', HeaderType.Cookie)] [InlineData('\u00A9', HeaderType.Cookie)] [InlineData('\u00FF', HeaderType.Cookie)] - [InlineData('\u0001', HeaderType.Cookie)] public async Task SendAsync_RequestWithLatin1HeaderValue_Succeeds(char safeChar, HeaderType headerType) { if (!IsWinHttpHandler && safeChar > 0x7F) @@ -2268,16 +2283,22 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => }, async server => { var data = await server.AcceptConnectionSendResponseAndCloseAsync(); + var expectedValue = headerValue; + if (safeChar > 0x7F) + { + Assert.True(IsWinHttpHandler); + expectedValue = headerValue.Replace(safeChar, '?'); // WinHttpHandler replaces Latin-1 characters with '?'. + } switch (headerType) { case HeaderType.Request: - Assert.Equal(headerValue, data.GetSingleHeaderValue("Custom-Header")); + Assert.Equal(expectedValue, data.GetSingleHeaderValue("Custom-Header")); break; case HeaderType.Content: - Assert.Equal(headerValue, data.GetSingleHeaderValue("Custom-Content-Header")); + Assert.Equal(expectedValue, data.GetSingleHeaderValue("Custom-Content-Header")); break; case HeaderType.Cookie: - Assert.Equal($"CustomCookie={headerValue}", data.GetSingleHeaderValue("cookie")); + Assert.Equal($"CustomCookie={expectedValue}", data.GetSingleHeaderValue("cookie")); break; } }); From 3299dda9bde074569f1809474c1f5e6d3501ce5a Mon Sep 17 00:00:00 2001 From: ManickaP Date: Fri, 20 Jun 2025 13:20:11 +0200 Subject: [PATCH 12/14] Fix tests. --- .../System/Net/Http/GenericLoopbackServer.cs | 18 +++++++++++++ .../System/Net/Http/HttpClientHandlerTest.cs | 25 ++++++++++++------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs index 66c64e3e8a578f..cd358a8f7594cf 100644 --- a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -258,6 +258,24 @@ public static async Task FromHttpRequestMessageAsync(System.Net return result; } + public HttpHeaderData[] GetHeaderData(string headerName) + { + return Headers.Where(h => h.Name.Equals(headerName, StringComparison.OrdinalIgnoreCase)).ToArray(); + } + + public HttpHeaderData GetSingleHeaderData(string headerName) + { + HttpHeaderData[] data = GetHeaderData(headerName); + if (data.Length != 1) + { + throw new Exception( + $"Expected single value for {headerName} header, actual count: {data.Length}{Environment.NewLine}" + + $"{"\t"}{string.Join(Environment.NewLine + "\t", data)}"); + } + + return data[0]; + } + public string[] GetHeaderValues(string headerName) { return Headers.Where(h => h.Name.Equals(headerName, StringComparison.OrdinalIgnoreCase)) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 1e3bfcd310a4b0..85d2694ea08e4a 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2283,23 +2283,30 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => }, async server => { var data = await server.AcceptConnectionSendResponseAndCloseAsync(); - var expectedValue = headerValue; - if (safeChar > 0x7F) - { - Assert.True(IsWinHttpHandler); - expectedValue = headerValue.Replace(safeChar, '?'); // WinHttpHandler replaces Latin-1 characters with '?'. - } + var encoding = Encoding.GetEncoding("ISO-8859-1"); switch (headerType) { case HeaderType.Request: - Assert.Equal(expectedValue, data.GetSingleHeaderValue("Custom-Header")); + { + var headerLine = encoding.GetString(data.GetSingleHeaderData("Custom-Header").Raw); + var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); + Assert.Equal(headerValue, receivedHeaderValue); break; + } case HeaderType.Content: - Assert.Equal(expectedValue, data.GetSingleHeaderValue("Custom-Content-Header")); + { + var headerLine = encoding.GetString(data.GetSingleHeaderData("Custom-Content-Header").Raw); + var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); + Assert.Equal(headerValue, receivedHeaderValue); break; + } case HeaderType.Cookie: - Assert.Equal($"CustomCookie={expectedValue}", data.GetSingleHeaderValue("cookie")); + { + var headerLine = encoding.GetString(data.GetSingleHeaderData("cookie").Raw); + var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); + Assert.Equal(headerValue, receivedHeaderValue); break; + } } }); } From 1e6bb3c089eed21da22594994d12f7c6a08ce681 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 24 Jun 2025 10:34:44 +0200 Subject: [PATCH 13/14] Fix for huffman --- .../Net/Http/Http2LoopbackConnection.cs | 12 +++++------ .../System/Net/Http/HttpClientHandlerTest.cs | 21 +++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index e607c42aa48ba8..769ee3fb062ef7 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -425,7 +425,7 @@ private static (int bytesConsumed, int value) DecodeInteger(ReadOnlySpan h return QPackTestDecoder.DecodeInteger(headerBlock, prefixMask); } - private static (int bytesConsumed, string value) DecodeString(ReadOnlySpan headerBlock) + private static (int bytesConsumed, string value, bool huffmanEncoded) DecodeString(ReadOnlySpan headerBlock) { (int bytesConsumed, int stringLength) = DecodeInteger(headerBlock, 0b01111111); if ((headerBlock[0] & 0b10000000) != 0) @@ -434,12 +434,12 @@ private static (int bytesConsumed, string value) DecodeString(ReadOnlySpan byte[] buffer = new byte[stringLength * 2]; int bytesDecoded = HuffmanDecoder.Decode(headerBlock.Slice(bytesConsumed, stringLength), buffer); string value = Encoding.ASCII.GetString(buffer, 0, bytesDecoded); - return (bytesConsumed + stringLength, value); + return (bytesConsumed + stringLength, value, true); } else { string value = Encoding.ASCII.GetString(headerBlock.Slice(bytesConsumed, stringLength).ToArray()); - return (bytesConsumed + stringLength, value); + return (bytesConsumed + stringLength, value, false); } } @@ -523,7 +523,7 @@ private static (int bytesConsumed, HttpHeaderData headerData) DecodeLiteralHeade string name; if (index == 0) { - (bytesConsumed, name) = DecodeString(headerBlock.Slice(i)); + (bytesConsumed, name, _) = DecodeString(headerBlock.Slice(i)); i += bytesConsumed; } else @@ -532,10 +532,10 @@ private static (int bytesConsumed, HttpHeaderData headerData) DecodeLiteralHeade } string value; - (bytesConsumed, value) = DecodeString(headerBlock.Slice(i)); + (bytesConsumed, value, bool huffmanEncoded) = DecodeString(headerBlock.Slice(i)); i += bytesConsumed; - return (i, new HttpHeaderData(name, value)); + return (i, new HttpHeaderData(name, value, huffmanEncoded)); } private static (int bytesConsumed, HttpHeaderData headerData) DecodeHeader(ReadOnlySpan headerBlock) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 85d2694ea08e4a..88727d7f190f36 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2283,31 +2283,44 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => }, async server => { var data = await server.AcceptConnectionSendResponseAndCloseAsync(); - var encoding = Encoding.GetEncoding("ISO-8859-1"); switch (headerType) { case HeaderType.Request: { - var headerLine = encoding.GetString(data.GetSingleHeaderData("Custom-Header").Raw); + var headerLine = DecodeHeaderValue("Custom-Header"); var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); Assert.Equal(headerValue, receivedHeaderValue); break; } case HeaderType.Content: { - var headerLine = encoding.GetString(data.GetSingleHeaderData("Custom-Content-Header").Raw); + var headerLine = DecodeHeaderValue("Custom-Content-Header"); var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); Assert.Equal(headerValue, receivedHeaderValue); break; } case HeaderType.Cookie: { - var headerLine = encoding.GetString(data.GetSingleHeaderData("cookie").Raw); + var headerLine = DecodeHeaderValue("cookie"); var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); Assert.Equal(headerValue, receivedHeaderValue); break; } } + + string DecodeHeaderValue(string headerName) + { + var encoding = Encoding.GetEncoding("ISO-8859-1"); + HttpHeaderData headerData = data.GetSingleHeaderData(headerName); + byte[] raw = headerData.Raw; + if (headerData.HuffmanEncoded) + { + byte[] buffer = new byte[raw.Length * 2]; + int length = HuffmanDecoder.Decode(headerData.Raw, buffer); + raw = buffer.AsSpan().Slice(0, length).ToArray(); + } + return encoding.GetString(raw); + } }); } From e5674458348ff0953e7d8571288395ec326c4fbd Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 24 Jun 2025 14:23:33 +0200 Subject: [PATCH 14/14] Damn this H/2 Huffman non-sense, give me some strings please, not this binary soup --- .../System/Net/Http/GenericLoopbackServer.cs | 4 +++- .../System/Net/Http/Http2LoopbackConnection.cs | 15 ++++++++------- .../System/Net/Http/HttpClientHandlerTest.cs | 8 ++++---- .../tests/System/Net/Http/LoopbackServer.cs | 2 +- .../tests/System/Net/Http/QPackTestDecoder.cs | 4 ++-- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs index cd358a8f7594cf..93a9c6318b601e 100644 --- a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -199,14 +199,16 @@ public struct HttpHeaderData public string Value { get; } public bool HuffmanEncoded { get; } public byte[] Raw { get; } + public int RawValueStart { get; } public Encoding ValueEncoding { get; } - public HttpHeaderData(string name, string value, bool huffmanEncoded = false, byte[] raw = null, Encoding valueEncoding = null) + public HttpHeaderData(string name, string value, bool huffmanEncoded = false, byte[] raw = null, int rawValueStart = 0, Encoding valueEncoding = null) { Name = name; Value = value; HuffmanEncoded = huffmanEncoded; Raw = raw; + RawValueStart = rawValueStart; ValueEncoding = valueEncoding; } diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index 769ee3fb062ef7..72e5cd31d984ca 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -425,7 +425,7 @@ private static (int bytesConsumed, int value) DecodeInteger(ReadOnlySpan h return QPackTestDecoder.DecodeInteger(headerBlock, prefixMask); } - private static (int bytesConsumed, string value, bool huffmanEncoded) DecodeString(ReadOnlySpan headerBlock) + private static (int bytesConsumed, string value, bool huffmanEncoded, int valueStart) DecodeString(ReadOnlySpan headerBlock) { (int bytesConsumed, int stringLength) = DecodeInteger(headerBlock, 0b01111111); if ((headerBlock[0] & 0b10000000) != 0) @@ -434,12 +434,12 @@ private static (int bytesConsumed, string value, bool huffmanEncoded) DecodeStri byte[] buffer = new byte[stringLength * 2]; int bytesDecoded = HuffmanDecoder.Decode(headerBlock.Slice(bytesConsumed, stringLength), buffer); string value = Encoding.ASCII.GetString(buffer, 0, bytesDecoded); - return (bytesConsumed + stringLength, value, true); + return (bytesConsumed + stringLength, value, true, bytesConsumed); } else { string value = Encoding.ASCII.GetString(headerBlock.Slice(bytesConsumed, stringLength).ToArray()); - return (bytesConsumed + stringLength, value, false); + return (bytesConsumed + stringLength, value, false, bytesConsumed); } } @@ -523,7 +523,7 @@ private static (int bytesConsumed, HttpHeaderData headerData) DecodeLiteralHeade string name; if (index == 0) { - (bytesConsumed, name, _) = DecodeString(headerBlock.Slice(i)); + (bytesConsumed, name, _, _) = DecodeString(headerBlock.Slice(i)); i += bytesConsumed; } else @@ -532,10 +532,11 @@ private static (int bytesConsumed, HttpHeaderData headerData) DecodeLiteralHeade } string value; - (bytesConsumed, value, bool huffmanEncoded) = DecodeString(headerBlock.Slice(i)); + (bytesConsumed, value, bool huffmanEncoded, int valueStart) = DecodeString(headerBlock.Slice(i)); + valueStart += i; i += bytesConsumed; - return (i, new HttpHeaderData(name, value, huffmanEncoded)); + return (i, new HttpHeaderData(name, value, huffmanEncoded, rawValueStart: valueStart)); } private static (int bytesConsumed, HttpHeaderData headerData) DecodeHeader(ReadOnlySpan headerBlock) @@ -680,7 +681,7 @@ public async IAsyncEnumerable ReadRequestHeadersFrames() (int bytesConsumed, HttpHeaderData headerData) = DecodeHeader(data.Span.Slice(i)); byte[] headerRaw = data.Span.Slice(i, bytesConsumed).ToArray(); - headerData = new HttpHeaderData(headerData.Name, headerData.Value, headerData.HuffmanEncoded, headerRaw); + headerData = new HttpHeaderData(headerData.Name, headerData.Value, headerData.HuffmanEncoded, headerRaw, headerData.RawValueStart); requestData.Headers.Add(headerData); i += bytesConsumed; diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 88727d7f190f36..1ea57d8b17eb86 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2312,14 +2312,14 @@ string DecodeHeaderValue(string headerName) { var encoding = Encoding.GetEncoding("ISO-8859-1"); HttpHeaderData headerData = data.GetSingleHeaderData(headerName); - byte[] raw = headerData.Raw; + ReadOnlySpan raw = headerData.Raw.AsSpan().Slice(headerData.RawValueStart); if (headerData.HuffmanEncoded) { byte[] buffer = new byte[raw.Length * 2]; - int length = HuffmanDecoder.Decode(headerData.Raw, buffer); - raw = buffer.AsSpan().Slice(0, length).ToArray(); + int length = HuffmanDecoder.Decode(raw, buffer); + raw = buffer.AsSpan().Slice(0, length); } - return encoding.GetString(raw); + return encoding.GetString(raw.ToArray()); } }); } diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index 3cdacf116f867c..4840a7ed39dfd7 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -812,7 +812,7 @@ public override async Task ReadRequestDataAsync(bool readBody = int offset = line.IndexOf(':'); string name = line.Substring(0, offset); string value = line.Substring(offset + 1).TrimStart(); - requestData.Headers.Add(new HttpHeaderData(name, value, raw: lineBytes)); + requestData.Headers.Add(new HttpHeaderData(name, value, raw: lineBytes, rawValueStart: offset + 1)); } if (requestData.Method != "GET") diff --git a/src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs b/src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs index 7190d8fea76e04..591d0776accc16 100644 --- a/src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs +++ b/src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs @@ -42,7 +42,7 @@ public static (int bytesConsumed, HttpHeaderData) DecodeHeader(ReadOnlySpan