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

Commit 0581bcf

Browse files
dougbudougbu
committed
Update MediaTypeHeaderValue.IsSubsetOf() to perform consistent checks
- aspnet/Mvc#3138 part 1/2 - check parameters with same polarity as type and subtype - ignore quality factors - bug was obscured because MVC has no formatters supporting wildcard media types nits: - add doc comments - spelling - correct typo in a `project.json` file
1 parent a0b29e3 commit 0581bcf

File tree

3 files changed

+65
-25
lines changed

3 files changed

+65
-25
lines changed

‎src/Microsoft.Net.Http.Headers/MediaTypeHeaderValue.cs

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,29 @@ public bool IsReadOnly
219219
get { return _isReadOnly; }
220220
}
221221

222+
/// <summary>
223+
/// Gets a value indicating whether this <see cref="MediaTypeHeaderValue"/> is a subset of
224+
/// <paramref name="otherMediaType"/>. A "subset" is defined as the same or a more specific media type
225+
/// according to the precedence described in https://www.ietf.org/rfc/rfc2068.txt section 14.1, Accept.
226+
/// </summary>
227+
/// <param name="otherMediaType">The <see cref="MediaTypeHeaderValue"/> to compare.</param>
228+
/// <returns>
229+
/// A value indicating whether this <see cref="MediaTypeHeaderValue"/> is a subset of
230+
/// <paramref name="otherMediaType"/>.
231+
/// </returns>
232+
/// <remarks>
233+
/// For example "multipart/mixed; boundary=1234" is a subset of "multipart/mixed; boundary=1234",
234+
/// "multipart/mixed", "multipart/*", and "*/*" but not "multipart/mixed; boundary=2345" or
235+
/// "multipart/message; boundary=1234".
236+
/// </remarks>
222237
public bool IsSubsetOf(MediaTypeHeaderValue otherMediaType)
223238
{
224239
if (otherMediaType == null)
225240
{
226241
return false;
227242
}
228243

244+
// "text/plain" is a subset of "text/plain", "text/*" and "*/*". "*/*" is a subset only of "*/*".
229245
if (!Type.Equals(otherMediaType.Type, StringComparison.OrdinalIgnoreCase))
230246
{
231247
if (!otherMediaType.MatchesAllTypes)
@@ -241,22 +257,29 @@ public bool IsSubsetOf(MediaTypeHeaderValue otherMediaType)
241257
}
242258
}
243259

