Remove From header validation from System.Net.Http (#52794)

* Remove From header validation from System.Net.Http The From header is not commonly used and this logic adds a decent amount of code to System.Net.Http that can't be trimmed. Fix #52664
dotnet · May 19, 2021 · 9a91eeb · 9a91eeb
1 parent 65082ab
commit 9a91eeb
Show file tree

Hide file tree

Showing 18 changed files with 44 additions and 130 deletions.
diff --git a/src/libraries/Common/src/System/Net/Mail/MailAddress.cs b/src/libraries/Common/src/System/Net/Mail/MailAddress.cs
diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx
@@ -150,9 +150,6 @@
   <data name="net_http_headers_invalid_host_header" xml:space="preserve">
     <value>The specified value is not a valid 'Host' header string.</value>
   </data>
-  <data name="net_http_headers_invalid_from_header" xml:space="preserve">
-    <value>The specified value is not a valid 'From' header string.</value>
-  </data>
   <data name="net_http_headers_invalid_etag_name" xml:space="preserve">
     <value>The specified value is not a valid quoted string.</value>
   </data>
@@ -228,9 +225,6 @@
   <data name="net_http_log_headers_invalid_quality" xml:space="preserve">
     <value>The 'q' value is invalid: '{0}'.</value>
   </data>
-  <data name="net_http_log_headers_wrong_email_format" xml:space="preserve">
-    <value>Value '{0}' is not a valid email address</value>
-  </data>
   <data name="net_http_handler_not_assigned" xml:space="preserve">
     <value>The inner handler has not been assigned.</value>
   </data>
@@ -270,18 +264,6 @@
   <data name="net_http_value_must_be_greater_than_or_equal" xml:space="preserve">
     <value>The specified value '{0}' must be greater than or equal to '{1}'.</value>
   </data>
-  <data name="MailHeaderFieldInvalidCharacter" xml:space="preserve">
-    <value>An invalid character was found in the mail header: '{0}'.</value>
-  </data>
-  <data name="MailAddressInvalidFormat" xml:space="preserve">
-    <value>The specified string is not in the form required for an e-mail address.</value>
-  </data>
-  <data name="MailHeaderFieldMalformedHeader" xml:space="preserve">
-    <value>The mail header is malformed.</value>
-  </data>
-  <data name="InvalidHeaderName" xml:space="preserve">
-    <value>An invalid character was found in header name.</value>
-  </data>
   <data name="net_cookie_attribute" xml:space="preserve">
     <value>The '{0}'='{1}' part of the cookie is invalid.</value>
   </data>

diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj
@@ -494,22 +494,6 @@
     <Compile Include="System\Net\Http\DiagnosticsHandler.cs" />
     <Compile Include="System\Net\Http\DiagnosticsHandlerLoggingStrings.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\SocketsHttpPlaintextStreamFilterContext.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DomainLiteralReader.cs"
-             Link="Common\System\Net\Mail\DomainLiteralReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DotAtomReader.cs"
-             Link="Common\System\Net\Mail\DotAtomReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailAddress.cs"
-             Link="Common\System\Net\Mail\MailAddress.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailAddressParser.cs"
-             Link="Common\System\Net\Mail\MailAddressParser.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailBnfHelper.cs"
-             Link="Common\System\Net\Mail\MailBnfHelper.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedPairReader.cs"
-             Link="Common\System\Net\Mail\QuotedPairReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedStringFormatReader.cs"
-             Link="Common\System\Net\Mail\QuotedStringFormatReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\WhitespaceReader.cs"
-             Link="Common\System\Net\Mail\WhitespaceReader.cs" />
     <Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
              Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
   </ItemGroup>

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs
@@ -18,7 +18,7 @@ internal sealed class GenericHeaderParser : BaseHeaderParser
         internal static readonly GenericHeaderParser MultipleValueNameValueWithParametersParser = new GenericHeaderParser(true, NameValueWithParametersHeaderValue.GetNameValueWithParametersLength);
         internal static readonly GenericHeaderParser SingleValueNameValueParser = new GenericHeaderParser(false, ParseNameValue);
         internal static readonly GenericHeaderParser MultipleValueNameValueParser = new GenericHeaderParser(true, ParseNameValue);
-        internal static readonly GenericHeaderParser MailAddressParser = new GenericHeaderParser(false, ParseMailAddress);
+        internal static readonly GenericHeaderParser SingleValueParserWithoutValidation = new GenericHeaderParser(false, ParseWithoutValidation);
         internal static readonly GenericHeaderParser SingleValueProductParser = new GenericHeaderParser(false, ParseProduct);
         internal static readonly GenericHeaderParser MultipleValueProductParser = new GenericHeaderParser(true, ParseProduct);
         internal static readonly GenericHeaderParser RangeConditionParser = new GenericHeaderParser(false, RangeConditionHeaderValue.GetRangeConditionLength);
@@ -114,22 +114,19 @@ private static int ParseMultipleEntityTags(string value, int startIndex, out obj
             return resultLength;
         }
 
-        private static int ParseMailAddress(string value, int startIndex, out object? parsedValue)
+        /// <summary>
+        /// Allows for arbitrary header values without validation (aside from newline, which is always invalid in a header value).
+        /// </summary>
+        private static int ParseWithoutValidation(string value, int startIndex, out object? parsedValue)
         {
-            parsedValue = null;
-
             if (HttpRuleParser.ContainsInvalidNewLine(value, startIndex))
             {
+                parsedValue = null;
                 return 0;
             }
 
             string result = value.Substring(startIndex);
 
-            if (!HeaderUtilities.IsValidEmailAddress(result))
-            {
-                return 0;
-            }
-
             parsedValue = result;
             return result.Length;
         }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs
@@ -373,19 +373,6 @@ internal static void DumpHeaders(StringBuilder sb, params HttpHeaders?[] headers
             sb.Append('}');
         }
 
-        internal static bool IsValidEmailAddress(string value)
-        {
-            if (MailAddressParser.TryParseAddress(value, out ParseAddressInfo _, throwExceptionIfFail: false))
-            {
-                return true;
-            }
-            else
-            {
-                if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(null, SR.Format(SR.net_http_log_headers_wrong_email_format, value));
-                return false;
-            }
-        }
-
         private static void ValidateToken(HttpHeaderValueCollection<string> collection, string value)
         {
             CheckValidToken(value, "item");

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs
@@ -102,10 +102,6 @@ public string? From
                     value = null;
                 }
 
-                if ((value != null) && !HeaderUtilities.IsValidEmailAddress(value))
-                {
-                    throw new FormatException(SR.net_http_headers_invalid_from_header);
-                }
                 SetOrRemoveParsedValue(KnownHeaders.From.Descriptor, value);
             }
         }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs
