Concatenate multiple cookies with semicolon (#67455)

* Concatenate cookies with semicolon

* Restore tests that run on .NET Framework

* Change Cookie header to Custom

* PR feedback

Author: devsko

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cookies.cs

@@ -168,12 +168,16 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
                 {
                     HttpRequestData requestData = await server.HandleRequestAsync();
 
-                    // Multiple Cookie header values are treated as any other header values and are
-                    // concatenated using ", " as the separator.
+                    // Multiple Cookie header values are concatenated using "; " as the separator.
 
                     string cookieHeaderValue = requestData.GetSingleHeaderValue("Cookie");
 
-                    var cookieValues = cookieHeaderValue.Split(new string[] { ", " }, StringSplitOptions.None);
+#if NETFRAMEWORK
+                    var separator = ", ";
+#else
+                    var separator = "; ";
+#endif
+                    var cookieValues = cookieHeaderValue.Split(new string[] { separator }, StringSplitOptions.None);
                     Assert.Contains("A=1", cookieValues);
                     Assert.Contains("B=2", cookieValues);
                     Assert.Contains("C=3", cookieValues);
@@ -262,32 +266,20 @@ await LoopbackServerFactory.CreateServerAsync(async (server, url) =>
                     HttpRequestData requestData = await serverTask;
                     string cookieHeaderValue = GetCookieValue(requestData);
 
-                    // Multiple Cookie header values are treated as any other header values and are
+#if NETFRAMEWORK
+                    // On .NET Framework multiple Cookie header values are treated as any other header values and are
                     // concatenated using ", " as the separator.  The container cookie is concatenated to
                     // one of these values using the "; " cookie separator.
 
-                    var cookieValues = cookieHeaderValue.Split(new string[] { ", " }, StringSplitOptions.None);
-                    Assert.Equal(2, cookieValues.Count());
-
-                    // Find container cookie and remove it so we can validate the rest of the cookie header values
-                    bool sawContainerCookie = false;
-                    for (int i = 0; i < cookieValues.Length; i++)
-                    {
-                        if (cookieValues[i].Contains(';'))
-                        {
-                            Assert.False(sawContainerCookie);
-
-                            var cookies = cookieValues[i].Split(new string[] { "; " }, StringSplitOptions.None);
-                            Assert.Equal(2, cookies.Count());
-                            Assert.Contains(s_expectedCookieHeaderValue, cookies);
-
-                            sawContainerCookie = true;
-                            cookieValues[i] = cookies.Where(c => c != s_expectedCookieHeaderValue).Single();
-                        }
-                    }
-
+                    var separators = new string[] { "; ", ", " };
+#else
+                    var separators = new string[] { "; " };
+#endif
+                    var cookieValues = cookieHeaderValue.Split(separators, StringSplitOptions.None);
+                    Assert.Contains(s_expectedCookieHeaderValue, cookieValues);
                     Assert.Contains("A=1", cookieValues);
                     Assert.Contains("B=2", cookieValues);
+                    Assert.Equal(3, cookieValues.Count());
                 }
             });
         }

src/libraries/System.Net.Http/src/System.Net.Http.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <WindowsRID>win</WindowsRID>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -83,6 +83,7 @@
     <Compile Include="System\Net\Http\Headers\CacheControlHeaderValue.cs" />
     <Compile Include="System\Net\Http\Headers\ContentDispositionHeaderValue.cs" />
     <Compile Include="System\Net\Http\Headers\ContentRangeHeaderValue.cs" />
+    <Compile Include="System\Net\Http\Headers\CookieHeaderParser.cs" />
     <Compile Include="System\Net\Http\Headers\DateHeaderParser.cs" />
     <Compile Include="System\Net\Http\Headers\EntityTagHeaderValue.cs" />
     <Compile Include="System\Net\Http\Headers\GenericHeaderParser.cs" />

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

@@ -0,0 +1,34 @@
+// 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.CodeAnalysis;
+
+namespace System.Net.Http.Headers
+{
+    internal sealed class CookieHeaderParser : HttpHeaderParser
+    {
+        internal static readonly CookieHeaderParser Parser = new CookieHeaderParser();
+
+        // According to RFC 6265 Section 4.2 multiple cookies have
+        // to be concatenated using "; " as the separator.
+        private CookieHeaderParser()
+            : base(true, "; ")
+        {
+        }
+
+        public override bool TryParseValue(string? value, object? storeValue, ref int index, [NotNullWhen(true)] out object? parsedValue)
+        {
+            // Some headers support empty/null values. This one doesn't.
+            if (string.IsNullOrEmpty(value) || (index == value.Length))
+            {
+                parsedValue = null;
+                return false;
+            }
+
+            parsedValue = value;
+            index = value.Length;
+
+            return true;
+        }
+    }
+}

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

