Skip to content

Commit

Permalink
Fix WhiteSpace validation in HttpHeaders (dotnet#102693)
Browse files Browse the repository at this point in the history
  • Loading branch information
@MihaZupan
MihaZupan committed Jun 10, 2024
1 parent ba6089e commit e2bc045
Show file tree
Hide file tree
Showing 11 changed files with 125 additions and 197 deletions.
3 changes: 0 additions & 3 deletions src/libraries/System.Net.Http/src/Resources/Strings.resx
View file
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
21 changes: 4 additions & 17 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs
View file
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
99 changes: 33 additions & 66 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/EntityTagHeaderValue.cs
View file
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);

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
14 changes: 6 additions & 8 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs
View file
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
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs
View file
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
3 changes: 1 addition & 2 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs
View file
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
82 changes: 31 additions & 51 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/ViaHeaderValue.cs
View file
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

0 comments on commit e2bc045

Please sign in to comment.