Skip to content

Commit

Permalink
Remove support for deprecated obs-fold in header values (#53505)
Browse files Browse the repository at this point in the history
* Remove support for deprecated obs-fold in header values

* Add new-line validation to Well-Known header parsing
  • Loading branch information
MihaZupan authored Jul 15, 2021
1 parent ce7759e commit 0733681
Show file tree
Hide file tree
Showing 46 changed files with 401 additions and 1,111 deletions.
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
<value>Invalid range. At least one of the two parameters must not be null.</value>
</data>
<data name="net_http_headers_no_newlines" xml:space="preserve">
<value>New-line characters in header values must be followed by a white-space character.</value>
<value>New-line characters are not allowed in header values.</value>
</data>
<data name="net_http_content_buffersize_exceeded" xml:space="preserve">
<value>Cannot write more bytes to the buffer than the configured maximum buffer size: {0}.</value>
Expand Down Expand Up @@ -217,7 +217,7 @@
<value>The field cannot be longer than {0} characters.</value>
</data>
<data name="net_http_log_headers_no_newlines" xml:space="preserve">
<value>Value for header '{0}' contains invalid new-line characters. Value: '{1}'.</value>
<value>Value for header '{0}' contains new-line characters. Value: '{1}'.</value>
</data>
<data name="net_http_log_headers_invalid_quality" xml:space="preserve">
<value>The 'q' value is invalid: '{0}'.</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public AuthenticationHeaderValue(string scheme)
public AuthenticationHeaderValue(string scheme, string? parameter)
{
HeaderUtilities.CheckValidToken(scheme, nameof(scheme));
HttpHeaders.CheckContainsNewLine(parameter);
_scheme = scheme;
_parameter = parameter;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private static int ParseMultipleEntityTags(string value, int startIndex, out obj
/// </summary>
private static int ParseWithoutValidation(string value, int startIndex, out object? parsedValue)
{
if (HttpRuleParser.ContainsInvalidNewLine(value, startIndex))
if (HttpRuleParser.ContainsNewLine(value, startIndex))
{
parsedValue = null;
return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ internal bool TryGetValues(HeaderDescriptor descriptor, [NotNullWhen(true)] out
internal bool Contains(HeaderDescriptor descriptor)
{
// We can't just call headerStore.ContainsKey() since after parsing the value the header may not exist
// anymore (if the value contains invalid newline chars, we remove the header). So try to parse the
// anymore (if the value contains newline chars, we remove the header). So try to parse the
// header value.
return _headerStore != null && TryGetAndParseHeaderInfo(descriptor, out _);
}
Expand Down Expand Up @@ -318,7 +318,7 @@ private IEnumerator<KeyValuePair<string, IEnumerable<string>>> GetEnumeratorCore
// values.
if (!ParseRawHeaderValues(descriptor, info, removeEmptyHeader: false))
{
// We have an invalid header value (contains invalid newline chars). Delete it.
// We have an invalid header value (contains newline chars). Delete it.
_headerStore.Remove(descriptor);
}
else
Expand Down Expand Up @@ -726,18 +726,17 @@ private bool ParseRawHeaderValues(HeaderDescriptor descriptor, HeaderStoreItemIn
}

// At this point all values are either in info.ParsedValue, info.InvalidValue, or were removed since they
// contain invalid newline chars. Reset RawValue.
// contain newline chars. Reset RawValue.
info.RawValue = null;

// During parsing, we removed the value since it contains invalid newline chars. Return false to indicate that
// During parsing, we removed the value since it contains newline chars. Return false to indicate that
// this is an empty header. If the caller specified to remove empty headers, we'll remove the header before
// returning.
if ((info.InvalidValue == null) && (info.ParsedValue == null))
{
if (removeEmptyHeader)
{
// After parsing the raw value, no value is left because all values contain invalid newline
// chars.
// After parsing the raw value, no value is left because all values contain newline chars.
Debug.Assert(_headerStore != null);
_headerStore.Remove(descriptor);
}
Expand All @@ -754,7 +753,7 @@ private static void ParseMultipleRawHeaderValues(HeaderDescriptor descriptor, He
{
foreach (string rawValue in rawValues)
{
if (!ContainsInvalidNewLine(rawValue, descriptor.Name))
if (!ContainsNewLine(rawValue, descriptor.Name))
{
AddParsedValue(info, rawValue);
}
Expand All @@ -779,7 +778,7 @@ private static void ParseSingleRawHeaderValue(HeaderDescriptor descriptor, Heade

if (descriptor.Parser == null)
{
if (!ContainsInvalidNewLine(rawValue, descriptor.Name))
if (!ContainsNewLine(rawValue, descriptor.Name))
{
AddParsedValue(info, rawValue);
}
Expand Down Expand Up @@ -868,7 +867,7 @@ private static bool TryParseAndAddRawHeaderValue(HeaderDescriptor descriptor, He
}
else
{
if (!ContainsInvalidNewLine(value, descriptor.Name) && addWhenInvalid)
if (!ContainsNewLine(value, descriptor.Name) && addWhenInvalid)
{
AddInvalidValue(info, value);
}
Expand All @@ -885,7 +884,7 @@ private static bool TryParseAndAddRawHeaderValue(HeaderDescriptor descriptor, He
}

Debug.Assert(value != null);
if (!ContainsInvalidNewLine(value, descriptor.Name) && addWhenInvalid)
if (!ContainsNewLine(value, descriptor.Name) && addWhenInvalid)
{
AddInvalidValue(info, value ?? string.Empty);
}
Expand Down Expand Up @@ -973,8 +972,8 @@ private void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreItemInfo i
if (descriptor.Parser == null)
{
// If we don't have a parser for the header, we consider the value valid if it doesn't contains
// invalid newline characters. We add the values as "parsed value". Note that we allow empty values.
CheckInvalidNewLine(value);
// newline characters. We add the values as "parsed value". Note that we allow empty values.
CheckContainsNewLine(value);
AddParsedValue(info, value ?? string.Empty);
return;
}
Expand Down Expand Up @@ -1077,22 +1076,22 @@ private bool TryGetHeaderDescriptor(string name, out HeaderDescriptor descriptor
return false;
}

private static void CheckInvalidNewLine(string? value)
internal static void CheckContainsNewLine(string? value)
{
if (value == null)
{
return;
}

if (HttpRuleParser.ContainsInvalidNewLine(value))
if (HttpRuleParser.ContainsNewLine(value))
{
throw new FormatException(SR.net_http_headers_no_newlines);
}
}

private static bool ContainsInvalidNewLine(string value, string name)
private static bool ContainsNewLine(string value, string name)
{
if (HttpRuleParser.ContainsInvalidNewLine(value))
if (HttpRuleParser.ContainsNewLine(value))
{
if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(null, SR.Format(SR.net_http_log_headers_no_newlines, name, value));
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;

namespace System.Net.Http.Headers
{
Expand Down Expand Up @@ -102,6 +101,8 @@ public string? From
value = null;
}

CheckContainsNewLine(value);

SetOrRemoveParsedValue(KnownHeaders.From.Descriptor, value);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ namespace System.Net.Http.Headers
{
internal sealed class MediaTypeHeaderParser : BaseHeaderParser
{
private readonly bool _supportsMultipleValues;
private readonly Func<MediaTypeHeaderValue> _mediaTypeCreator;

internal static readonly MediaTypeHeaderParser SingleValueParser = new MediaTypeHeaderParser(false, CreateMediaType);
Expand All @@ -18,8 +17,6 @@ private MediaTypeHeaderParser(bool supportsMultipleValues, Func<MediaTypeHeaderV
: base(supportsMultipleValues)
{
Debug.Assert(mediaTypeCreator != null);

_supportsMultipleValues = supportsMultipleValues;
_mediaTypeCreator = mediaTypeCreator;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

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

namespace System.Net.Http.Headers
Expand Down Expand Up @@ -362,18 +360,24 @@ private static void CheckValueFormat(string? value)
// 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));
ThrowFormatException(value);
}

// If it's not a token we check if it's a valid quoted string
if (HttpRuleParser.GetTokenLength(value, 0) == 0)
if (value[0] == '"')
{
HttpParseResult parseResult = HttpRuleParser.GetQuotedStringLength(value, 0, out int valueLength);
if ((parseResult == HttpParseResult.Parsed && valueLength != value.Length) || parseResult != HttpParseResult.Parsed)
if (parseResult != HttpParseResult.Parsed || valueLength != value.Length)
{
throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
ThrowFormatException(value);
}
}
else if (HttpRuleParser.ContainsNewLine(value))
{
ThrowFormatException(value);
}

static void ThrowFormatException(string value) =>
throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
}

private static NameValueHeaderValue CreateNameValue()
Expand Down
63 changes: 11 additions & 52 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Globalization;
using System.Text;

namespace System.Net.Http
Expand Down Expand Up @@ -141,63 +140,16 @@ internal static int GetWhitespaceLength(string input, int startIndex)
continue;
}

if (c == '\r')
{
// If we have a #13 char, it must be followed by #10 and then at least one SP or HT.
if ((current + 2 < input.Length) && (input[current + 1] == '\n'))
{
char spaceOrTab = input[current + 2];
if ((spaceOrTab == ' ') || (spaceOrTab == '\t'))
{
current += 3;
continue;
}
}
}

return current - startIndex;
}

// All characters between startIndex and the end of the string are LWS characters.
return input.Length - startIndex;
}

internal static bool ContainsInvalidNewLine(string value)
internal static bool ContainsNewLine(string value, int startIndex = 0)
{
return ContainsInvalidNewLine(value, 0);
}

internal static bool ContainsInvalidNewLine(string value, int startIndex)
{
// Search for newlines followed by non-whitespace: This is not allowed in any header (be it a known or
// custom header). E.g. "value\r\nbadformat: header" is invalid. However "value\r\n goodformat: header"
// is valid: newlines followed by whitespace are allowed in header values.
int current = startIndex;
while (current < value.Length)
{
if (value[current] == '\r')
{
int char10Index = current + 1;
if ((char10Index < value.Length) && (value[char10Index] == '\n'))
{
current = char10Index + 1;

if (current == value.Length)
{
return true; // We have a string terminating with \r\n. This is invalid.
}

char c = value[current];
if ((c != ' ') && (c != '\t'))
{
return true;
}
}
}
current++;
}

return false;
return value.AsSpan(startIndex).IndexOfAny('\r', '\n') != -1;
}

internal static int GetNumberLength(string input, int startIndex, bool allowDecimal)
Expand Down Expand Up @@ -331,7 +283,7 @@ internal static HttpParseResult GetQuotedPairLength(string input, int startIndex
}

// TEXT = <any OCTET except CTLs, but including LWS>
// LWS = [CRLF] 1*( SP | HT )
// LWS = SP | HT
// CTL = <any US-ASCII control character (octets 0 - 31) and DEL (127)>
//
// Since we don't really care about the content of a quoted string or comment, we're more tolerant and
Expand Down Expand Up @@ -370,8 +322,15 @@ private static HttpParseResult GetExpressionLength(string input, int startIndex,
continue;
}

char c = input[current];

if (c == '\r' || c == '\n')
{
return HttpParseResult.InvalidFormat;
}

// If we support nested expressions and we find an open-char, then parse the nested expressions.
if (supportsNesting && (input[current] == openChar))
if (supportsNesting && (c == openChar))
{
// Check if we exceeded the number of nested calls.
if (nestedCount > MaxNestedCount)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Net.Http.Headers;
using Xunit;
using Xunit.Sdk;

namespace System.Net.Http.Tests
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Net.Http.Headers;
using System.Text;

using Xunit;

Expand Down
Loading

0 comments on commit 0733681

Please sign in to comment.