@@ -47,7 +47,7 @@ internal static class KnownHeaders
         public static readonly KnownHeader Expect = new KnownHeader("Expect", HttpHeaderType.Request | HttpHeaderType.NonTrailing, GenericHeaderParser.MultipleValueNameValueWithParametersParser, new string[] { "100-continue" }, H2StaticTable.Expect);
         public static readonly KnownHeader ExpectCT = new KnownHeader("Expect-CT");
         public static readonly KnownHeader Expires = new KnownHeader("Expires", HttpHeaderType.Content | HttpHeaderType.NonTrailing, DateHeaderParser.Parser, null, H2StaticTable.Expires);
-        public static readonly KnownHeader From = new KnownHeader("From", HttpHeaderType.Request, GenericHeaderParser.MailAddressParser, null, H2StaticTable.From);
+        public static readonly KnownHeader From = new KnownHeader("From", HttpHeaderType.Request, GenericHeaderParser.SingleValueParserWithoutValidation, null, H2StaticTable.From);
         public static readonly KnownHeader GrpcEncoding = new KnownHeader("grpc-encoding", HttpHeaderType.Custom, null, new string[] { "identity", "gzip", "deflate" });
         public static readonly KnownHeader GrpcMessage = new KnownHeader("grpc-message");
         public static readonly KnownHeader GrpcStatus = new KnownHeader("grpc-status", HttpHeaderType.Custom, null, new string[] { "0" });

diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs
@@ -720,8 +720,18 @@ public void From_ReadAndWriteProperty_ValueMatchesPriorSetValue()
             Assert.False(headers.Contains("From"),
                 "Header store should not contain a header 'From' after setting it to null.");
 