@@ -40,7 +40,7 @@ internal static class KnownHeaders
         public static readonly KnownHeader ContentRange = new KnownHeader("Content-Range", HttpHeaderType.Content | HttpHeaderType.NonTrailing, GenericHeaderParser.ContentRangeParser, null, H2StaticTable.ContentRange);
         public static readonly KnownHeader ContentSecurityPolicy = new KnownHeader("Content-Security-Policy", http3StaticTableIndex: H3StaticTable.ContentSecurityPolicyAllNone);
         public static readonly KnownHeader ContentType = new KnownHeader("Content-Type", HttpHeaderType.Content | HttpHeaderType.NonTrailing, MediaTypeHeaderParser.SingleValueParser, null, H2StaticTable.ContentType, H3StaticTable.ContentTypeApplicationDnsMessage);
-        public static readonly KnownHeader Cookie = new KnownHeader("Cookie", H2StaticTable.Cookie, H3StaticTable.Cookie);
+        public static readonly KnownHeader Cookie = new KnownHeader("Cookie", HttpHeaderType.Custom, CookieHeaderParser.Parser, null, H2StaticTable.Cookie, H3StaticTable.Cookie);
         public static readonly KnownHeader Cookie2 = new KnownHeader("Cookie2");
         public static readonly KnownHeader Date = new KnownHeader("Date", HttpHeaderType.General | HttpHeaderType.NonTrailing, DateHeaderParser.Parser, null, H2StaticTable.Date, H3StaticTable.Date);
         public static readonly KnownHeader ETag = new KnownHeader("ETag", HttpHeaderType.Response, GenericHeaderParser.SingleValueEntityTagParser, null, H2StaticTable.ETag, H3StaticTable.ETag);

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

@@ -427,22 +427,22 @@ await server.HandleRequestAsync(headers: new[]
                 });
         }
 
