Fix various HTTP header parsing issues

src/libraries/Fuzzing/.gitignore

@@ -3,3 +3,5 @@ publish
 deployment
 inputs
 crash-*
+timeout-*
+inputs-*

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

@@ -73,7 +73,22 @@ static void Test(HttpHeaders headers, string name, string value)
                 }
                 catch (FormatException) { }
 
-                foreach (var _ in headers) { }
+                // If values were added with validation, they should not contain CR or LF characters.
+                foreach ((_, HeaderStringValues values) in headers.NonValidated)
+                {
+                    foreach (string headerValue in values)
+                    {
+                        Assert.False(headerValue.ContainsAny('\r', '\n'));
+                    }
+                }
+
+                foreach ((_, IEnumerable<string> values) in headers)
+                {
+                    foreach (string headerValue in values)
+                    {
+                        Assert.False(headerValue.ContainsAny('\r', '\n'));
+                    }
+                }
             }
         }
     }

src/libraries/Fuzzing/README.md

@@ -24,7 +24,7 @@ Build the runtime with the desired configuration if you haven't already:
 ```
 
 > [!TIP]
-> The `-rc release` configuration here builds runime in `Release` and libraries in `Debug` mode.
+> The `-rc release` configuration here builds runtime in `Release` and libraries in `Debug` mode.
 > Automated fuzzing runs use a `Checked` runtime + `Debug` libraries configuration by default.
 > You can use any configuration locally, but `Checked` is recommended when testing changes in `System.Private.CoreLib`.
 
@@ -44,10 +44,10 @@ dotnet build
 ```
 
 Run `run.bat`, which will create separate directories for each fuzzing target, instrument the relevant assemblies, and generate a helper script for running them locally.
-When iterating on changes, remember to rebuild the project again: `dotnet build; .\run.bat`.
+When iterating on changes, remember to rebuild the project again.
 
 ```cmd
-run.bat
+dotnet build; .\run.bat
 ```
 
 Start fuzzing by running the `local-run.bat` script in the folder of the fuzzer you are interested in.

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

@@ -259,14 +259,14 @@ private static bool TryReadUnknownPercentEncodedAlpnProtocolName(ReadOnlySpan<ch
                     builder.Append(value.Slice(0, idx));
                 }
 
-                if ((value.Length - idx) < 3 || !TryReadAlpnHexDigit(value[1], out int hi) || !TryReadAlpnHexDigit(value[2], out int lo))
+                if ((value.Length - idx) < 3 || !TryReadAlpnHexDigit(value[idx + 1], out int hi) || !TryReadAlpnHexDigit(value[idx + 2], out int lo))
                 {
                     builder.Dispose();
                     result = null;
                     return false;
                 }
 
-                builder.Append((char)((hi << 8) | lo));
+                builder.Append((char)((hi << 4) | lo));
 
                 value = value.Slice(idx + 3);
                 idx = value.IndexOf('%');
@@ -279,7 +279,7 @@ private static bool TryReadUnknownPercentEncodedAlpnProtocolName(ReadOnlySpan<ch
             }
 
             result = builder.ToString();
-            return true;
+            return !HttpRuleParser.ContainsNewLine(result);
         }
 
         /// <summary>

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

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 
 namespace System.Net.Http.Headers
@@ -18,8 +19,10 @@ private CookieHeaderParser()
 
         public override bool TryParseValue(string? value, object? storeValue, ref int index, [NotNullWhen(true)] out object? parsedValue)
         {
+            Debug.Assert(index == 0);
+
             // Some headers support empty/null values. This one doesn't.
-            if (string.IsNullOrEmpty(value) || (index == value.Length))
+            if (string.IsNullOrEmpty(value) || HttpRuleParser.ContainsNewLine(value))
             {
                 parsedValue = null;
                 return false;

src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs

@@ -20,6 +20,10 @@ internal static class HttpRuleParser
         private static readonly SearchValues<char> s_hostDelimiterChars =
             SearchValues.Create("/ \t\r,");
 
+        // Characters such as '?' or '#' are interpreted as an end of the host part of the URI, so they will not be validated in the same way.
+        private static readonly SearchValues<char> s_disallowedHostChars =
+            SearchValues.Create("/\\?#@");
+
         private const int MaxNestedCount = 5;
 
         internal const char CR = (char)13;
@@ -193,8 +197,8 @@ 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. If not, it's an invalid value.
-            if ((startIndex + 2 > input.Length) || (input[startIndex + 1] > 127))
+            // 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'))
             {
                 return HttpParseResult.InvalidFormat;
             }
@@ -303,8 +307,19 @@ private static HttpParseResult GetExpressionLength(string input, int startIndex,
 
         private static bool IsValidHostName(ReadOnlySpan<char> host)
         {
-            // Also add user info (u@) to make sure 'host' doesn't include user info.
-            return Uri.TryCreate($"http://u@{host}/", UriKind.Absolute, out _);
+            if (host.ContainsAny(s_disallowedHostChars))
+            {
+                return false;
+            }
+
+            // Using a trailing slash as Uri ignores trailing whitespace otherwise.
+            if (!Uri.TryCreate($"http://{host}/", UriKind.Absolute, out _))
+            {
+                return false;
+            }
+
+            Debug.Assert(!ContainsNewLine(host.ToString()));
+            return true;
         }
     }
 }

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

@@ -9,14 +9,18 @@ namespace System.Net.Http.Tests
 {
     public class AltSvcHeaderParserTest
     {
-        [Fact]
-        public void TryParse_InvalidValueString_ReturnsFalse()
+        [Theory]
+        [InlineData("a=")]
+        [InlineData("%aa=\":123\"")] // Only uppercase hex is allowed
+        [InlineData("%0A=\":123\"")] // Encoded new line
+        public void TryParse_InvalidValueString_ReturnsFalse(string value)
         {
             HttpHeaderParser parser = AltSvcHeaderParser.Parser;
-            string invalidInput = "a=";
             int startIndex = 0;
 
-            Assert.False(parser.TryParseValue(invalidInput, null, ref startIndex, out var _));
+            Assert.False(parser.TryParseValue(value, null, ref startIndex, out object? parsedValue));
+            Assert.Equal(0, startIndex);
+            Assert.Null(parsedValue);
         }
 
         [Theory]
@@ -117,6 +121,15 @@ public static IEnumerable<object[]> SuccessfulParseData()
                     AltSvcHeaderValue.Clear
                 }
             };
+
+            // Encoded protocol name
+            yield return new object[]
+            {
+                "AB%43%44%EF=\":123\"", new[]
+                {
+                    new AltSvcHeaderValue("ABCD\u00EF", host: null, 123, TimeSpan.FromTicks(AltSvcHeaderParser.DefaultMaxAgeTicks), persist: false)
+                }
+            };
         }
     }
 }

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

