Allow cookies with spaces in the name (dotnet/corefx#36566)

* Allow cookies with spaces in the name

* Apply suggestions from code review

Co-Authored-By: PaitoAnderson <pj.paito@gmail.com>
* Added supporting HttpClient functional cookie test

* Updated UWP cookie class and added additional tests

* Prevent cookies with spaces at the start or end

* Forgot UWP

* Fix could allocate on trim

Co-Authored-By: Ben Adams <thundercat@illyriad.co.uk>
* Better cookie space start/end check

Co-Authored-By: Ben Adams <thundercat@illyriad.co.uk>
* Bring checks into line

Co-Authored-By: Ben Adams <thundercat@illyriad.co.uk>
* Skip test on .net framework

* Skip additional tests on .net framework

* Fix typo

Commit migrated from https://github.com/dotnet/corefx/commit/8a22438a1af870cdd04eaf798b155b8a1ab04cb4

diff --git a/src/libraries/System.Net.Http/src/uap/System/Net/cookie.cs b/src/libraries/System.Net.Http/src/uap/System/Net/cookie.cs
index 5fe6ce5..250e47a 100644 (file)
--- a/src/libraries/System.Net.Http/src/uap/System/Net/cookie.cs
+++ b/src/libraries/System.Net.Http/src/uap/System/Net/cookie.cs
@@ -65,7 +65,8 @@ namespace System.Net
         internal const string SpecialAttributeLiteral = "$";
 
         internal static readonly char[] PortSplitDelimiters = new char[] { ' ', ',', '\"' };
-        internal static readonly char[] Reserved2Name = new char[] { ' ', '\t', '\r', '\n', '=', ';', ',' };
+        // 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
+        internal static readonly char[] Reserved2Name = new char[] { '\t', '\r', '\n', '=', ';', ',' };
         internal static readonly char[] Reserved2Value = new char[] { ';', ',' };
 
         // fields
@@ -286,7 +287,7 @@ namespace System.Net
 
         internal bool InternalSetName(string value)
         {
-            if (string.IsNullOrEmpty(value) || value[0] == '$' || value.IndexOfAny(Reserved2Name) != -1)
+            if (string.IsNullOrEmpty(value) || value[0] == '$' || value.IndexOfAny(Reserved2Name) != -1 ||  value[0] == ' ' || value[value.Length - 1] == ' ')
             {
                 m_name = string.Empty;
                 return false;
@@ -399,7 +400,7 @@ namespace System.Net
             }
 
             //Check the name
-            if (m_name == null || m_name.Length == 0 || m_name[0] == '$' || m_name.IndexOfAny(Reserved2Name) != -1)
+            if (string.IsNullOrEmpty(m_name) || m_name[0] == '$' || m_name.IndexOfAny(Reserved2Name) != -1 ||  m_name[0] == ' ' || m_name[m_name.Length - 1] == ' ')
             {
                 if (isThrow)
                 {

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Cookies.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Cookies.cs
index e471ab7..ed7822a 100644 (file)
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Cookies.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Cookies.cs
@@ -628,6 +628,10 @@ namespace System.Net.Http.Functional.Tests
                 yield return new object[] { "Hello", "World", useCookies };
                 yield return new object[] { "foo", "bar", useCookies };
 
+                if (!PlatformDetection.IsFullFramework) {
+                    yield return new object[] { "Hello World", "value", useCookies };
+                }
+
                 yield return new object[] { ".AspNetCore.Session", "RAExEmXpoCbueP_QYM", useCookies };
 
                 yield return new object[]

diff --git a/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.Cookies.cs b/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.Cookies.cs
index a618911..0eec139 100644 (file)
--- a/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.Cookies.cs
+++ b/src/libraries/System.Net.HttpListener/tests/HttpListenerResponseTests.Cookies.cs
@@ -57,6 +57,18 @@ namespace System.Net.Tests
                 144, "Set-Cookie: name=value", null
             };
 
+            if (!PlatformDetection.IsFullFramework)
+            {
+                yield return new object[]
+                {
+                    new CookieCollection()
+                    {
+                        new Cookie("foo bar", "value")
+                    },
+                    147, "Set-Cookie: foo bar=value", null
+                };
+            }
+
             yield return new object[]
             {
                 new CookieCollection()

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 fb3d106..04a51e3 100644 (file)
--- a/src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs
+++ b/src/libraries/System.Net.Primitives/src/System/Net/Cookie.cs
@@ -41,7 +41,8 @@ namespace System.Net
         internal const string SpecialAttributeLiteral = "$";
 
         internal static readonly char[] PortSplitDelimiters = new char[] { ' ', ',', '\"' };
-        internal static readonly char[] ReservedToName = new char[] { ' ', '\t', '\r', '\n', '=', ';', ',' };
+        // 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
+        internal static readonly char[] ReservedToName = new char[] { '\t', '\r', '\n', '=', ';', ',' };
         internal static readonly char[] ReservedToValue = new char[] { ';', ',' };
 
         private string m_comment = string.Empty; // Do not rename (binary serialization)
@@ -255,7 +256,7 @@ namespace System.Net
 #endif
         bool InternalSetName(string value)
         {
-            if (string.IsNullOrEmpty(value) || value[0] == '$' || value.IndexOfAny(ReservedToName) != -1)
+            if (string.IsNullOrEmpty(value) || value[0] == '$' || value.IndexOfAny(ReservedToName) != -1 || value[0] == ' ' || value[value.Length - 1] == ' ')
             {
                 m_name = string.Empty;
                 return false;
@@ -370,7 +371,7 @@ namespace System.Net
             }
 
             // Check the name
-            if (m_name == null || m_name.Length == 0 || m_name[0] == '$' || m_name.IndexOfAny(ReservedToName) != -1)
+            if (string.IsNullOrEmpty(m_name) || m_name[0] == '$' || m_name.IndexOfAny(ReservedToName) != -1 || m_name[0] == ' ' || m_name[m_name.Length - 1] == ' ')
             {
                 if (shouldThrow)
                 {

diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieTest.cs
index a0fa794..3d8b06d 100644 (file)
--- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieTest.cs
+++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/CookieTest.cs
@@ -144,11 +144,18 @@ namespace System.Net.Primitives.Functional.Tests
         [Fact]
         public static void Name_GetSet_Success()
         {
-            Cookie c = new Cookie();
-            Assert.Equal(string.Empty, c.Name);
+            Cookie c1 = new Cookie();
+            Assert.Equal(string.Empty, c1.Name);
 
-            c.Name = "hello";
-            Assert.Equal("hello", c.Name);
+            c1.Name = "hello";
+            Assert.Equal("hello", c1.Name);
+
+            if (!PlatformDetection.IsFullFramework)
+            {
+                Cookie c2 = new Cookie();
+                c2.Name = "hello world";
+                Assert.Equal("hello world", c2.Name);
+            }
         }
 
         [Theory]
@@ -156,6 +163,7 @@ namespace System.Net.Primitives.Functional.Tests
         [InlineData("")]
         [InlineData("$hello")]
         [InlineData("hello ")]
+        [InlineData(" hello")]
         [InlineData("hello\t")]
         [InlineData("hello\r")]
         [InlineData("hello\n")]

diff --git a/src/libraries/System.Net.Primitives/tests/UnitTests/CookieContainerTest.cs b/src/libraries/System.Net.Primitives/tests/UnitTests/CookieContainerTest.cs
index 810465d..389b634 100644 (file)
--- a/src/libraries/System.Net.Primitives/tests/UnitTests/CookieContainerTest.cs
+++ b/src/libraries/System.Net.Primitives/tests/UnitTests/CookieContainerTest.cs
@@ -331,6 +331,19 @@ namespace System.Net.Primitives.Unit.Tests
                     new Cookie("_m_ask_fm_session", "session1")
                 }
             }; // Empty header followed by another empty header at the end
+
+            if (!PlatformDetection.IsFullFramework)
+            {
+                yield return new object[]
+                {
+                    uSecure,
+                    "hello world=value",
+                    new Cookie[]
+                    {
+                        new Cookie("hello world", "value"),
+                    }
+                }; // Name with space in it
+            }
         }
 
         [Theory]