-        private static readonly (string Name, Encoding ValueEncoding, string[] Values)[] s_nonAsciiHeaders = new[]
+        private static readonly (string Name, Encoding ValueEncoding, string Separator, string[] Values)[] s_nonAsciiHeaders = new[]
         {
-            ("foo",             Encoding.ASCII,     new[] { "bar" }),
-            ("header-0",        Encoding.UTF8,      new[] { "\uD83D\uDE03", "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A" }),
-            ("Cache-Control",   Encoding.UTF8,      new[] { "no-cache" }),
-            ("header-1",        Encoding.UTF8,      new[] { "\uD83D\uDE03" }),
-            ("Some-Header1",    Encoding.Latin1,    new[] { "\uD83D\uDE03", "UTF8-best-fit-to-latin1" }),
-            ("Some-Header2",    Encoding.Latin1,    new[] { "\u00FF", "\u00C4nd", "Ascii\u00A9" }),
-            ("Some-Header3",    Encoding.ASCII,     new[] { "\u00FF", "\u00C4nd", "Ascii\u00A9", "Latin1-best-fit-to-ascii" }),
-            ("header-2",        Encoding.UTF8,      new[] { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A" }),
-            ("header-3",        Encoding.UTF8,      new[] { "\uFFFD" }),
-            ("header-4",        Encoding.UTF8,      new[] { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A", "\uD83D\uDE03" }),
-            ("Cookie",          Encoding.UTF8,      new[] { "Cookies", "\uD83C\uDF6A", "everywhere" }),
-            ("Set-Cookie",      Encoding.UTF8,      new[] { "\uD83C\uDDF8\uD83C\uDDEE" }),
-            ("header-5",        Encoding.UTF8,      new[] { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A", "foo", "\uD83D\uDE03", "bar" }),
-            ("bar",             Encoding.UTF8,      new[] { "foo" })
+            ("foo",             Encoding.ASCII,     ", ", new[] { "bar" }),
+            ("header-0",        Encoding.UTF8,      ", ", new[] { "\uD83D\uDE03", "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A" }),
+            ("Cache-Control",   Encoding.UTF8,      ", ", new[] { "no-cache" }),
+            ("header-1",        Encoding.UTF8,      ", ", new[] { "\uD83D\uDE03" }),
+            ("Some-Header1",    Encoding.Latin1,    ", ", new[] { "\uD83D\uDE03", "UTF8-best-fit-to-latin1" }),
+            ("Some-Header2",    Encoding.Latin1,    ", ", new[] { "\u00FF", "\u00C4nd", "Ascii\u00A9" }),
+            ("Some-Header3",    Encoding.ASCII,     ", ", new[] { "\u00FF", "\u00C4nd", "Ascii\u00A9", "Latin1-best-fit-to-ascii" }),
+            ("header-2",        Encoding.UTF8,      ", ", new[] { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A" }),
+            ("header-3",        Encoding.UTF8,      ", ", new[] { "\uFFFD" }),
+            ("header-4",        Encoding.UTF8,      ", ", new[] { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A", "\uD83D\uDE03" }),
+            ("Cookie",          Encoding.UTF8,      "; ", new[] { "Cookies", "\uD83C\uDF6A", "everywhere" }),
+            ("Set-Cookie",      Encoding.UTF8,      ", ", new[] { "\uD83C\uDDF8\uD83C\uDDEE" }),
+            ("header-5",        Encoding.UTF8,      ", ", new[] { "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A", "foo", "\uD83D\uDE03", "bar" }),
+            ("bar",             Encoding.UTF8,      ", ", new[] { "foo" })
         };
 
         [Fact]
@@ -457,7 +457,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
                         Version = UseVersion
                     };
 
-                    foreach ((string name, _, string[] values) in s_nonAsciiHeaders)
+                    foreach ((string name, _, _, string[] values) in s_nonAsciiHeaders)
                     {
                         requestMessage.Headers.Add(name, values);
                     }
@@ -479,7 +479,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
 
                     await client.SendAsync(TestAsync, requestMessage);
 
-                    foreach ((string name, _, _) in s_nonAsciiHeaders)
+                    foreach ((string name, _, _, _) in s_nonAsciiHeaders)
                     {
                         Assert.Contains(name, seenHeaderNames);
                     }
@@ -491,9 +491,9 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
                     Assert.All(requestData.Headers,
                         h => Assert.False(h.HuffmanEncoded, "Expose raw decoded bytes once HuffmanEncoding is supported"));
 
-                    foreach ((string name, Encoding valueEncoding, string[] values) in s_nonAsciiHeaders)
+                    foreach ((string name, Encoding valueEncoding, string separator, string[] values) in s_nonAsciiHeaders)
                     {
-                        byte[] valueBytes = valueEncoding.GetBytes(string.Join(", ", values));
+                        byte[] valueBytes = valueEncoding.GetBytes(string.Join(separator, values));
                         Assert.Single(requestData.Headers,
                             h => h.Name.Equals(name, StringComparison.OrdinalIgnoreCase) && h.Raw.AsSpan().IndexOf(valueBytes) != -1);
                     }
@@ -536,20 +536,20 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
 
                     using HttpResponseMessage response = await client.SendAsync(TestAsync, requestMessage);
 
-                    foreach ((string name, Encoding valueEncoding, string[] values) in s_nonAsciiHeaders)
+                    foreach ((string name, Encoding valueEncoding, string separator, string[] values) in s_nonAsciiHeaders)
                     {
                         Assert.Contains(name, seenHeaderNames);
                         IEnumerable<string> receivedValues = Assert.Single(response.Headers, h => h.Key.Equals(name, StringComparison.OrdinalIgnoreCase)).Value;
                         string value = Assert.Single(receivedValues);
 
-                        string expected = valueEncoding.GetString(valueEncoding.GetBytes(string.Join(", ", values)));
+                        string expected = valueEncoding.GetString(valueEncoding.GetBytes(string.Join(separator, values)));
                         Assert.Equal(expected, value, StringComparer.OrdinalIgnoreCase);
                     }
                 },
                 async server =>
                 {
                     List<HttpHeaderData> headerData = s_nonAsciiHeaders
-                        .Select(h => new HttpHeaderData(h.Name, string.Join(", ", h.Values), valueEncoding: h.ValueEncoding))
+                        .Select(h => new HttpHeaderData(h.Name, string.Join(h.Separator, h.Values), valueEncoding: h.ValueEncoding))
                         .ToList();
 
                     await server.HandleRequestAsync(headers: headerData);

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

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <StringResourcesPath>../../src/Resources/Strings.resx</StringResourcesPath>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -99,6 +99,8 @@
              Link="ProductionCode\System\Net\Http\Headers\ContentDispositionHeaderValue.cs" />
     <Compile Include="..\..\src\System\Net\Http\Headers\ContentRangeHeaderValue.cs"
              Link="ProductionCode\System\Net\Http\Headers\ContentRangeHeaderValue.cs" />
+    <Compile Include="..\..\src\System\Net\Http\Headers\CookieHeaderParser.cs"
+             Link="ProductionCode\System\Net\Http\Headers\CookieHeaderParser.cs" />
     <Compile Include="..\..\src\System\Net\Http\Headers\DateHeaderParser.cs"
              Link="ProductionCode\System\Net\Http\Headers\DateHeaderParser.cs" />
     <Compile Include="..\..\src\System\Net\Http\Headers\EntityTagHeaderValue.cs"