@@ -0,0 +1,36 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Net.Http.Headers;
+
+using Xunit;
+
+namespace System.Net.Http.Tests
+{
+    public class CookieHeaderParserTest
+    {
+        [Fact]
+        public void TryParse_ValidValueString_ReturnsTrue()
+        {
+            HttpHeaderParser parser = CookieHeaderParser.Parser;
+            string validInput = "Hello World";
+            int startIndex = 0;
+
+            Assert.True(parser.TryParseValue(validInput, null, ref startIndex, out object? parsedValue));
+            Assert.Equal(validInput.Length, startIndex);
+            Assert.Same(validInput, parsedValue);
+        }
+
+        [Fact]
+        public void TryParse_InvalidValueString_ReturnsFalse()
+        {
+            HttpHeaderParser parser = CookieHeaderParser.Parser;
+            string invalidInput = "Hello\r\nWorld";
+            int startIndex = 0;
+
+            Assert.False(parser.TryParseValue(invalidInput, null, ref startIndex, out object? parsedValue));
+            Assert.Equal(0, startIndex);
+            Assert.Null(parsedValue);
+        }
+    }
+}

src/libraries/System.Net.Http/tests/UnitTests/Headers/GenericHeaderParserTest/HostParserTest.cs

@@ -51,9 +51,15 @@ public void TryParse_SetOfInvalidValueStrings_ReturnsFalse()
             CheckInvalidParsedValue("host host", 0);
             CheckInvalidParsedValue("host,", 0);
             CheckInvalidParsedValue("host ,", 0);
+            CheckInvalidParsedValue("host/", 0);
             CheckInvalidParsedValue("/", 0);
             CheckInvalidParsedValue(" , ", 0);
             CheckInvalidParsedValue(" host\r\n ", 0);
+            CheckInvalidParsedValue(" host\r\n ", 1);
+            CheckInvalidParsedValue("host#\r\n", 0);
+            CheckInvalidParsedValue(" host?\r\n", 0);
+            CheckInvalidParsedValue("foo:bar@host", 0);
+            CheckInvalidParsedValue("host\n", 0);
         }
 
         #region Helper methods

src/libraries/System.Net.Http/tests/UnitTests/HttpRuleParserTest.cs

@@ -164,6 +164,10 @@ public void GetQuotedPairLength_SetOfInvalidQuotedPairs_AllConsideredInvalid()
             // only ASCII chars allowed in quoted-pair
             AssertGetQuotedPairLength("\\\u00FC", 0, 0, HttpParseResult.InvalidFormat);
 
+            // New lines are not allowed
+            AssertGetQuotedPairLength("\\\r", 0, 0, HttpParseResult.InvalidFormat);
+            AssertGetQuotedPairLength("\\\n", 0, 0, HttpParseResult.InvalidFormat);
+
             // a quoted-pair needs 1 char after '\'
             AssertGetQuotedPairLength("\\", 0, 0, HttpParseResult.InvalidFormat);
         }

src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj

@@ -267,6 +267,7 @@
     <Compile Include="Headers\ContentDispositionHeaderValueTest.cs" />
     <Compile Include="Headers\ContentRangeHeaderValueTest.cs" />
     <Compile Include="Headers\DateHeaderParserTest.cs" />
+    <Compile Include="Headers\CookieHeaderParserTest.cs" />
     <Compile Include="Headers\EntityTagHeaderParserTest.cs" />
     <Compile Include="Headers\EntityTagHeaderValueTest.cs" />
     <Compile Include="Headers\GenericHeaderParserTest\AuthenticationParserTest.cs" />