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.

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

Fix WhiteSpace validation in HttpHeaders #102693

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 0 additions & 3 deletions src/libraries/System.Net.Http/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,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_etag_name" xml:space="preserve">
<value>The specified value is not a valid quoted string.</value>
</data>
<data name="net_http_headers_invalid_range" xml:space="preserve">
<value>Invalid range. At least one of the two parameters must not be null.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Text;

namespace System.Net.Http.Headers
Expand Down Expand Up @@ -35,7 +34,7 @@ public string DispositionType
get { return _dispositionType; }
set
{
CheckDispositionTypeFormat(value);
HeaderUtilities.CheckValidToken(value);
_dispositionType = value;
}
}
Expand Down Expand Up @@ -125,7 +124,7 @@ public long? Size

#region Constructors

internal ContentDispositionHeaderValue()
private ContentDispositionHeaderValue()
{
// Used by the parser to create a new instance of this type.
}
Expand All @@ -140,7 +139,8 @@ protected ContentDispositionHeaderValue(ContentDispositionHeaderValue source)

public ContentDispositionHeaderValue(string dispositionType)
{
CheckDispositionTypeFormat(dispositionType);
HeaderUtilities.CheckValidToken(dispositionType);

_dispositionType = dispositionType;
}

Expand Down Expand Up @@ -271,19 +271,6 @@ private static int GetDispositionTypeExpressionLength(string input, int startInd
return typeLength;
}

private static void CheckDispositionTypeFormat(string dispositionType, [CallerArgumentExpression(nameof(dispositionType))] string? parameterName = null)
{
ArgumentException.ThrowIfNullOrWhiteSpace(dispositionType, parameterName);

// When adding values using strongly typed objects, no leading/trailing LWS (whitespace) are allowed.
int dispositionTypeLength = GetDispositionTypeExpressionLength(dispositionType, 0, out string? tempDispositionType);
if ((dispositionTypeLength == 0) || (tempDispositionType!.Length != dispositionType.Length))
{
throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture,
SR.net_http_headers_invalid_value, dispositionType));
}
}

#endregion Parsing

