Skip to content

Commit 31fcaf9

Browse files
MihaZupanMihaZupan
committed
Add new-line validation to Well-Known header parsing
1 parent 3f3e304 commit 31fcaf9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+324
-993
lines changed

‎src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public AuthenticationHeaderValue(string scheme)
3838
public AuthenticationHeaderValue(string scheme, string? parameter)
3939
{
4040
HeaderUtilities.CheckValidToken(scheme, nameof(scheme));
41+
HttpHeaders.CheckContainsNewLine(parameter);
4142
_scheme = scheme;
4243
_parameter = parameter;
4344
}

‎src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,7 @@ private bool TryGetHeaderDescriptor(string name, out HeaderDescriptor descriptor
10761076
return false;
10771077
}
10781078

1079-
private static void CheckContainsNewLine(string? value)
1079+
internal static void CheckContainsNewLine(string? value)
10801080
{
10811081
if (value == null)
10821082
{

‎src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics;
5-
using System.Diagnostics.CodeAnalysis;
65

76
namespace System.Net.Http.Headers
87
{
@@ -102,6 +101,8 @@ public string? From
102101
value = null;
103102
}
104103

104+
CheckContainsNewLine(value);
105+
105106
SetOrRemoveParsedValue(KnownHeaders.From.Descriptor, value);
106107
}
107108
}

‎src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderParser.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ namespace System.Net.Http.Headers
77
{
88
internal sealed class MediaTypeHeaderParser : BaseHeaderParser
99
{
10-
private readonly bool _supportsMultipleValues;
1110
private readonly Func<MediaTypeHeaderValue> _mediaTypeCreator;
1211

1312
internal static readonly MediaTypeHeaderParser SingleValueParser = new MediaTypeHeaderParser(false, CreateMediaType);
@@ -18,8 +17,6 @@ private MediaTypeHeaderParser(bool supportsMultipleValues, Func<MediaTypeHeaderV
1817
: base(supportsMultipleValues)
1918
{
2019
Debug.Assert(mediaTypeCreator != null);
21-
22-
_supportsMultipleValues = supportsMultipleValues;
2320
_mediaTypeCreator = mediaTypeCreator;
2421
}
2522

‎src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Collections.Generic;
54
using System.Diagnostics;
65
using System.Diagnostics.CodeAnalysis;
7-
using System.IO;
86
using System.Text;
97

108
namespace System.Net.Http.Headers
@@ -362,18 +360,24 @@ private static void CheckValueFormat(string? value)
362360
// Trailing/leading space are not allowed
363361
if (value[0] == ' ' || value[0] == '\t' || value[^1] == ' ' || value[^1] == '\t')
364362
{
365-
throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
363+
ThrowFormatException(value);
366364
}
367365

368-
// If it's not a token we check if it's a valid quoted string
369-
if (HttpRuleParser.GetTokenLength(value, 0) == 0)
366+
if (value[0] == '"')
370367
{
371368
HttpParseResult parseResult = HttpRuleParser.GetQuotedStringLength(value, 0, out int valueLength);
372-
if ((parseResult == HttpParseResult.Parsed && valueLength != value.Length) || parseResult != HttpParseResult.Parsed)
369+
if (parseResult != HttpParseResult.Parsed || valueLength != value.Length)
373370
{
374-
throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
371+
ThrowFormatException(value);
375372
}
376373
}
374+
else if (HttpRuleParser.ContainsNewLine(value))
375+
{
376+
ThrowFormatException(value);
377+
}
378+
379+
static void ThrowFormatException(string value) =>
380+
throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, value));
377381
}
378382

379383
private static NameValueHeaderValue CreateNameValue()

‎src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics;
5-
using System.Globalization;
65
using System.Text;
76

87
namespace System.Net.Http
@@ -141,20 +140,6 @@ internal static int GetWhitespaceLength(string input, int startIndex)
141140
continue;
142141
}
143142

144-
if (c == '\r')
145-
{
146-
// If we have a #13 char, it must be followed by #10 and then at least one SP or HT.
147-
if ((current + 2 < input.Length) && (input[current + 1] == '\n'))
148-
{
149-
char spaceOrTab = input[current + 2];
150-
if ((spaceOrTab == ' ') || (spaceOrTab == '\t'))
151-
{
152-
current += 3;
153-
continue;
154-
}
155-
}
156-
}
157-
158143
return current - startIndex;
159144
}
160145

@@ -298,7 +283,7 @@ internal static HttpParseResult GetQuotedPairLength(string input, int startIndex
298283
}
299284

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

