Replace dangerous characters in response headers

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

@@ -148,7 +148,7 @@ public abstract class GenericLoopbackConnection : IAsyncDisposable
         /// If isFinal is false, the body is not completed and you can call SendResponseBodyAsync to send more.</summary>
         public abstract Task SendResponseAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null, string content = "", bool isFinal = true);
         /// <summary>Sends response headers.</summary>
-        public abstract Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null);
+        public abstract Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null, bool isTrailingHeader = false);
         /// <summary>Sends valid but incomplete headers. Once called, there is no way to continue the response past this point.</summary>
         public abstract Task SendPartialResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null);
         /// <summary>Sends Response body after SendResponse was called with isFinal: false.</summary>

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

@@ -959,10 +959,10 @@ public override Task SendResponseAsync(HttpStatusCode statusCode = HttpStatusCod
             return SendResponseAsync(statusCode, headers, content, isFinal, requestId: 0);
         }
 
-        public override Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null)
+        public override Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null, bool isTrailingHeader = false)
         {
             int streamId = _lastStreamId;
-            return SendResponseHeadersAsync(streamId, endStream: false, statusCode, isTrailingHeader: false, endHeaders: true, headers);
+            return SendResponseHeadersAsync(streamId, endStream: isTrailingHeader, statusCode, isTrailingHeader: isTrailingHeader, endHeaders: true, headers);
         }
 
         public override Task SendPartialResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null)

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

@@ -229,9 +229,15 @@ public override async Task SendResponseBodyAsync(byte[] content, bool isFinal =
             }
         }
 
-        public override Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null)
+        public override async Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null, bool isTrailingHeader = false)
         {
-            return _currentStream.SendResponseHeadersAsync(statusCode, headers);
+            await _currentStream.SendResponseHeadersAsync(statusCode: isTrailingHeader ? null : statusCode, headers);
+
+            if (isTrailingHeader)
+            {
+                _currentStream.Stream.CompleteWrites();
+                await DisposeCurrentStream().ConfigureAwait(false);
+            }
         }
 
         public override Task SendPartialResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null)

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

@@ -979,9 +979,21 @@ private string GetResponseHeaderString(HttpStatusCode statusCode, IList<HttpHead
                 return headerString;
             }
 
-            public override async Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null)
+            private string GetTrailerString(IList<HttpHeaderData> trailers)
             {
-                string headerString = GetResponseHeaderString(statusCode, headers);
+                StringBuilder bld = new StringBuilder();
+                bld.Append("0\r\n");
+                foreach (HttpHeaderData headerData in trailers)
+                {
+                    bld.Append($"{headerData.Name}: {headerData.Value}\r\n");
+                }
+                bld.Append("\r\n");
+                return bld.ToString();
+            }
+
+            public override async Task SendResponseHeadersAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null, bool isTrailingHeader = false)
+            {
+                string headerString = isTrailingHeader ? GetTrailerString(headers) : GetResponseHeaderString(statusCode, headers);
                 await SendResponseAsync(headerString).ConfigureAwait(false);
             }
 

src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs

@@ -15,6 +15,8 @@ namespace System.Net.Http.Headers
     // Use HeaderDescriptor.TryGet to resolve an arbitrary header name to a HeaderDescriptor.
     internal readonly struct HeaderDescriptor : IEquatable<HeaderDescriptor>
     {
+        private static readonly SearchValues<byte> s_dangerousCharacterBytes = SearchValues.Create((byte)'\0', (byte)'\r', (byte)'\n');
+
         /// <summary>
         /// Either a <see cref="KnownHeader"/> or <see cref="string"/>.
         /// </summary>
@@ -169,7 +171,16 @@ public string GetHeaderValue(ReadOnlySpan<byte> headerValue, Encoding? valueEnco
                 }
             }
 
