[HTTP] Stricter header value validation

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<HttpRequestData> 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))

src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs

@@ -425,7 +425,7 @@ private static (int bytesConsumed, int value) DecodeInteger(ReadOnlySpan<byte> h
             return QPackTestDecoder.DecodeInteger(headerBlock, prefixMask);
         }
 
-        private static (int bytesConsumed, string value) DecodeString(ReadOnlySpan<byte> headerBlock)
+        private static (int bytesConsumed, string value, bool huffmanEncoded, int valueStart) DecodeString(ReadOnlySpan<byte> 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>
                 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<byte> headerBlock)
@@ -680,7 +681,7 @@ public async IAsyncEnumerable<Frame> 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;

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<NotSupportedException>(() => 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<Exception>(() => client.SendAsync(request));
+                _output.WriteLine(ex.ToString());
+                if (IsWinHttpHandler)
+                {
+                    var fex = Assert.IsType<FormatException>(ex);
+                    Assert.Contains("Latin-1", fex.Message);
+                }
+                else
+                {
+                    var hrex = Assert.IsType<HttpRequestException>(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<byte> 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
+        }
     }
 }

src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs

@@ -812,7 +812,7 @@ public override async Task<HttpRequestData> 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")

src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs

@@ -42,7 +42,7 @@ public static (int bytesConsumed, HttpHeaderData) DecodeHeader(ReadOnlySpan<byte
                     (int valueLength, string value) = DecodeString(buffer.Slice(nameLength), 0b0111_1111);
 
                     int headerLength = nameLength + valueLength;
-                    var header = new HttpHeaderData(s_staticTable[staticIndex].Name, value, raw: buffer.Slice(0, headerLength).ToArray());
+                    var header = new HttpHeaderData(s_staticTable[staticIndex].Name, value, raw: buffer.Slice(0, headerLength).ToArray(), rawValueStart: nameLength);
 
                     return (headerLength, header);
                 }
@@ -52,7 +52,7 @@ public static (int bytesConsumed, HttpHeaderData) DecodeHeader(ReadOnlySpan<byte
                     (int valueLength, string value) = DecodeString(buffer.Slice(nameLength), 0b0111_1111);
 
                     int headerLength = nameLength + valueLength;
-                    var header = new HttpHeaderData(name, value, raw: buffer.Slice(0, headerLength).ToArray());
+                    var header = new HttpHeaderData(name, value, raw: buffer.Slice(0, headerLength).ToArray(), rawValueStart: nameLength);
 
                     return (headerLength, header);
                 }

src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/HttpHeadersFuzzer.cs

@@ -78,15 +78,15 @@ 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'));
                     }
                 }
 
                 foreach ((_, IEnumerable<string> values) in headers)
                 {
                     foreach (string headerValue in values)
                     {
-                        Assert.False(headerValue.ContainsAny('\r', '\n'));
+                        Assert.False(headerValue.ContainsAny('\r', '\n', '\0'));
                     }
                 }
             }

src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx

@@ -135,4 +135,7 @@
   <data name="net_http_unsupported_version" xml:space="preserve">
     <value>Request version value must be one of 1.0, 1.1, 2.0, or 3.0.</value>
   </data>
+  <data name="net_http_invalid_header_value" xml:space="preserve">
+    <value>Request headers must be valid Latin-1 characters.</value>
+  </data>
 </root>

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));
+                    }
                 }
             }
         }

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));
+                }
             }
         }
 

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);
         }