-            Assert.Throws<FormatException>(() => { headers.From = " "; });
-            Assert.Throws<FormatException>(() => { headers.From = "invalid email address"; });
+            // values are not validated, so invalid values are accepted
+            headers.From = " ";
+            Assert.Equal(" ", headers.From);
+
+            headers.From = "invalid email address";
+            Assert.Equal("invalid email address", headers.From);
+
+            // Null and empty string are equivalent. Setting to empty means remove the From header value (if any).
+            headers.From = string.Empty;
+            Assert.Null(headers.From);
+            Assert.False(headers.Contains("From"),
+                "Header store should not contain a header 'From' after setting it to string.Empty.");
         }
 
         [Fact]
@@ -739,14 +749,15 @@ public void From_UseAddMethod_AddedValueCanBeRetrievedUsingProperty()
         [Fact]
         public void From_UseAddMethodWithInvalidValue_InvalidValueRecognized()
         {
+            // values are not validated, so invalid values are accepted
             headers.TryAddWithoutValidation("From", " info@example.com ,");
-            Assert.Null(headers.GetParsedValues(KnownHeaders.From.Descriptor));
+            Assert.Equal("info@example.com ,", headers.GetParsedValues(KnownHeaders.From.Descriptor));
             Assert.Equal(1, headers.GetValues("From").Count());
-            Assert.Equal(" info@example.com ,", headers.GetValues("From").First());
+            Assert.Equal("info@example.com ,", headers.GetValues("From").First());
 
             headers.Clear();
             headers.TryAddWithoutValidation("From", "info@");
-            Assert.Null(headers.GetParsedValues(KnownHeaders.From.Descriptor));
+            Assert.Equal("info@", headers.GetParsedValues(KnownHeaders.From.Descriptor));
             Assert.Equal(1, headers.GetValues("From").Count());
             Assert.Equal("info@", headers.GetValues("From").First());
         }

diff --git a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj
@@ -52,22 +52,6 @@
              Link="Common\System\Net\Http\HPackEncoder.cs" />
     <Compile Include="$(CommonTestPath)System\Net\Http\HuffmanEncoder.cs"
              Link="Common\System\Net\Http\HuffmanEncoder.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailAddress.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\MailAddress.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DomainLiteralReader.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\DomainLiteralReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DotAtomReader.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\DotAtomReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailAddressParser.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\MailAddressParser.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailBnfHelper.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\MailBnfHelper.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedPairReader.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\QuotedPairReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedStringFormatReader.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\QuotedStringFormatReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\WhitespaceReader.cs"
-             Link="ProductionCode\Common\src\System\Net\Mail\WhitespaceReader.cs" />
     <Compile Include="$(CommonPath)System\Threading\Tasks\TaskToApm.cs"
              Link="ProductionCode\Common\System\Threading\Tasks\TaskToApm.cs" />
     <Compile Include="..\..\src\System\Net\Http\SocketsHttpHandler\AuthenticationHelper.Digest.cs"
@@ -283,7 +267,6 @@
     <Compile Include="Headers\GenericHeaderParserTest\ContentRangeParserTest.cs" />
     <Compile Include="Headers\GenericHeaderParserTest\EntityTagParserTest.cs" />
     <Compile Include="Headers\GenericHeaderParserTest\HostParserTest.cs" />
-    <Compile Include="Headers\GenericHeaderParserTest\MailAddressParserTest.cs" />
     <Compile Include="Headers\GenericHeaderParserTest\NameValueParserTest.cs" />
     <Compile Include="Headers\GenericHeaderParserTest\NameValueWithParametersParserTest.cs" />
     <Compile Include="Headers\GenericHeaderParserTest\ProductParserTest.cs" />

diff --git a/src/libraries/System.Net.Mail/src/System.Net.Mail.csproj b/src/libraries/System.Net.Mail/src/System.Net.Mail.csproj
@@ -42,10 +42,17 @@
     <Compile Include="System\Net\BufferedReadStream.cs" />
     <Compile Include="System\Net\Mail\LinkedResource.cs" />
     <Compile Include="System\Net\Mail\LinkedResourceCollection.cs" />