-            return (valueEncoding ?? HttpRuleParser.DefaultHttpEncoding).GetString(headerValue);
+            string value = (valueEncoding ?? HttpRuleParser.DefaultHttpEncoding).GetString(headerValue);
+            if (headerValue.ContainsAny(s_dangerousCharacterBytes))
+            {
+                // Depending on the encoding, 'value' may contain a dangerous character.
+                // We are replacing them with SP to conform with https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5.
+                // This is a low-occurrence corner case, so we don't care about the cost of Replace() and the extra allocations.
+                value = value.Replace('\0', ' ').Replace('\r', ' ').Replace('\n', ' ');
+            }
+
+            return value;
         }
 
         internal static string? GetKnownContentType(ReadOnlySpan<byte> contentTypeValue)

src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs

@@ -5,11 +5,10 @@
 using System.IO;
 using System.Linq;
 using System.Net.Http.Headers;
-using System.Net.Quic;
 using System.Net.Test.Common;
 using System.Text;
 using System.Threading.Tasks;
-
+using Microsoft.DotNet.XUnitExtensions;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -160,7 +159,8 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
                     // Client should abort at some point so this is going to throw.
                     HttpRequestData requestData = await server.HandleRequestAsync(HttpStatusCode.OK).ConfigureAwait(false);
                 }
-                catch (Exception) { };
+                catch (Exception) { }
+                ;
             });
         }
 
@@ -595,5 +595,59 @@ await connection.SendResponseAsync(HttpStatusCode.OK,
                 });
             });
         }
+
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
+        [InlineData(false, "test\nxwow\nmore\n", false)]
+        [InlineData(false, "test\rwow\rmore\r\n", false)]
+        [InlineData(true, "one\0two\0three\0", false)]
+        [InlineData(false, "test\nxwow\nmore\n", true)]
+        [InlineData(false, "test\rwow\rmore\r\n", true)]
+        [InlineData(true, "one\0two\0three\0", true)]
+        public async Task SendAsync_InvalidCharactersInResponseHeader_ReplacedWithSpaces(bool testHttp11, string value, bool testTrailers)
+        {
+            if (!testHttp11 && UseVersion == HttpVersion.Version11)
+            {
+                throw new SkipTestException("This case is not valid for HTTP 1.1");
+            }
+
+            string expectedValue = value.Replace('\r', ' ').Replace('\n', ' ').Replace('\0', ' ');
+            await LoopbackServerFactory.CreateClientAndServerAsync(
+                async uri =>
+                {
+                    using HttpClient client = CreateHttpClient();
+
+                    using HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, uri)
+                    {
+                        Version = UseVersion,
+                        VersionPolicy = HttpVersionPolicy.RequestVersionExact
+                    };
+
+                    using HttpResponseMessage response = await client.SendAsync(request);
+                    HttpResponseHeaders headerCollection = testTrailers ? response.TrailingHeaders : response.Headers;
+                    Assert.Equal(expectedValue, headerCollection.GetValues("test").Single());
+                },
+                async server =>
+                {
+                    List<HttpHeaderData>? headers = testTrailers ? null : [new HttpHeaderData("test", value)];
+                    List<HttpHeaderData>? trailers = testTrailers ? [new HttpHeaderData("test", value)] : null;
+                    string content = "hello";
+
+                    if (testTrailers && UseVersion == HttpVersion.Version11)
+                    {
+                        headers = [new HttpHeaderData("Transfer-Encoding", "chunked")];
+                        content = $"{content.Length:X}\r\n{content}\r\n";
+                    }
+
+                    await server.AcceptConnectionAsync(async connection =>
+                    {
+                        await connection.ReadRequestDataAsync();
+                        await connection.SendResponseAsync(headers: headers, content: content, isFinal: trailers is null);
+                        if (trailers is { })
+                        {
+                            await connection.SendResponseHeadersAsync(headers: trailers, isTrailingHeader: true);
+                        }
+                    });
+                });
+        }
     }
 }

src/libraries/System.Net.Http/tests/UnitTests/Headers/HeaderEncodingTest.cs

@@ -1,37 +1,85 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Linq;
 using System.Net.Http.Headers;
 using System.Text;