244-
if (Parameters != null)
260+
// "text/plain; charset=utf-8; level=1" is a subset of "text/plain; charset=utf-8". In turn
261+
// "text/plain; charset=utf-8" is a subset of "text/plain".
262+
if (otherMediaType._parameters != null && otherMediaType._parameters.Count != 0)
245263
{
246-
if (Parameters.Count != 0 && (otherMediaType.Parameters == null || otherMediaType.Parameters.Count == 0))
264+
// Make sure all parameters in the potential superset are included locally. Fine to have additional
265+
// parameters locally; they make this one more specific.
266+
foreach (var parameter in otherMediaType._parameters)
247267
{
248-
return false;
249-
}
268+
if (string.Equals(parameter.Name, "q", StringComparison.OrdinalIgnoreCase))
269+
{
270+
// "q" and later parameters are not involved in media type matching. Quoting the RFC: The first
271+
// "q" parameter (if any) separates the media-range parameter(s) from the accept-params.
272+
break;
273+
}
250274

251-
// Make sure all parameters listed locally are listed in the other one. The other one may have additional parameters.
252-
foreach (var param in _parameters)
253-
{
254-
var otherParam = NameValueHeaderValue.Find(otherMediaType._parameters, param.Name);
255-
if (otherParam == null)
275+
var localParameter = NameValueHeaderValue.Find(_parameters, parameter.Name);
276+
if (localParameter == null)
256277
{
278+
// Not found.
257279
return false;
258280
}
259-
if (!string.Equals(param.Value, otherParam.Value, StringComparison.OrdinalIgnoreCase))
281+
282+
if (!string.Equals(parameter.Value, localParameter.Value, StringComparison.OrdinalIgnoreCase))
260283
{
261284
return false;
262285
}
@@ -364,7 +387,7 @@ private static int GetMediaTypeLength(string input, int startIndex, out MediaTyp
364387
return 0;
365388
}
366389

367-
// Caller must remove leading whitespaces. If not, we'll return 0.
390+
// Caller must remove leading whitespace. If not, we'll return 0.
368391
string mediaType = null;
369392
var mediaTypeLength = MediaTypeHeaderValue.GetMediaTypeExpressionLength(input, startIndex, out mediaType);
370393

@@ -432,7 +455,7 @@ private static int GetMediaTypeExpressionLength(string input, int startIndex, ou
432455
return 0;
433456
}
434457

435-
// If there are no whitespaces between <type> and <subtype> in <type>/<subtype> get the media type using
458+
// If there is no whitespace between <type> and <subtype> in <type>/<subtype> get the media type using
436459
// one Substring call. Otherwise get substrings for <type> and <subtype> and combine them.
437460
var mediatTypeLength = current + subtypeLength - startIndex;
438461
if (typeLength + subtypeLength + 1 == mediatTypeLength)
@@ -454,8 +477,8 @@ private static void CheckMediaTypeFormat(string mediaType, string parameterName)
454477
throw new ArgumentException("An empty string is not allowed.", parameterName);
455478
}
456479

457-
// When adding values using strongly typed objects, no leading/trailing LWS (whitespaces) are allowed.
458-
// Also no LWS between type and subtype are allowed.
480+
// When adding values using strongly typed objects, no leading/trailing LWS (whitespace) is allowed.
481+
// Also no LWS between type and subtype is allowed.
459482
string tempMediaType;
460483
var mediaTypeLength = GetMediaTypeExpressionLength(mediaType, 0, out tempMediaType);
461484
if ((mediaTypeLength == 0) || (tempMediaType.Length != mediaType.Length))

‎test/Microsoft.Extensions.BufferedHtmlContent.Test/Microsoft.Extensions.BufferedHtmlContent.Test.xproj

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<?xml version="1.0" encoding="utf-8"?>
1+
<?xml version="1.0" encoding="utf-8"?>
22
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
33
<PropertyGroup>
44
<VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">14.0</VisualStudioVersion>
@@ -11,9 +11,11 @@
1111
<BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath>
1212
<OutputPath Condition="'$(OutputPath)'=='' ">..\..\artifacts\bin\$(MSBuildProjectName)\</OutputPath>
1313
</PropertyGroup>
14-
1514
<PropertyGroup>
1615
<SchemaVersion>2.0</SchemaVersion>
1716
</PropertyGroup>
17+
<ItemGroup>
18+
<Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
19+
</ItemGroup>
1820
<Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.targets" Condition="'$(VSToolsPath)' != ''" />
1921
</Project>

‎test/Microsoft.Net.Http.Headers.Tests/MediaTypeHeaderValueTest.cs

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -542,13 +542,16 @@ public void TryParseList_WithSomeInvlaidValues_ReturnsFalse()
542542

543543
[Theory]
544544
[InlineData("*/*;", "*/*")]
545-
[InlineData("text/*;", "text/*")]
545+
[InlineData("text/*", "text/*")]
546+
[InlineData("text/*;", "*/*")]
546547
[InlineData("text/plain;", "text/plain")]
547-
[InlineData("*/*;", "*/*;charset=utf-8;")]
548-
[InlineData("text/*;", "*/*;charset=utf-8;")]
549-
[InlineData("text/plain;", "*/*;charset=utf-8;")]
550-
[InlineData("text/plain;", "text/*;charset=utf-8;")]
551-
[InlineData("text/plain;", "text/plain;charset=utf-8;")]
548+
[InlineData("text/plain", "text/*")]
549+
[InlineData("text/plain;", "*/*")]
550+
[InlineData("*/*;missingparam=4", "*/*")]
551+
[InlineData("text/*;missingparam=4;", "*/*;")]
552+
[InlineData("text/plain;missingparam=4", "*/*;")]
553+
[InlineData("text/plain;missingparam=4", "text/*")]
554+
[InlineData("text/plain;charset=utf-8", "text/plain;charset=utf-8")]
552555
[InlineData("text/plain;version=v1", "Text/plain;Version=v1")]
553556
[InlineData("text/plain;version=v1", "tExT/plain;version=V1")]
554557
[InlineData("text/plain;version=v1", "TEXT/PLAIN;VERSION=V1")]
@@ -558,26 +561,38 @@ public void TryParseList_WithSomeInvlaidValues_ReturnsFalse()
558561
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "*/*;charset=utf-8;foo=bar;q=0.0")]
559562
public void IsSubsetOf_PositiveCases(string mediaType1, string mediaType2)
560563
{
564+
// Arrange
561565
var parsedMediaType1 = MediaTypeHeaderValue.Parse(mediaType1);
562566
var parsedMediaType2 = MediaTypeHeaderValue.Parse(mediaType2);
563567

568+
// Act
564569
var isSubset = parsedMediaType1.IsSubsetOf(parsedMediaType2);
570+
571+
// Assert
565572
Assert.True(isSubset);
566573
}
567574

568575
[Theory]
576+
[InlineData("application/html", "text/*")]
577+
[InlineData("application/json", "application/html")]
569578
[InlineData("text/plain;version=v1", "text/plain;version=")]
570579
[InlineData("*/*;", "text/plain;charset=utf-8;foo=bar;q=0.0")]
571580
[InlineData("text/*;", "text/plain;charset=utf-8;foo=bar;q=0.0")]
572-
[InlineData("text/plain;missingparam=4;", "text/plain;charset=utf-8;foo=bar;q=0.0")]
573-
[InlineData("text/plain;missingparam=4;", "text/*;charset=utf-8;foo=bar;q=0.0")]
574-
[InlineData("text/plain;missingparam=4;", "*/*;charset=utf-8;foo=bar;q=0.0")]
581+
[InlineData("text/*;charset=utf-8;foo=bar;q=0.0", "text/plain;missingparam=4;")]
582+
[InlineData("*/*;charset=utf-8;foo=bar;q=0.0", "text/plain;missingparam=4;")]
583+
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "text/plain;missingparam=4;")]
584+
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "text/*;missingparam=4;")]
585+
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "*/*;missingparam=4;")]
575586
public void IsSubsetOf_NegativeCases(string mediaType1, string mediaType2)
576587
{
588+
// Arrange
577589
var parsedMediaType1 = MediaTypeHeaderValue.Parse(mediaType1);
578590
var parsedMediaType2 = MediaTypeHeaderValue.Parse(mediaType2);
579591

592+
// Act
580593
var isSubset = parsedMediaType1.IsSubsetOf(parsedMediaType2);
594+
595+
// Assert
581596
Assert.False(isSubset);
582597
}
583598

0 commit comments

Comments
 (0)