+    <Compile Include="System\Net\Mail\DomainLiteralReader.cs" />
+    <Compile Include="System\Net\Mail\DotAtomReader.cs" />
     <Compile Include="System\Net\Mail\MailAddress.cs" />
     <Compile Include="System\Net\Mail\MailAddressCollection.cs" />
+    <Compile Include="System\Net\Mail\MailAddressParser.cs" />
+    <Compile Include="System\Net\Mail\MailBnfHelper.cs" />
     <Compile Include="System\Net\Mail\MailMessage.cs" />
     <Compile Include="System\Net\Mail\MailPriority.cs" />
+    <Compile Include="System\Net\Mail\QuotedPairReader.cs" />
+    <Compile Include="System\Net\Mail\QuotedStringFormatReader.cs" />
+    <Compile Include="System\Net\Mail\WhitespaceReader.cs" />
     <Compile Include="System\Net\Mime\MimeMultiPart.cs" />
     <Compile Include="System\Net\Mime\MimeMultiPartType.cs" />
     <Compile Include="System\Net\Mime\MimeWriter.cs" />
@@ -56,22 +63,8 @@
     <Compile Include="System\Net\DelegatedStream.cs" />
     <Compile Include="$(CommonPath)System\Net\LazyAsyncResult.cs"
              Link="Common\System\Net\LazyAsyncResult.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailBnfHelper.cs"
-             Link="Common\System\Net\Mail\MailBnfHelper.cs" />
     <Compile Include="$(CommonPath)System\Net\Logging\NetEventSource.Common.cs"
              Link="Common\System\Net\Logging\NetEventSource.Common.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DotAtomReader.cs"
-             Link="Common\System\Net\Mail\DotAtomReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailAddressParser.cs"
-             Link="Common\System\Net\Mail\MailAddressParser.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedStringFormatReader.cs"
-             Link="Common\System\Net\Mail\QuotedStringFormatReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\WhitespaceReader.cs"
-             Link="Common\System\Net\Mail\WhitespaceReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedPairReader.cs"
-             Link="Common\System\Net\Mail\QuotedPairReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DomainLiteralReader.cs"
-             Link="Common\System\Net\Mail\DomainLiteralReader.cs" />
     <Compile Include="$(CommonPath)System\StringExtensions.cs"
              Link="Common\System\StringExtensions.cs" />
     <Compile Include="$(CommonPath)System\HexConverter.cs"

diff --git a/...rc/System/Net/Mail/DomainLiteralReader.cs → ...rc/System/Net/Mail/DomainLiteralReader.cs b/...rc/System/Net/Mail/DomainLiteralReader.cs → ...rc/System/Net/Mail/DomainLiteralReader.cs
diff --git a/...mmon/src/System/Net/Mail/DotAtomReader.cs → ...Mail/src/System/Net/Mail/DotAtomReader.cs b/...mmon/src/System/Net/Mail/DotAtomReader.cs → ...Mail/src/System/Net/Mail/DotAtomReader.cs
diff --git a/.../src/System/Net/Mail/MailAddressParser.cs → .../src/System/Net/Mail/MailAddressParser.cs b/.../src/System/Net/Mail/MailAddressParser.cs → .../src/System/Net/Mail/MailAddressParser.cs
diff --git a/...mmon/src/System/Net/Mail/MailBnfHelper.cs → ...Mail/src/System/Net/Mail/MailBnfHelper.cs b/...mmon/src/System/Net/Mail/MailBnfHelper.cs → ...Mail/src/System/Net/Mail/MailBnfHelper.cs
diff --git a/...n/src/System/Net/Mail/QuotedPairReader.cs → ...l/src/System/Net/Mail/QuotedPairReader.cs b/...n/src/System/Net/Mail/QuotedPairReader.cs → ...l/src/System/Net/Mail/QuotedPairReader.cs
diff --git a/...stem/Net/Mail/QuotedStringFormatReader.cs → ...stem/Net/Mail/QuotedStringFormatReader.cs b/...stem/Net/Mail/QuotedStringFormatReader.cs → ...stem/Net/Mail/QuotedStringFormatReader.cs
diff --git a/...n/src/System/Net/Mail/WhitespaceReader.cs → ...l/src/System/Net/Mail/WhitespaceReader.cs b/...n/src/System/Net/Mail/WhitespaceReader.cs → ...l/src/System/Net/Mail/WhitespaceReader.cs
diff --git a/src/libraries/System.Net.Mail/tests/Unit/System.Net.Mail.Unit.Tests.csproj b/src/libraries/System.Net.Mail/tests/Unit/System.Net.Mail.Unit.Tests.csproj
@@ -31,16 +31,30 @@
              Link="ProductionCode\SmtpStatusCode.cs" />
     <Compile Include="..\..\src\System\Net\DelegatedStream.cs"
              Link="ProductionCode\DelegatedStream.cs" />