#region Helpers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,25 @@ namespace System.Net.Http.Headers
{
public class EntityTagHeaderValue : ICloneable
{
private readonly string _tag;
private readonly bool _isWeak;
public string Tag { get; private init; }

public string Tag
{
get { return _tag; }
}

public bool IsWeak
{
get { return _isWeak; }
}
public bool IsWeak { get; private init; }

public static EntityTagHeaderValue Any { get; } = new EntityTagHeaderValue();
public static EntityTagHeaderValue Any { get; } = new EntityTagHeaderValue("*", isWeak: false, false);

private EntityTagHeaderValue()
private EntityTagHeaderValue(string tag, bool isWeak, bool _)
{
_tag = "*";
#if DEBUG
// This constructor should only be used with already validated values.
// "*" is a special case that can only be created via the static Any property.
if (tag != "*")
{
new EntityTagHeaderValue(tag, isWeak);
}
#endif

Tag = tag;
IsWeak = isWeak;
}

public EntityTagHeaderValue(string tag)
Expand All @@ -35,56 +36,31 @@ public EntityTagHeaderValue(string tag)

public EntityTagHeaderValue(string tag, bool isWeak)
{
ArgumentException.ThrowIfNullOrWhiteSpace(tag);
HeaderUtilities.CheckValidQuotedString(tag);
liveans marked this conversation as resolved.
Show resolved Hide resolved

int length;
if ((HttpRuleParser.GetQuotedStringLength(tag, 0, out length) != HttpParseResult.Parsed) ||
(length != tag.Length))
{
// Note that we don't allow 'W/' prefixes for weak ETags in the 'tag' parameter. If the user wants to
// add a weak ETag, they can set 'isWeak' to true.
throw new FormatException(SR.net_http_headers_invalid_etag_name);
}

_tag = tag;
_isWeak = isWeak;
Tag = tag;
IsWeak = isWeak;
}

private EntityTagHeaderValue(EntityTagHeaderValue source)
{
Debug.Assert(source != null);

_tag = source._tag;
_isWeak = source._isWeak;
Tag = source.Tag;
IsWeak = source.IsWeak;
}

public override string ToString()
{
if (_isWeak)
{
return "W/" + _tag;
}
return _tag;
}

public override bool Equals([NotNullWhen(true)] object? obj)
{
EntityTagHeaderValue? other = obj as EntityTagHeaderValue;

if (other == null)
{
return false;
}
public override string ToString() =>
IsWeak ? $"W/{Tag}" : Tag;

public override bool Equals([NotNullWhen(true)] object? obj) =>
obj is EntityTagHeaderValue other &&
IsWeak == other.IsWeak &&
// Since the tag is a quoted-string we treat it case-sensitive.
return ((_isWeak == other._isWeak) && string.Equals(_tag, other._tag, StringComparison.Ordinal));
}
string.Equals(Tag, other.Tag, StringComparison.Ordinal);

public override int GetHashCode()
{
// Since the tag is a quoted-string we treat it case-sensitive.
return _tag.GetHashCode() ^ _isWeak.GetHashCode();
}
public override int GetHashCode() =>
HashCode.Combine(Tag, IsWeak);

public static EntityTagHeaderValue Parse(string input)
{
Expand Down Expand Up @@ -144,24 +120,15 @@ internal static int GetEntityTagLength(string? input, int startIndex, out Entity
current += HttpRuleParser.GetWhitespaceLength(input, current);
}

int tagStartIndex = current;
int tagLength;
if (current == input.Length || HttpRuleParser.GetQuotedStringLength(input, current, out tagLength) != HttpParseResult.Parsed)
if (current == input.Length || HttpRuleParser.GetQuotedStringLength(input, current, out int tagLength) != HttpParseResult.Parsed)
{
return 0;
}

if (tagLength == input.Length)
{
// Most of the time we'll have strong ETags without leading/trailing whitespace.
Debug.Assert(startIndex == 0);
Debug.Assert(!isWeak);
parsedValue = new EntityTagHeaderValue(input);
}
else
{
parsedValue = new EntityTagHeaderValue(input.Substring(tagStartIndex, tagLength), isWeak);
}
// Most of the time we'll have strong ETags without leading/trailing whitespace.
Debug.Assert(tagLength != input.Length || (startIndex == 0 && !isWeak));

parsedValue = new EntityTagHeaderValue(input.Substring(current, tagLength), isWeak, false);

current += tagLength;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,20 +142,19 @@ private static void AddHexEscaped(byte c, ref ValueStringBuilder destination)

internal static void CheckValidToken(string value, [CallerArgumentExpression(nameof(value))] string? parameterName = null)
{
ArgumentException.ThrowIfNullOrWhiteSpace(value, parameterName);
ArgumentException.ThrowIfNullOrEmpty(value, parameterName);

if (HttpRuleParser.GetTokenLength(value, 0) != value.Length)
if (!HttpRuleParser.IsToken(value))
{
throw new FormatException(SR.Format(CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
}
}

internal static void CheckValidComment(string value, [CallerArgumentExpression(nameof(value))] string? parameterName = null)
{
ArgumentException.ThrowIfNullOrWhiteSpace(value, parameterName);
ArgumentException.ThrowIfNullOrEmpty(value, parameterName);

int length;
if ((HttpRuleParser.GetCommentLength(value, 0, out length) != HttpParseResult.Parsed) ||
if ((HttpRuleParser.GetCommentLength(value, 0, out int length) != HttpParseResult.Parsed) ||
(length != value.Length)) // no trailing spaces allowed
{
throw new FormatException(SR.Format(CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
Expand All @@ -164,10 +163,9 @@ internal static void CheckValidComment(string value, [CallerArgumentExpression(n

internal static void CheckValidQuotedString(string value, [CallerArgumentExpression(nameof(value))] string? parameterName = null)
{
ArgumentException.ThrowIfNullOrWhiteSpace(value, parameterName);
ArgumentException.ThrowIfNullOrEmpty(value, parameterName);

int length;
if ((HttpRuleParser.GetQuotedStringLength(value, 0, out length) != HttpParseResult.Parsed) ||
if ((HttpRuleParser.GetQuotedStringLength(value, 0, out int length) != HttpParseResult.Parsed) ||
(length != value.Length)) // no trailing spaces allowed
{
throw new FormatException(SR.Format(CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte

private HeaderDescriptor GetHeaderDescriptor(string name)
{
ArgumentException.ThrowIfNullOrWhiteSpace(name);
ArgumentException.ThrowIfNullOrEmpty(name);

if (!HeaderDescriptor.TryGet(name, out HeaderDescriptor descriptor))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Text;
using static System.HexConverter;

namespace System.Net.Http.Headers
{
Expand Down Expand Up @@ -275,7 +274,7 @@ private static int GetMediaTypeExpressionLength(string input, int startIndex, ou

private static void CheckMediaTypeFormat(string mediaType, [CallerArgumentExpression(nameof(mediaType))] string? parameterName = null)
{
ArgumentException.ThrowIfNullOrWhiteSpace(mediaType, parameterName);
ArgumentException.ThrowIfNullOrEmpty(mediaType, parameterName);

// When adding values using strongly typed objects, no leading/trailing LWS (whitespace) are allowed.
// Also no LWS between type and subtype are allowed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Text;

namespace System.Net.Http.Headers
Expand All @@ -15,24 +14,25 @@ public class ViaHeaderValue : ICloneable
private readonly string _receivedBy;
private readonly string? _comment;

public string? ProtocolName
{
get { return _protocolName; }
}
public string? ProtocolName => _protocolName;

public string ProtocolVersion
{
get { return _protocolVersion; }
}
public string ProtocolVersion => _protocolVersion;

public string ReceivedBy
{
get { return _receivedBy; }
}
public string ReceivedBy => _receivedBy;

public string? Comment => _comment;

public string? Comment
private ViaHeaderValue(string protocolVersion, string receivedBy, string? protocolName, string? comment, bool _)
{
get { return _comment; }
#if DEBUG
// This constructor should only be used with already validated values.
new ViaHeaderValue(protocolVersion, receivedBy, protocolName, comment);
#endif

_protocolVersion = protocolVersion;
_receivedBy = receivedBy;
_protocolName = protocolName;
_comment = comment;
}

public ViaHeaderValue(string protocolVersion, string receivedBy)
Expand Down Expand Up @@ -99,40 +99,21 @@ public override string ToString()
return sb.ToString();
}

public override bool Equals([NotNullWhen(true)] object? obj)
{
ViaHeaderValue? other = obj as ViaHeaderValue;

if (other == null)
{
return false;
}

public override bool Equals([NotNullWhen(true)] object? obj) =>
obj is ViaHeaderValue other &&
// Note that for token and host case-insensitive comparison is used. Comments are compared using case-
// sensitive comparison.
return string.Equals(_protocolVersion, other._protocolVersion, StringComparison.OrdinalIgnoreCase) &&
string.Equals(_receivedBy, other._receivedBy, StringComparison.OrdinalIgnoreCase) &&
string.Equals(_protocolName, other._protocolName, StringComparison.OrdinalIgnoreCase) &&
string.Equals(_comment, other._comment, StringComparison.Ordinal);
}

public override int GetHashCode()
{
int result = StringComparer.OrdinalIgnoreCase.GetHashCode(_protocolVersion) ^
StringComparer.OrdinalIgnoreCase.GetHashCode(_receivedBy);

if (!string.IsNullOrEmpty(_protocolName))
{
result ^= StringComparer.OrdinalIgnoreCase.GetHashCode(_protocolName);
}

if (!string.IsNullOrEmpty(_comment))
{
result ^= _comment.GetHashCode();
}

return result;
}
string.Equals(_protocolVersion, other._protocolVersion, StringComparison.OrdinalIgnoreCase) &&
string.Equals(_receivedBy, other._receivedBy, StringComparison.OrdinalIgnoreCase) &&
string.Equals(_protocolName, other._protocolName, StringComparison.OrdinalIgnoreCase) &&
string.Equals(_comment, other._comment, StringComparison.Ordinal);

public override int GetHashCode() =>
HashCode.Combine(
StringComparer.OrdinalIgnoreCase.GetHashCode(_protocolVersion),
StringComparer.OrdinalIgnoreCase.GetHashCode(_receivedBy),
_protocolName is null ? 0 : StringComparer.OrdinalIgnoreCase.GetHashCode(_protocolName),
_comment);

public static ViaHeaderValue Parse(string input)
{
Expand Down Expand Up @@ -191,8 +172,7 @@ internal static int GetViaLength(string? input, int startIndex, out object? pars
if ((current < input.Length) && (input[current] == '('))
{
// We have a <comment> in '[<protocolName>/]<protocolVersion> <receivedBy> [<comment>]'
int commentLength;
if (HttpRuleParser.GetCommentLength(input, current, out commentLength) != HttpParseResult.Parsed)
if (HttpRuleParser.GetCommentLength(input, current, out int commentLength) != HttpParseResult.Parsed)
{
return 0; // We found a '(' character but it wasn't a valid comment. Abort.
}
Expand All @@ -203,7 +183,7 @@ internal static int GetViaLength(string? input, int startIndex, out object? pars
current += HttpRuleParser.GetWhitespaceLength(input, current);
}

parsedValue = new ViaHeaderValue(protocolVersion, receivedBy!, protocolName, comment);
parsedValue = new ViaHeaderValue(protocolVersion, receivedBy, protocolName, comment, false);
return current - startIndex;
}

Expand Down Expand Up @@ -275,7 +255,7 @@ object ICloneable.Clone()

private static void CheckReceivedBy(string receivedBy)
{
ArgumentException.ThrowIfNullOrWhiteSpace(receivedBy);
ArgumentException.ThrowIfNullOrEmpty(receivedBy);

// 'receivedBy' can either be a host or a token. Since a token is a valid host, we only verify if the value
// is a valid host.;
Expand Down
Loading