+using Microsoft.DotNet.XUnitExtensions;
 using Xunit;
 
 namespace System.Net.Http.Tests
 {
     public class HeaderEncodingTest
     {
-        [Theory]
-        [InlineData("")]
-        [InlineData("foo")]
-        [InlineData("\uD83D\uDE03")]
-        [InlineData("\0")]
-        [InlineData("\x01")]
-        [InlineData("\xFF")]
-        [InlineData("\uFFFF")]
-        [InlineData("\uFFFD")]
-        [InlineData("\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A")]
-        public void RoundTripsUtf8(string input)
+        public static readonly TheoryData<string, string?> RoundTrips_Data = new TheoryData<string, string?>
         {
-            byte[] encoded = Encoding.UTF8.GetBytes(input);
+            { "", null },
+            { "foo", null },
+            { "\uD83D\uDE03", null },
+            { "\x01", null },
+            { "\xFF", null },
+            { "\uFFFF", null },
+            { "\uFFFD", null },
+            { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A", null },
+            { "\0", null },
+            { "abc\056", null },
+            { "abc\rq", null },
+            { "abc\r\n", null },
+            { "abc\rfoo", null },
+
+            { "", "UTF-8" },
+            { "foo", "UTF-8" },
+            { "\uD83D\uDE03", "UTF-8" },
+            { "\x01", "UTF-8" },
+            { "\xFF", "UTF-8" },
+            { "\uFFFF", "UTF-8" },
+            { "\uFFFD", "UTF-8" },
+            { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A", "UTF-8" },
+            { "\0", "UTF-8" },
+            { "abc\056", "UTF-8" },
+            { "abc\rq", "UTF-8" },
+            { "abc\r\n", "UTF-8" },
+            { "abc\rfoo", "UTF-8" },
+
+            // Fixed, multi byte encodings are discouraged, but we want them to function at HeaderDescriptor level.
+            { "", "UTF-16" },
+            { "foo", "UTF-16" },
+            { "\uD83D\uDE03", "UTF-16" },
+            { "\x01", "UTF-16" },
+            { "\xFF", "UTF-16" },
+            { "\uFFFF", "UTF-16" },
+            { "\uFFFD", "UTF-16" },
+            { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A", "UTF-16" },
+            { "\0", "UTF-16" },
+            { "abc\056", "UTF-16" },
+            { "abc\rq", "UTF-16" },
+            { "abc\r\n", "UTF-16" },
+            { "abc\rfoo", "UTF-16" },
+        };
+
+        [ConditionalTheory]
+        [MemberData(nameof(RoundTrips_Data))]
+        public void GetHeaderValue_RoundTrips_ReplacesDangerousCharacters(string input, string? encodingName)
+        {
+            bool isUnicode = input.Any(c => c > 255);
+            if (isUnicode && encodingName == null)
+            {
+                throw new SkipTestException("The test case is invalid for the default encoding.");
+            }
+
+            Encoding encoding = encodingName == null ? null : Encoding.GetEncoding(encodingName);
+            byte[] encoded = (encoding ?? Encoding.Latin1).GetBytes(input);
+            string expectedValue = input.Replace('\0', ' ').Replace('\r', ' ').Replace('\n', ' ');
 
             Assert.True(HeaderDescriptor.TryGet("custom-header", out HeaderDescriptor descriptor));
             Assert.Null(descriptor.KnownHeader);
-            string roundtrip = descriptor.GetHeaderValue(encoded, Encoding.UTF8);
-            Assert.Equal(input, roundtrip);
+            string roundtrip = descriptor.GetHeaderValue(encoded, encoding);
+            Assert.Equal(expectedValue, roundtrip);
 
             Assert.True(HeaderDescriptor.TryGet("Cache-Control", out descriptor));
             Assert.NotNull(descriptor.KnownHeader);
-            roundtrip = descriptor.GetHeaderValue(encoded, Encoding.UTF8);
-            Assert.Equal(input, roundtrip);
+            roundtrip = descriptor.GetHeaderValue(encoded, encoding);
+            Assert.Equal(expectedValue, roundtrip);
         }
     }
 }