325+
char c = input[current];
326+
327+
if (c == '\r' || c == '\n')
328+
{
329+
return HttpParseResult.InvalidFormat;
330+
}
331+
340332
// If we support nested expressions and we find an open-char, then parse the nested expressions.
341-
if (supportsNesting && (input[current] == openChar))
333+
if (supportsNesting && (c == openChar))
342334
{
343335
// Check if we exceeded the number of nested calls.
344336
if (nestedCount > MaxNestedCount)

‎src/libraries/System.Net.Http/tests/UnitTests/Headers/AltSvcHeaderParserTest.cs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
54
using System.Collections.Generic;
6-
using System.Linq;
7-
using System.Text;
8-
using System.Threading.Tasks;
95
using System.Net.Http.Headers;
106
using Xunit;
11-
using Xunit.Sdk;
127

138
namespace System.Net.Http.Tests
149
{

‎src/libraries/System.Net.Http/tests/UnitTests/Headers/ByteArrayHeaderParserTest.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
5-
using System.Collections.Generic;
64
using System.Net.Http.Headers;
7-
using System.Text;
85

96
using Xunit;
107

‎src/libraries/System.Net.Http/tests/UnitTests/Headers/ContentDispositionHeaderValueTest.cs‎

Lines changed: 9 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
54
using System.Collections.Generic;
65
using System.Linq;
76
using System.Net.Http.Headers;
8-
using System.Text;
97

108
using Xunit;
119

@@ -487,8 +485,8 @@ public void GetDispositionTypeLength_DifferentValidScenarios_AllReturnNonZero()
487485
Assert.Equal("custom", value.Parameters.ElementAt(0).Name);
488486
Assert.Null(value.Parameters.ElementAt(0).Value);
489487

490-
Assert.Equal(40, ContentDispositionHeaderValue.GetDispositionTypeLength(
491-
"inline ; custom =\r\n \"x\" ; name = myName , next", 0, out result));
488+
Assert.Equal(38, ContentDispositionHeaderValue.GetDispositionTypeLength(
489+
"inline ; custom = \"x\" ; name = myName , next", 0, out result));
492490
value = (ContentDispositionHeaderValue)result;
493491
Assert.Equal("inline", value.DispositionType);
494492
Assert.Equal("myName", value.Name);
@@ -535,14 +533,14 @@ public void GetDispositionTypeLength_DifferentInvalidScenarios_AllReturnZero()
535533
public void Parse_SetOfValidValueStrings_ParsedCorrectly()
536534
{
537535
ContentDispositionHeaderValue expected = new ContentDispositionHeaderValue("inline");
538-
CheckValidParse("\r\n inline ", expected);
536+
CheckValidParse(" inline ", expected);
539537
CheckValidParse("inline", expected);
540538

541539
// We don't have to test all possible input strings, since most of the pieces are handled by other parsers.
542540
// The purpose of this test is to verify that these other parsers are combined correctly to build a
543541
// Content-Disposition parser.
544542
expected.Name = "myName";
545-
CheckValidParse("\r\n inline ; name = myName ", expected);
543+
CheckValidParse(" inline ; name = myName ", expected);
546544
CheckValidParse(" inline;name=myName", expected);
547545

548546
expected.Name = null;
@@ -565,35 +563,7 @@ public void Parse_SetOfInvalidValueStrings_Throws()
565563
CheckInvalidParse("inline; name=myName,");
566564
CheckInvalidParse("inline; name=my\u4F1AName");
567565
CheckInvalidParse("inline/");
568-
}
569-
570-
[Fact]
571-
public void TryParse_SetOfValidValueStrings_ParsedCorrectly()
572-
{
573-
ContentDispositionHeaderValue expected = new ContentDispositionHeaderValue("inline");
574-
CheckValidTryParse("\r\n inline ", expected);
575-
CheckValidTryParse("inline", expected);
576-
577-
// We don't have to test all possible input strings, since most of the pieces are handled by other parsers.
578-
// The purpose of this test is to verify that these other parsers are combined correctly to build a
579-
// Content-Disposition parser.
580-
expected.Name = "myName";
581-
CheckValidTryParse("\r\n inline ; name = myName ", expected);
582-
CheckValidTryParse(" inline;name=myName", expected);
583-
}
584-
585-
[Fact]
586-
public void TryParse_SetOfInvalidValueStrings_ReturnsFalse()
587-
{
588-
CheckInvalidTryParse("");
589-
CheckInvalidTryParse(" ");
590-
CheckInvalidTryParse(null);
591-
CheckInvalidTryParse("inline\u4F1A");
592-
CheckInvalidTryParse("inline ,");
593-
CheckInvalidTryParse("inline,");
594-
CheckInvalidTryParse("inline; name=myName ,");
595-
CheckInvalidTryParse("inline; name=myName,");
596-
CheckInvalidTryParse("text/");
566+
CheckInvalidParse(" inline ; \r name = myName ");
597567
}
598568

599569
#region Tests from HenrikN
@@ -1209,24 +1179,16 @@ private void CheckValidParse(string input, ContentDispositionHeaderValue expecte
12091179
{
12101180
ContentDispositionHeaderValue result = ContentDispositionHeaderValue.Parse(input);
12111181
Assert.Equal(expectedResult, result);
1212-
}
1213-
1214-
private void CheckInvalidParse(string input)
1215-
{
1216-
Assert.Throws<FormatException>(() => { ContentDispositionHeaderValue.Parse(input); });
1217-
}
12181182

1219-
private void CheckValidTryParse(string input, ContentDispositionHeaderValue expectedResult)
1220-
{
1221-
ContentDispositionHeaderValue result = null;
12221183
Assert.True(ContentDispositionHeaderValue.TryParse(input, out result), input);
12231184
Assert.Equal(expectedResult, result);
12241185
}
12251186

1226-
private void CheckInvalidTryParse(string input)
1187+
private void CheckInvalidParse(string input)
12271188
{
1228-
ContentDispositionHeaderValue result = null;
1229-
Assert.False(ContentDispositionHeaderValue.TryParse(input, out result), input);
1189+
Assert.Throws<FormatException>(() => { ContentDispositionHeaderValue.Parse(input); });
1190+
1191+
Assert.False(ContentDispositionHeaderValue.TryParse(input, out ContentDispositionHeaderValue result), input);
12301192
Assert.Null(result);
12311193
}
12321194

‎src/libraries/System.Net.Http/tests/UnitTests/Headers/DateHeaderParserTest.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
5-
using System.Collections.Generic;
6-
using System.Linq;
74
using System.Net.Http.Headers;
8-
using System.Text;
95

106
using Xunit;
117

0 commit comments

Comments
 (0)