Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit 9579806

Browse files
javiercnjaviercn
committed
[Fixes #5150] parsing issue on asp.net when request quality factor is specified
1 parent 38aa486 commit 9579806

File tree

4 files changed

+101
-18
lines changed

4 files changed

+101
-18
lines changed

‎src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,12 @@ public static Encoding GetEncoding(StringSegment mediaType)
292292
public static MediaTypeSegmentWithQuality CreateMediaTypeSegmentWithQuality(string mediaType, int start)
293293
{
294294
var parsedMediaType = new MediaType(mediaType, start, length: null);
295+
if (parsedMediaType.Type.Equals(default(StringSegment)) ||
296+
parsedMediaType.SubType.Equals(default(StringSegment)))
297+
{
298+
return default(MediaTypeSegmentWithQuality);
299+
}
300+
295301
var parser = parsedMediaType._parameterParser;
296302

297303
double quality = 1.0d;
@@ -306,9 +312,9 @@ public static MediaTypeSegmentWithQuality CreateMediaTypeSegmentWithQuality(stri
306312
}
307313
}
308314

309-
// We check if the parsed media type has value at this stage when we have iterated
315+
// We check if the parsed media type has a value at this stage when we have iterated
310316
// over all the parameters and we know if the parsing was sucessful.
311-
if (!parser.ParsingFailed && parser.CurrentOffset >= start)
317+
if (!parser.ParsingFailed)
312318
{
313319
return new MediaTypeSegmentWithQuality(
314320
new StringSegment(mediaType, start, parser.CurrentOffset - start),
@@ -395,6 +401,7 @@ public bool ParseNextParameter(out MediaTypeParameter result)
395401
{
396402
if (_mediaTypeBuffer == null)
397403
{
404+
ParsingFailed = true;
398405
result = default(MediaTypeParameter);
399406
return false;
400407
}

‎src/Microsoft.AspNetCore.Mvc.Core/Internal/AcceptHeaderParser.cs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ public static void ParseAcceptHeader(IList<string> acceptHeaders, IList<MediaTyp
2929
{
3030
throw new ArgumentNullException(nameof(parsedValues));
3131
}
32-
3332
for (var i = 0; i < acceptHeaders.Count; i++)
3433
{
3534
var charIndex = 0;
@@ -46,13 +45,6 @@ public static void ParseAcceptHeader(IList<string> acceptHeaders, IList<MediaTyp
4645
parsedValues.Add(output);
4746
}
4847
}
49-
else
50-
{
51-
var invalidValuesError = Resources.FormatAcceptHeaderParser_ParseAcceptHeader_InvalidValues(
52-
value.Substring(charIndex));
53-
54-
throw new FormatException(invalidValuesError);
55-
}
5648
}
5749
}
5850
}
@@ -80,11 +72,34 @@ private static bool TryParseValue(string value, ref int index, out MediaTypeSegm
8072
return true;
8173
}
8274

83-
MediaTypeSegmentWithQuality result;
84-
var length = GetMediaTypeWithQualityLength(value, currentIndex, out result);
75+
// We deliberately want to ignore media types that we are not capable of parsing.
76+
// This is due to the fact that some browsers will send invalid media types like
77+
// ; q=0.9 or */;q=0.2, etc.
78+
// In this scenario, our recovery action consists of advancing the pointer to the
79+
// next separator and moving on.
80+
// In case we don't find the next separator, we simply advance the cursor to the
81+
// end of the string to signal that we are done parsing.
82+
var result = default(MediaTypeSegmentWithQuality);
83+
var length = 0;
84+
try
85+
{
86+
length = GetMediaTypeWithQualityLength(value, currentIndex, out result);
87+
}
88+
catch
89+
{
90+
length = 0;
91+
}
8592

8693
if (length == 0)
8794
{
95+
// The parsing failed.
96+
currentIndex = value.IndexOf(',', currentIndex);
97+
if (currentIndex == -1)
98+
{
99+
index = value.Length;
100+
return false;
101+
}
102+
index = currentIndex;
88103
return false;
89104
}
90105

‎test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AcceptHeaderParserTest.cs

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Linq;
67
using Microsoft.Extensions.Primitives;
78
using Xunit;
89

@@ -54,14 +55,54 @@ public void ParseAcceptHeader_ParsesSimpleHeaderWithMultipleValues_InvalidFormat
5455
{
5556
// Arrange
5657
var header = "application/json, application/xml,;q=0.8";
57-
var expectedException = "\"Invalid values ';q=0.8'.\"";
58+
var expectedMediaTypes = new List<MediaTypeSegmentWithQuality>
59+
{
60+
new MediaTypeSegmentWithQuality(new StringSegment("application/json"),1.0),
61+
new MediaTypeSegmentWithQuality(new StringSegment("application/xml"),1.0),
62+
};
63+
64+
// Act
65+
var mediaTypes = AcceptHeaderParser.ParseAcceptHeader(new List<string> { header });
66+
67+
// Assert
68+
Assert.Equal(expectedMediaTypes, mediaTypes);
69+
}
70+
71+
public static TheoryData<string[], string[]> ParseAcceptHeaderWithInvalidMediaTypesData =>
72+
new TheoryData<string[], string[]>
73+
{
74+
{ new [] { ";q=0.9" }, new string[] { } },
75+
{ new [] { "/" }, new string[] { } },
76+
{ new [] { "*/" }, new string[] { } },
77+
{ new [] { "/*" }, new string[] { } },
78+
{ new [] { "/;q=0.9" }, new string[] { } },
79+
{ new [] { "*/;q=0.9" }, new string[] { } },
80+
{ new [] { "/*;q=0.9" }, new string[] { } },
81+
{ new [] { "/;q=0.9,text/html" }, new string[] { "text/html" } },
82+
{ new [] { "*/;q=0.9,text/html" }, new string[] { "text/html" } },
83+
{ new [] { "/*;q=0.9,text/html" }, new string[] { "text/html" } },
84+
{ new [] { "img/png,/;q=0.9,text/html" }, new string[] { "img/png", "text/html" } },
85+
{ new [] { "img/png,*/;q=0.9,text/html" }, new string[] { "img/png", "text/html" } },
86+
{ new [] { "img/png,/*;q=0.9,text/html" }, new string[] { "img/png", "text/html" } },
87+
{ new [] { "img/png, /;q=0.9" }, new string[] { "img/png", } },
88+
{ new [] { "img/png, */;q=0.9" }, new string[] { "img/png", } },
89+
{ new [] { "img/png;q=1.0, /*;q=0.9" }, new string[] { "img/png;q=1.0", } },
90+
};
91+
92+
[Theory]
93+
[MemberData(nameof(ParseAcceptHeaderWithInvalidMediaTypesData))]
94+
public void ParseAcceptHeader_GracefullyRecoversFromInvalidMediaTypeValues_AndReturnsValidMediaTypes(
95+
string[] acceptHeader,
96+
string[] expected)
97+
{
98+
// Arrange
99+
var expectedMediaTypes = expected.Select(e => new MediaTypeSegmentWithQuality(new StringSegment(e), 1.0)).ToList();
58100

59101
// Act
60-
var ex = Assert.Throws<FormatException>(
61-
() => { AcceptHeaderParser.ParseAcceptHeader(new List<string> { header }); });
102+
var parsed = AcceptHeaderParser.ParseAcceptHeader(acceptHeader);
62103

63104
// Assert
64-
Assert.Equal(expectedException, ex.Message);
105+
Assert.Equal(expectedMediaTypes, parsed);
65106
}
66107

67108
[Fact]
@@ -70,8 +111,8 @@ public void ParseAcceptHeader_ParsesMultipleHeaderValues()
70111
// Arrange
71112
var expected = new List<MediaTypeSegmentWithQuality>
72113
{
73-
new MediaTypeSegmentWithQuality(new StringSegment("application/json"),1.0),
74-
new MediaTypeSegmentWithQuality(new StringSegment("application/xml;q=0.8"),0.8)
114+
new MediaTypeSegmentWithQuality(new StringSegment("application/json"), 1.0),
115+
new MediaTypeSegmentWithQuality(new StringSegment("application/xml;q=0.8"), 0.8)
75116
};
76117

77118
// Act

‎test/Microsoft.AspNetCore.Mvc.FunctionalTests/ContentNegotiationTest.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,26 @@ public async Task NoProducesAttribute_ActionReturningAnyObject_RunsUsingDefaultF
8585
Assert.Equal(expectedContentType, response.Content.Headers.ContentType);
8686
}
8787

88+
[Theory]
89+
[InlineData("/;q=0.9")]
90+
[InlineData("/;q=0.9, invalid;q=0.5;application/json;q=0.1")]
91+
[InlineData("/invalid;q=0.9, application/json;q=0.1,invalid;q=0.5")]
92+
[InlineData("text/html, application/json, image/jpeg, *; q=.2, */*; q=.2")]
93+
public async Task ContentNegotiationWithPartiallyValidAcceptHeader_SkipsInvalidEntries(string acceptHeader)
94+
{
95+
// Arrange
96+
var expectedContentType = MediaTypeHeaderValue.Parse("application/json;charset=utf-8");
97+
var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/ContentNegotiation/UserInfo_ProducesWithTypeOnly");
98+
request.Headers.TryAddWithoutValidation("Accept", acceptHeader);
99+
100+
// Act
101+
var response = await Client.SendAsync(request);
102+
103+
// Assert
104+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
105+
Assert.Equal(expectedContentType, response.Content.Headers.ContentType);
106+
}
107+
88108
[Fact]
89109
public async Task ProducesAttributeWithTypeOnly_RunsRegularContentNegotiation()
90110
{

0 commit comments

Comments
 (0)