Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
May 19, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18f3a7e
Remove From header validation from System.Net.Http
eerhardt May 14, 2021
4d63785
Respond to PR feedback
eerhardt May 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions src/libraries/Common/src/System/Net/Mail/MailAddress.cs
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions src/libraries/System.Net.Http/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
16 changes: 0 additions & 16 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,22 +496,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" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay :)

<Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
</ItemGroup>
Expand Down
15 changes: 6 additions & 9 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 0 additions & 13 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
4 changes: 0 additions & 4 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" });
Expand Down
21 changes: 16 additions & 5 deletions src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -739,14 +749,15 @@ public void From_UseAddMethod_AddedValueCanBeRetrievedUsingProperty()
[Fact]
public void From_UseAddMethodWithInvalidValue_InvalidValueRecognized()
{
// values are not validated, so invalid values are accepted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this one can be removed, it's calling GetParsedValues which is pointless if we don't have a parser.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, per my comments above about using TokenParser, we should keep this and use it to verify that multiple-valued headers are disallowed.

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());
}
Expand Down
17 changes: 0 additions & 17 deletions src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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" />
Expand Down
21 changes: 7 additions & 14 deletions src/libraries/System.Net.Mail/src/System.Net.Mail.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand All @@ -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"
Expand Down
0 ...rc/System/Net/Mail/DomainLiteralReader.cs → ...rc/System/Net/Mail/DomainLiteralReader.cs
View file
Open in desktop
File renamed without changes.
0 ...mmon/src/System/Net/Mail/DotAtomReader.cs → ...Mail/src/System/Net/Mail/DotAtomReader.cs
View file
Open in desktop
File renamed without changes.
0 .../src/System/Net/Mail/MailAddressParser.cs → .../src/System/Net/Mail/MailAddressParser.cs
View file
Open in desktop
File renamed without changes.
0 ...mmon/src/System/Net/Mail/MailBnfHelper.cs → ...Mail/src/System/Net/Mail/MailBnfHelper.cs
View file
Open in desktop
File renamed without changes.
0 ...n/src/System/Net/Mail/QuotedPairReader.cs → ...l/src/System/Net/Mail/QuotedPairReader.cs
View file
Open in desktop
File renamed without changes.
0 ...stem/Net/Mail/QuotedStringFormatReader.cs → ...stem/Net/Mail/QuotedStringFormatReader.cs
View file
Open in desktop
File renamed without changes.
0 ...n/src/System/Net/Mail/WhitespaceReader.cs → ...l/src/System/Net/Mail/WhitespaceReader.cs
View file
Open in desktop
File renamed without changes.
28 changes: 14 additions & 14 deletions src/libraries/System.Net.Mail/tests/Unit/System.Net.Mail.Unit.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down