+    <Compile Include="..\..\src\System\Net\Mail\DomainLiteralReader.cs"
+             Link="ProductionCode\DomainLiteralReader.cs" />
+    <Compile Include="..\..\src\System\Net\Mail\DotAtomReader.cs"
+             Link="ProductionCode\DotAtomReader.cs" />
     <Compile Include="..\..\src\System\Net\Mail\MailAddress.cs"
              Link="ProductionCode\MailAddress.cs" />
+    <Compile Include="..\..\src\System\Net\Mail\MailAddressParser.cs"
+             Link="CProductionCode\MailAddressParser.cs" />
+    <Compile Include="..\..\src\System\Net\Mail\MailBnfHelper.cs"
+             Link="ProductionCode\MailBnfHelper.cs" />
     <Compile Include="..\..\src\System\Net\Mail\NetEventSource.Mail.cs"
              Link="ProductionCode\NetEventSource.Mail.cs" />
+    <Compile Include="..\..\src\System\Net\Mail\QuotedPairReader.cs"
+             Link="ProductionCode\QuotedPairReader.cs" />
+    <Compile Include="..\..\src\System\Net\Mail\QuotedStringFormatReader.cs"
+             Link="ProductionCode\QuotedStringFormatReader.cs" />
     <Compile Include="..\..\src\System\Net\Mail\ISmtpAuthenticationModule.cs"
              Link="ProductionCode\ISmtpAuthenticationModule.cs" />
     <Compile Include="..\..\src\System\Net\Mail\MailAddressCollection.cs"
              Link="ProductionCode\MailAddressCollection.cs" />
     <Compile Include="..\..\src\System\Net\Mail\MailPriority.cs"
              Link="ProductionCode\MailPriority.cs" />
+    <Compile Include="..\..\src\System\Net\Mail\WhitespaceReader.cs"
+             Link="ProductionCode\WhitespaceReader.cs" />
     <Compile Include="..\..\src\System\Net\Mime\MimePart.cs"
              Link="ProductionCode\MimePart.cs" />
     <Compile Include="..\..\src\System\Net\Mime\Base64WriteStateInfo.cs"
@@ -103,22 +117,8 @@
              Link="Common\System\Net\InternalException.cs" />
     <Compile Include="$(CommonPath)System\Net\LazyAsyncResult.cs"
              Link="Common\System\Net\LazyAsyncResult.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailBnfHelper.cs"
-             Link="Common\System\Net\Mail\MailBnfHelper.cs" />
     <Compile Include="$(CommonPath)System\Net\Logging\NetEventSource.Common.cs"
              Link="Common\System\Net\Logging\NetEventSource.Common.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DotAtomReader.cs"
-             Link="Common\System\Net\Mail\DotAtomReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\MailAddressParser.cs"
-             Link="Common\System\Net\Mail\MailAddressParser.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedStringFormatReader.cs"
-             Link="Common\System\Net\Mail\QuotedStringFormatReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\WhitespaceReader.cs"
-             Link="Common\System\Net\Mail\WhitespaceReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\QuotedPairReader.cs"
-             Link="Common\System\Net\Mail\QuotedPairReader.cs" />
-    <Compile Include="$(CommonPath)System\Net\Mail\DomainLiteralReader.cs"
-             Link="Common\System\Net\Mail\DomainLiteralReader.cs" />
     <Compile Include="$(CommonPath)System\Net\SecurityProtocol.cs"
              Link="Common\System\Net\SecurityProtocol.cs" />
     <Compile Include="$(CommonPath)System\StringExtensions.cs"