diff --git a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs index 66c64e3e8a578f..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; } @@ -258,6 +260,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/Http2LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index e607c42aa48ba8..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) 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) 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, bytesConsumed); } else { string value = Encoding.ASCII.GetString(headerBlock.Slice(bytesConsumed, stringLength).ToArray()); - return (bytesConsumed + stringLength, value); + 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) = DecodeString(headerBlock.Slice(i)); + (bytesConsumed, value, bool huffmanEncoded, int valueStart) = DecodeString(headerBlock.Slice(i)); + valueStart += i; i += bytesConsumed; - return (i, new HttpHeaderData(name, value)); + 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 582e4b8a1fd434..1ea57d8b17eb86 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2150,5 +2150,185 @@ public async Task SendAsync_InvalidRequestUri_Throws() request = new HttpRequestMessage(HttpMethod.Get, new Uri("foo://foo.bar")); await Assert.ThrowsAsync(() => invoker.SendAsync(request, CancellationToken.None)); } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] + [InlineData('\r', HeaderType.Request)] + [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 + 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.ThrowsAnyAsync(() => client.SendAsync(request)); + _output.WriteLine(ex.ToString()); + if (IsWinHttpHandler) + { + var fex = Assert.IsType(ex); + Assert.Contains("Latin-1", fex.Message); + } + else + { + var hrex = Assert.IsType(ex); + var message = UseVersion == HttpVersion30 ? hrex.InnerException.Message : hrex.Message; + Assert.Contains("ASCII", message); + } + } + } + + [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.Content)] + [InlineData('\u0007', HeaderType.Content)] + [InlineData('\u007F', HeaderType.Content)] + [InlineData('\u00A0', HeaderType.Content)] + [InlineData('\u00A9', HeaderType.Content)] + [InlineData('\u00FF', 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)] + 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. + } + var headerValue = $"HeaderValue{safeChar}WithSafeChar"; + 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); + break; + case HeaderType.Content: + request.Content = new StringContent("test content"); + 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", headerValue)); + break; + } + + using (HttpResponseMessage response = await client.SendAsync(request)) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + }, async server => + { + var data = await server.AcceptConnectionSendResponseAndCloseAsync(); + switch (headerType) + { + case HeaderType.Request: + { + var headerLine = DecodeHeaderValue("Custom-Header"); + var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); + Assert.Equal(headerValue, receivedHeaderValue); + break; + } + case HeaderType.Content: + { + var headerLine = DecodeHeaderValue("Custom-Content-Header"); + var receivedHeaderValue = headerLine.Substring(headerLine.IndexOf("HeaderValue")); + Assert.Equal(headerValue, receivedHeaderValue); + break; + } + case HeaderType.Cookie: + { + 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); + ReadOnlySpan raw = headerData.Raw.AsSpan().Slice(headerData.RawValueStart); + if (headerData.HuffmanEncoded) + { + byte[] buffer = new byte[raw.Length * 2]; + int length = HuffmanDecoder.Decode(raw, buffer); + raw = buffer.AsSpan().Slice(0, length); + } + return encoding.GetString(raw.ToArray()); + } + }); + } + + public enum HeaderType + { + Request, + Content, + Cookie + } } } 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 Request version value must be one of 1.0, 1.1, 2.0, or 3.0. + + 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 fd61e8fab619fc..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 @@ -769,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)); + } } } 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..2505480230e59e 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); } diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 8b1e61d8cda685..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. @@ -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/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/AuthenticationHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs index 930cb8f8408587..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.CheckContainsNewLine(parameter); + HttpHeaders.CheckContainsNewLineOrNull(parameter); _scheme = scheme; _parameter = parameter; } @@ -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/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/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..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 @@ -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,8 +1024,8 @@ 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. - CheckContainsNewLine(value); + // newline or \0 characters. We add the values as "parsed value". Note that we allow empty values. + CheckContainsNewLineOrNull(value); AddParsedValue(info, value ?? string.Empty); return; } @@ -1127,16 +1127,16 @@ internal bool TryGetHeaderDescriptor(string name, out HeaderDescriptor descripto return false; } - internal static void CheckContainsNewLine(string? value) + internal static void CheckContainsNewLineOrNull(string? value) { if (value == null) { return; } - if (HttpRuleParser.ContainsNewLine(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..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; } - CheckContainsNewLine(value); + CheckContainsNewLineOrNull(value); SetOrRemoveParsedValue(KnownHeaders.From.Descriptor, value); } @@ -167,7 +167,7 @@ public string? Protocol get => _specialCollectionsSlots is null ? null : (string?)_specialCollectionsSlots[ProtocolSlot]; set { - CheckContainsNewLine(value); + CheckContainsNewLineOrNull(value); _specialCollectionsSlots ??= new object[NumCollectionsSlots]; _specialCollectionsSlots[ProtocolSlot] = value; } 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..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 @@ -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) { @@ -198,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; } @@ -250,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; } @@ -318,7 +320,7 @@ private static bool IsValidHostName(ReadOnlySpan host) return false; } - Debug.Assert(!ContainsNewLine(host.ToString())); + Debug.Assert(!ContainsNewLineOrNull(host.ToString())); return true; } } 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"; 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 ?? "")); 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