Fix NameValueHeaderValue.CheckValueFormat (dotnet#794)

Fixes NameHeaderValue validation to be in line with RFC. Resolves dotnet#1504
layomia · Feb 6, 2021 · 64090bb · 64090bb
1 parent 306f9e2
commit 64090bb
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 23 deletions.
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs
@@ -353,11 +353,27 @@ private static void CheckNameValueFormat(string name, string? value)
 
         private static void CheckValueFormat(string? value)
         {
-            // Either value is null/empty or a valid token/quoted string
-            if (!(string.IsNullOrEmpty(value) || (GetValueLength(value, 0) == value.Length)))
+            // Either value is null/empty or a valid token/quoted string https://tools.ietf.org/html/rfc7230#section-3.2.6
+            if (string.IsNullOrEmpty(value))
+            {
+                return;
+            }
+
+            // Trailing/leading space are not allowed
+            if (value[0] == ' ' || value[0] == '\t' || value[^1] == ' ' || value[^1] == '\t')
             {
                 throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
             }
+
+            // If it's not a token we check if it's a valid quoted string
+            if (HttpRuleParser.GetTokenLength(value, 0) == 0)
+            {
+                HttpParseResult parseResult = HttpRuleParser.GetQuotedStringLength(value, 0, out int valueLength);
+                if ((parseResult == HttpParseResult.Parsed && valueLength != value.Length) || parseResult != HttpParseResult.Parsed)
+                {
+                    throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
+                }
+            }
         }
 
         private static NameValueHeaderValue CreateNameValue()

diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/NameValueHeaderValueTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/NameValueHeaderValueTest.cs
@@ -26,35 +26,50 @@ public void Ctor_NameEmpty_Throw()
             AssertExtensions.Throws<ArgumentException>("name", () => { NameValueHeaderValue nameValue = new NameValueHeaderValue(string.Empty); });
         }
 
-        [Fact]
-        public void Ctor_NameInvalidFormat_ThrowFormatException()
+        [Theory]
+        [InlineData(" text ")]
+        [InlineData("text ")]
+        [InlineData(" text")]
+        [InlineData("\ttext\t")]
+        [InlineData("text\t")]
+        [InlineData("\ttext")]
+        [InlineData("te xt")]
+        // The ctor takes a name which must not contain '='.
+        [InlineData("te=xt")]
+        [InlineData("te\u00E4xt")]
+        public void Ctor_NameInvalidFormat_ThrowFormatException(string value)
         {
             // When adding values using strongly typed objects, no leading/trailing LWS (whitespace) are allowed.
-            AssertFormatException(" text ", null);
-            AssertFormatException("text ", null);
-            AssertFormatException(" text", null);
-            AssertFormatException("te xt", null);
-            AssertFormatException("te=xt", null); // The ctor takes a name which must not contain '='.
-            AssertFormatException("te\u00E4xt", null);
+            AssertFormatException(value, null);
         }
 
-        [Fact]
-        public void Ctor_NameValidFormat_SuccessfullyCreated()
+        [Theory]
+        [InlineData("text", null)]
+        [InlineData("host", "server.example.com:80")]
+        [InlineData("quoted", "\"value\"")]
+        [InlineData("empty", "")]
+        public void Ctor_NameValidFormat_SuccessfullyCreated(string name, string value)
         {
-            NameValueHeaderValue nameValue = new NameValueHeaderValue("text", null);
-            Assert.Equal("text", nameValue.Name);
+            NameValueHeaderValue nameValue = new NameValueHeaderValue(name, value);
+            Assert.Equal(name, nameValue.Name);
+            Assert.Equal(value, nameValue.Value);
         }
 
-        [Fact]
-        public void Ctor_ValueInvalidFormat_ThrowFormatException()
+        [Theory]
+        [InlineData(" token ")]
+        [InlineData("token ")]
+        [InlineData(" token")]
+        [InlineData("\ttoken")]
+        [InlineData("\ttoken\t")]
+        [InlineData("token\t")]
+        [InlineData("\"quoted string with \" quotes\"")]
+        [InlineData("\"quoted string with \"two\" quotes\"")]
+        [InlineData("\"")]
+        [InlineData(" ")]
+        public void Ctor_ValueInvalidFormat_ThrowFormatException(string value)
         {
             // When adding values using strongly typed objects, no leading/trailing LWS (whitespace) are allowed.
-            AssertFormatException("text", " token ");
-            AssertFormatException("text", "token ");
-            AssertFormatException("text", " token");
-            AssertFormatException("text", "token string");
-            AssertFormatException("text", "\"quoted string with \" quotes\"");
-            AssertFormatException("text", "\"quoted string with \"two\" quotes\"");
+            AssertFormatException("text", value);
         }
 
         [Fact]
@@ -72,7 +87,6 @@ public void Value_CallSetterWithInvalidValues_Throw()
         {
             // Just verify that the setter calls the same validation the ctor invokes.
             Assert.Throws<FormatException>(() => { var x = new NameValueHeaderValue("name"); x.Value = " x "; });
-            Assert.Throws<FormatException>(() => { var x = new NameValueHeaderValue("name"); x.Value = "x y"; });
         }
 
         [Fact]