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

Update MediaTypeHeaderValue.IsSubsetOf() to perform consistent checks #438

Closed
wants to merge 1 commit into from
Closed

Update MediaTypeHeaderValue.IsSubsetOf() to perform consistent checks #438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 37 additions & 14 deletions src/Microsoft.Net.Http.Headers/MediaTypeHeaderValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,13 +219,29 @@ public bool IsReadOnly
get { return _isReadOnly; }
}

/// <summary>
/// Gets a value indicating whether this <see cref="MediaTypeHeaderValue"/> is a subset of
/// <paramref name="otherMediaType"/>. A "subset" is defined as the same or a more specific media type
/// according to the precedence described in https://www.ietf.org/rfc/rfc2068.txt section 14.1, Accept.
/// </summary>
/// <param name="otherMediaType">The <see cref="MediaTypeHeaderValue"/> to compare.</param>
/// <returns>
/// A value indicating whether this <see cref="MediaTypeHeaderValue"/> is a subset of
/// <paramref name="otherMediaType"/>.
/// </returns>
/// <remarks>
/// For example "multipart/mixed; boundary=1234" is a subset of "multipart/mixed; boundary=1234",
/// "multipart/mixed", "multipart/*", and "*/*" but not "multipart/mixed; boundary=2345" or
/// "multipart/message; boundary=1234".
/// </remarks>
public bool IsSubsetOf(MediaTypeHeaderValue otherMediaType)
{
if (otherMediaType == null)
{
return false;
}

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

if (Parameters != null)
// "text/plain; charset=utf-8; level=1" is a subset of "text/plain; charset=utf-8". In turn
// "text/plain; charset=utf-8" is a subset of "text/plain".
if (otherMediaType._parameters != null && otherMediaType.Parameters.Count != 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird to mix fields and properties here. Pick one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🆗 will use the field because first reference to the property will allocate.

{
if (Parameters.Count != 0 && (otherMediaType.Parameters == null || otherMediaType.Parameters.Count == 0))
// Make sure all parameters in the potential superset are included locally. Fine to have additional
// parameters locally; they make this one more specific.
foreach (var parameter in otherMediaType.Parameters)
{
return false;
}
if (string.Equals(parameter.Name, "q", StringComparison.OrdinalIgnoreCase))
{
// "q" and later parameters are not involved in media type matching. Quoting the RFC: The first
// "q" parameter (if any) separates the media-range parameter(s) from the accept-params.
break;
}

// Make sure all parameters listed locally are listed in the other one. The other one may have additional parameters.
foreach (var param in _parameters)
{
var otherParam = NameValueHeaderValue.Find(otherMediaType._parameters, param.Name);
if (otherParam == null)
var localParameter = NameValueHeaderValue.Find(_parameters, parameter.Name);
if (localParameter == null)
{
// Not found.
return false;
}
if (!string.Equals(param.Value, otherParam.Value, StringComparison.OrdinalIgnoreCase))

if (!string.Equals(parameter.Value, localParameter.Value, StringComparison.OrdinalIgnoreCase))
{
return false;
}
Expand Down Expand Up @@ -364,7 +387,7 @@ private static int GetMediaTypeLength(string input, int startIndex, out MediaTyp
return 0;
}

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

Expand Down Expand Up @@ -432,7 +455,7 @@ private static int GetMediaTypeExpressionLength(string input, int startIndex, ou
return 0;
}

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

// When adding values using strongly typed objects, no leading/trailing LWS (whitespaces) are allowed.
// Also no LWS between type and subtype are allowed.
// When adding values using strongly typed objects, no leading/trailing LWS (whitespace) is allowed.
// Also no LWS between type and subtype is allowed.
string tempMediaType;
var mediaTypeLength = GetMediaTypeExpressionLength(mediaType, 0, out tempMediaType);
if ((mediaTypeLength == 0) || (tempMediaType.Length != mediaType.Length))
Expand Down
6 changes: 4 additions & 2 deletions ...t.Extensions.BufferedHtmlContent.Test/Microsoft.Extensions.BufferedHtmlContent.Test.xproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">14.0</VisualStudioVersion>
Expand All @@ -11,9 +11,11 @@
<BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath>
<OutputPath Condition="'$(OutputPath)'=='' ">..\..\artifacts\bin\$(MSBuildProjectName)\</OutputPath>
</PropertyGroup>

<PropertyGroup>
<SchemaVersion>2.0</SchemaVersion>
</PropertyGroup>
<ItemGroup>
<Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
</ItemGroup>
<Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.targets" Condition="'$(VSToolsPath)' != ''" />
</Project>
33 changes: 24 additions & 9 deletions test/Microsoft.Net.Http.Headers.Tests/MediaTypeHeaderValueTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,13 +542,16 @@ public void TryParseList_WithSomeInvlaidValues_ReturnsFalse()

[Theory]
[InlineData("*/*;", "*/*")]
[InlineData("text/*;", "text/*")]
[InlineData("text/*", "text/*")]
[InlineData("text/*;", "*/*")]
[InlineData("text/plain;", "text/plain")]
[InlineData("*/*;", "*/*;charset=utf-8;")]
[InlineData("text/*;", "*/*;charset=utf-8;")]
[InlineData("text/plain;", "*/*;charset=utf-8;")]
[InlineData("text/plain;", "text/*;charset=utf-8;")]
[InlineData("text/plain;", "text/plain;charset=utf-8;")]
[InlineData("text/plain", "text/*")]
[InlineData("text/plain;", "*/*")]
[InlineData("*/*;missingparam=4", "*/*")]
[InlineData("text/*;missingparam=4;", "*/*;")]
[InlineData("text/plain;missingparam=4", "*/*;")]
[InlineData("text/plain;missingparam=4", "text/*")]
[InlineData("text/plain;charset=utf-8", "text/plain;charset=utf-8")]
[InlineData("text/plain;version=v1", "Text/plain;Version=v1")]
[InlineData("text/plain;version=v1", "tExT/plain;version=V1")]
[InlineData("text/plain;version=v1", "TEXT/PLAIN;VERSION=V1")]
Expand All @@ -558,26 +561,38 @@ public void TryParseList_WithSomeInvlaidValues_ReturnsFalse()
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "*/*;charset=utf-8;foo=bar;q=0.0")]
public void IsSubsetOf_PositiveCases(string mediaType1, string mediaType2)
{
// Arrange
var parsedMediaType1 = MediaTypeHeaderValue.Parse(mediaType1);
var parsedMediaType2 = MediaTypeHeaderValue.Parse(mediaType2);

// Act
var isSubset = parsedMediaType1.IsSubsetOf(parsedMediaType2);

// Assert
Assert.True(isSubset);
}

[Theory]
[InlineData("application/html", "text/*")]
[InlineData("application/json", "application/html")]
[InlineData("text/plain;version=v1", "text/plain;version=")]
[InlineData("*/*;", "text/plain;charset=utf-8;foo=bar;q=0.0")]
[InlineData("text/*;", "text/plain;charset=utf-8;foo=bar;q=0.0")]
[InlineData("text/plain;missingparam=4;", "text/plain;charset=utf-8;foo=bar;q=0.0")]
[InlineData("text/plain;missingparam=4;", "text/*;charset=utf-8;foo=bar;q=0.0")]
[InlineData("text/plain;missingparam=4;", "*/*;charset=utf-8;foo=bar;q=0.0")]
[InlineData("text/*;charset=utf-8;foo=bar;q=0.0", "text/plain;missingparam=4;")]
[InlineData("*/*;charset=utf-8;foo=bar;q=0.0", "text/plain;missingparam=4;")]
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "text/plain;missingparam=4;")]
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "text/*;missingparam=4;")]
[InlineData("text/plain;charset=utf-8;foo=bar;q=0.0", "*/*;missingparam=4;")]
public void IsSubsetOf_NegativeCases(string mediaType1, string mediaType2)
{
// Arrange
var parsedMediaType1 = MediaTypeHeaderValue.Parse(mediaType1);
var parsedMediaType2 = MediaTypeHeaderValue.Parse(mediaType2);

// Act
var isSubset = parsedMediaType1.IsSubsetOf(parsedMediaType2);

// Assert
Assert.False(isSubset);
}

Expand Down