From 3fdcaecaa8ba6cfd5f9ff575935d3eda80f8ab1e Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Fri, 14 Oct 2016 19:02:07 -0700 Subject: [PATCH] Correctly handle quoted media type parameter values - #5349 - fix or add comments about other parsing errors and inconsistencies - `MediaType` did not skip whitespace before the type nits: - use `+=` - `` -> `` since the former is not for use within a paragraph - split tests up to remove `bool expectedResult` parameters --- .../Formatters/MediaType.cs | 119 ++++----- .../Formatters/MediaTypeTest.cs | 248 ++++++++++++------ 2 files changed, 221 insertions(+), 146 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs b/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs index e30b52408e..7aaa9715c4 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs @@ -5,7 +5,6 @@ using System; using System.Globalization; using System.Text; using Microsoft.AspNetCore.Mvc.Formatters.Internal; -using Microsoft.Extensions.Internal; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Mvc.Formatters @@ -103,19 +102,20 @@ namespace Microsoft.AspNetCore.Mvc.Formatters return 0; } - // Parse the type, i.e. in media type string "/; param1=value1; param2=value2" - var typeLength = HttpTokenParsingRules.GetTokenLength(input, offset); + var current = offset + HttpTokenParsingRules.GetWhitespaceLength(input, offset); + // Parse the type, i.e. in media type string "/; param1=value1; param2=value2" + var typeLength = HttpTokenParsingRules.GetTokenLength(input, current); if (typeLength == 0) { type = default(StringSegment); return 0; } - type = new StringSegment(input, offset, typeLength); + type = new StringSegment(input, current, typeLength); - var current = offset + typeLength; - current = current + HttpTokenParsingRules.GetWhitespaceLength(input, current); + current += typeLength; + current += HttpTokenParsingRules.GetWhitespaceLength(input, current); return current - offset; } @@ -123,6 +123,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters private static int GetSubtypeLength(string input, int offset, out StringSegment subType) { var current = offset; + // Parse the separator between type and subtype if (current < 0 || current >= input.Length || input[current] != '/') { @@ -131,10 +132,9 @@ namespace Microsoft.AspNetCore.Mvc.Formatters } current++; // skip delimiter. - current = current + HttpTokenParsingRules.GetWhitespaceLength(input, current); + current += HttpTokenParsingRules.GetWhitespaceLength(input, current); var subtypeLength = HttpTokenParsingRules.GetTokenLength(input, current); - if (subtypeLength == 0) { subType = default(StringSegment); @@ -143,8 +143,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters subType = new StringSegment(input, current, subtypeLength); - current = current + subtypeLength; - current = current + HttpTokenParsingRules.GetWhitespaceLength(input, current); + current += subtypeLength; + current += HttpTokenParsingRules.GetWhitespaceLength(input, current); return current - offset; } @@ -180,11 +180,12 @@ namespace Microsoft.AspNetCore.Mvc.Formatters public StringSegment Charset => GetParameter("charset"); /// - /// Determines whether the current is a subset of the . + /// Determines whether the current is a subset of the + /// . /// /// The set . /// - /// true if this is a subset of ; otherwisefalse. + /// true if this is a subset of ; otherwise false. /// public bool IsSubsetOf(MediaType set) { @@ -197,7 +198,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// Gets the parameter of the media type. /// /// The name of the parameter to retrieve. - /// The for the given if found; otherwisenull. + /// + /// The for the given if found; otherwise + /// null. + /// public StringSegment GetParameter(string parameterName) { return GetParameter(new StringSegment(parameterName)); @@ -207,7 +211,10 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// Gets the parameter of the media type. /// /// The name of the parameter to retrieve. - /// The for the given if found; otherwisenull. + /// + /// The for the given if found; otherwise + /// null. + /// public StringSegment GetParameter(StringSegment parameterName) { var parametersParser = _parameterParser; @@ -229,7 +236,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// . /// /// The media type whose encoding will be replaced. - /// The encoding that will replace the encoding in the + /// The encoding that will replace the encoding in the . + /// /// A media type with the replaced encoding. public static string ReplaceEncoding(string mediaType, Encoding encoding) { @@ -241,15 +249,15 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// . /// /// The media type whose encoding will be replaced. - /// The encoding that will replace the encoding in the + /// The encoding that will replace the encoding in the . + /// /// A media type with the replaced encoding. public static string ReplaceEncoding(StringSegment mediaType, Encoding encoding) { var parsedMediaType = new MediaType(mediaType); var charset = parsedMediaType.GetParameter("charset"); - if (charset.HasValue && - charset.Equals(encoding.WebName, StringComparison.OrdinalIgnoreCase)) + if (charset.HasValue && charset.Equals(encoding.WebName, StringComparison.OrdinalIgnoreCase)) { return mediaType.Value; } @@ -292,6 +300,9 @@ namespace Microsoft.AspNetCore.Mvc.Formatters public static MediaTypeSegmentWithQuality CreateMediaTypeSegmentWithQuality(string mediaType, int start) { var parsedMediaType = new MediaType(mediaType, start, length: null); + + // Short-circuit use of the MediaTypeParameterParser if constructor detected an invalid type or subtype. + // Parser would set ParsingFailed==true in this case. But, we handle invalid parameters as a separate case. if (parsedMediaType.Type.Equals(default(StringSegment)) || parsedMediaType.SubType.Equals(default(StringSegment))) { @@ -300,12 +311,13 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var parser = parsedMediaType._parameterParser; - double quality = 1.0d; + var quality = 1.0d; MediaTypeParameter parameter; while (parser.ParseNextParameter(out parameter)) { if (parameter.HasName(QualityParameter)) { + // If media type contains two `q` values i.e. it's invalid in an uncommon way, pick last value. quality = double.Parse( parameter.Value.Value, NumberStyles.AllowDecimalPoint, NumberFormatInfo.InvariantInfo); @@ -313,17 +325,15 @@ namespace Microsoft.AspNetCore.Mvc.Formatters } // We check if the parsed media type has a value at this stage when we have iterated - // over all the parameters and we know if the parsing was sucessful. - if (!parser.ParsingFailed) - { - return new MediaTypeSegmentWithQuality( - new StringSegment(mediaType, start, parser.CurrentOffset - start), - quality); - } - else + // over all the parameters and we know if the parsing was successful. + if (parser.ParsingFailed) { return default(MediaTypeSegmentWithQuality); } + + return new MediaTypeSegmentWithQuality( + new StringSegment(mediaType, start, parser.CurrentOffset - start), + quality); } private static Encoding GetEncodingFromCharset(StringSegment charset) @@ -407,7 +417,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters } var parameterLength = GetParameterLength(_mediaTypeBuffer, CurrentOffset, out result); - CurrentOffset = CurrentOffset + parameterLength; + CurrentOffset += parameterLength; if (parameterLength == 0) { @@ -420,8 +430,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters private static int GetParameterLength(string input, int startIndex, out MediaTypeParameter parsedValue) { - if (OffsetIsOutOfRange(startIndex, input.Length) || - input[startIndex] != ';') + if (OffsetIsOutOfRange(startIndex, input.Length) || input[startIndex] != ';') { parsedValue = default(MediaTypeParameter); return 0; @@ -442,8 +451,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var valueLength = GetValueLength(input, current, out value); parsedValue = new MediaTypeParameter(name, value); + current += valueLength; - current = current + valueLength; return current - startIndex; } @@ -452,10 +461,9 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var current = startIndex; current++; // skip ';' - current = current + HttpTokenParsingRules.GetWhitespaceLength(input, current); + current += HttpTokenParsingRules.GetWhitespaceLength(input, current); var nameLength = HttpTokenParsingRules.GetTokenLength(input, current); - if (nameLength == 0) { name = default(StringSegment); @@ -464,8 +472,9 @@ namespace Microsoft.AspNetCore.Mvc.Formatters name = new StringSegment(input, current, nameLength); - current = current + nameLength; - current = current + HttpTokenParsingRules.GetWhitespaceLength(input, current); + current += nameLength; + current += HttpTokenParsingRules.GetWhitespaceLength(input, current); + return current - startIndex; } @@ -474,25 +483,31 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var current = startIndex; current++; // skip '='. - current = current + HttpTokenParsingRules.GetWhitespaceLength(input, current); + current += HttpTokenParsingRules.GetWhitespaceLength(input, current); var valueLength = HttpTokenParsingRules.GetTokenLength(input, current); if (valueLength == 0) { // A value can either be a token or a quoted string. Check if it is a quoted string. - if (HttpTokenParsingRules.GetQuotedStringLength(input, current, out valueLength) != HttpParseResult.Parsed) + var result = HttpTokenParsingRules.GetQuotedStringLength(input, current, out valueLength); + if (result != HttpParseResult.Parsed) { // We have an invalid value. Reset the name and return. value = default(StringSegment); return 0; } + + // Quotation marks are not part of a quoted parameter value. + value = new StringSegment(input, current + 1, valueLength - 2); + } + else + { + value = new StringSegment(input, current, valueLength); } - value = new StringSegment(input, current, valueLength); - - current = current + valueLength; - current = current + HttpTokenParsingRules.GetWhitespaceLength(input, current); + current += valueLength; + current += HttpTokenParsingRules.GetWhitespaceLength(input, current); return current - startIndex; } @@ -530,27 +545,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters public bool Equals(MediaTypeParameter other) { - return HasName(other.Name) && - Value.Equals(other.Value, StringComparison.OrdinalIgnoreCase); - } - - /// - public override bool Equals(object obj) - { - if (ReferenceEquals(null, obj)) - { - return false; - } - - return obj is MediaTypeParameter && Equals((MediaTypeParameter)obj); - } - - public override int GetHashCode() - { - HashCodeCombiner hashCode = HashCodeCombiner.Start(); - hashCode.Add(Name.Value); - hashCode.Add(Value.Value); - return hashCode; + return HasName(other.Name) && Value.Equals(other.Value, StringComparison.OrdinalIgnoreCase); } public override string ToString() => $"{Name}={Value}"; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs index e3306bed1a..f79075f2e8 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Text; using Microsoft.Extensions.Primitives; using Xunit; @@ -11,7 +12,8 @@ namespace Microsoft.AspNetCore.Mvc.Formatters [Theory] [InlineData("application/json")] [InlineData("application /json")] - public void CanParse_ParameterlessMediaTypes(string mediaType) + [InlineData(" application / json ")] + public void Constructor_CanParseParameterlessMediaTypes(string mediaType) { // Arrange & Act var result = new MediaType(mediaType, 0, mediaType.Length); @@ -21,16 +23,30 @@ namespace Microsoft.AspNetCore.Mvc.Formatters Assert.Equal(new StringSegment("json"), result.SubType); } + public static TheoryData MediaTypesWithParameters + { + get + { + return new TheoryData + { + "application/json;format=pretty;charset=utf-8;q=0.8", + "application/json;format=pretty;charset=\"utf-8\";q=0.8", + "application/json;format=pretty;charset=utf-8; q=0.8 ", + "application/json;format=pretty;charset=utf-8 ; q=0.8 ", + "application/json;format=pretty; charset=utf-8 ; q=0.8 ", + "application/json;format=pretty ; charset=utf-8 ; q=0.8 ", + "application/json; format=pretty ; charset=utf-8 ; q=0.8 ", + "application/json; format=pretty ; charset=utf-8 ; q= 0.8 ", + "application/json; format=pretty ; charset=utf-8 ; q = 0.8 ", + " application / json; format = pretty ; charset = utf-8 ; q = 0.8 ", + " application / json; format = \"pretty\" ; charset = \"utf-8\" ; q = \"0.8\" ", + }; + } + } + [Theory] - [InlineData("application/json;format=pretty;charset=utf-8;q=0.8")] - [InlineData("application/json;format=pretty;charset=utf-8; q=0.8 ")] - [InlineData("application/json;format=pretty;charset=utf-8 ; q=0.8 ")] - [InlineData("application/json;format=pretty; charset=utf-8 ; q=0.8 ")] - [InlineData("application/json;format=pretty ; charset=utf-8 ; q=0.8 ")] - [InlineData("application/json; format=pretty ; charset=utf-8 ; q=0.8 ")] - [InlineData("application/json; format=pretty ; charset=utf-8 ; q= 0.8 ")] - [InlineData("application/json; format=pretty ; charset=utf-8 ; q = 0.8 ")] - public void CanParse_MediaTypesWithParameters(string mediaType) + [MemberData(nameof(MediaTypesWithParameters))] + public void Constructor_CanParseMediaTypesWithParameters(string mediaType) { // Arrange & Act var result = new MediaType(mediaType, 0, mediaType.Length); @@ -43,6 +59,21 @@ namespace Microsoft.AspNetCore.Mvc.Formatters Assert.Equal(new StringSegment("utf-8"), result.GetParameter("charset")); } + [Theory] + [MemberData(nameof(MediaTypesWithParameters))] + public void ReplaceEncoding_ReturnsExpectedMediaType(string mediaType) + { + // Arrange + var encoding = Encoding.GetEncoding("iso-8859-1"); + var expectedMediaType = mediaType.Replace("utf-8", "iso-8859-1"); + + // Act + var result = MediaType.ReplaceEncoding(mediaType, encoding); + + // Assert + Assert.Equal(expectedMediaType, result); + } + [Theory] [InlineData("application/json;charset=utf-8")] [InlineData("application/json;format=indent;q=0.8;charset=utf-8")] @@ -52,89 +83,15 @@ namespace Microsoft.AspNetCore.Mvc.Formatters { // Arrange var expectedParameter = new StringSegment("utf-8"); - var parsedMediaType = new MediaType(mediaType, 0, mediaType.Length); // Act var result = parsedMediaType.GetParameter("charset"); // Assert - Assert.NotNull(result); Assert.Equal(expectedParameter, result); } - [Fact] - public void GetParameter_IsCaseInsensitive() - { - // Arrange - var mediaType = "application/json;charset=utf-8"; - var expectedParameter = new StringSegment("utf-8"); - - var parsedMediaType = new MediaType(mediaType); - - // Act - var result = parsedMediaType.GetParameter("CHARSET"); - - // Assert - Assert.NotNull(result); - Assert.Equal(expectedParameter, result); - } - - [Theory] - [InlineData("application/json", "application/json", true)] - [InlineData("application/json", "application/json;charset=utf-8", true)] - [InlineData("application/json;charset=utf-8", "application/json", false)] - [InlineData("application/json;q=0.8", "application/json;q=0.9", true)] - [InlineData("application/json;q=0.8;charset=utf-7", "application/json;charset=utf-8;q=0.9", true)] - [InlineData("application/json;format=indent;charset=utf-8", "application/json", false)] - [InlineData("application/json", "application/json;format=indent;charset=utf-8", true)] - [InlineData("application/json;format=indent;charset=utf-8", "application/json;format=indent;charset=utf-8", true)] - [InlineData("application/json;charset=utf-8;format=indent", "application/json;format=indent;charset=utf-8", true)] - public void IsSubsetOf(string set, string subset, bool expectedResult) - { - // Arrange - var setMediaType = new MediaType(set); - var subSetMediaType = new MediaType(subset); - - // Act - var result = subSetMediaType.IsSubsetOf(setMediaType); - - // Assert - Assert.Equal(expectedResult, result); - } - - [Theory] - [InlineData("*/*", true)] - [InlineData("text/*", false)] - [InlineData("text/plain", false)] - public void MatchesAllTypes(string value, bool expectedResult) - { - // Arrange - var mediaType = new MediaType(value); - - // Act - var result = mediaType.MatchesAllTypes; - - // Assert - Assert.Equal(expectedResult, result); - } - - [Theory] - [InlineData("*/*", true)] - [InlineData("text/*", true)] - [InlineData("text/plain", false)] - public void MatchesAllSubtypes(string value, bool expectedResult) - { - // Arrange - var mediaType = new MediaType(value); - - // Act - var result = mediaType.MatchesAllSubTypes; - - // Assert - Assert.Equal(expectedResult, result); - } - [Fact] public void GetParameter_ReturnsNull_IfParameterIsNotInMediaType() { @@ -148,5 +105,128 @@ namespace Microsoft.AspNetCore.Mvc.Formatters // Assert Assert.False(result.HasValue); } + + [Fact] + public void GetParameter_IsCaseInsensitive() + { + // Arrange + var mediaType = "application/json;charset=utf-8"; + var expectedParameter = new StringSegment("utf-8"); + + var parsedMediaType = new MediaType(mediaType); + + // Act + var result = parsedMediaType.GetParameter("CHARSET"); + + // Assert + Assert.Equal(expectedParameter, result); + } + + [Theory] + [InlineData("application/json", "application/json")] + [InlineData("application/json", "application/json;charset=utf-8")] + [InlineData("application/json;q=0.8", "application/json;q=0.9")] + [InlineData("application/json;q=0.8;charset=utf-7", "application/json;charset=utf-8;q=0.9")] + [InlineData("application/json", "application/json;format=indent;charset=utf-8")] + [InlineData("application/json;format=indent;charset=utf-8", "application/json;format=indent;charset=utf-8")] + [InlineData("application/json;charset=utf-8;format=indent", "application/json;format=indent;charset=utf-8")] + public void IsSubsetOf_ReturnsTrueWhenExpected(string set, string subset) + { + // Arrange + var setMediaType = new MediaType(set); + var subSetMediaType = new MediaType(subset); + + // Act + var result = subSetMediaType.IsSubsetOf(setMediaType); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("application/json;charset=utf-8", "application/json")] + [InlineData("application/json;format=indent;charset=utf-8", "application/json")] + [InlineData("application/json;format=indent;charset=utf-8", "application/json;charset=utf-8")] + public void IsSubsetOf_ReturnsFalseWhenExpected(string set, string subset) + { + // Arrange + var setMediaType = new MediaType(set); + var subSetMediaType = new MediaType(subset); + + // Act + var result = subSetMediaType.IsSubsetOf(setMediaType); + + // Assert + Assert.False(result); + } + + [Fact] + public void MatchesAllTypes_ReturnsTrueWhenExpected() + { + // Arrange + var mediaType = new MediaType("*/*"); + + // Act + var result = mediaType.MatchesAllTypes; + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("text/*")] + [InlineData("text/plain")] + public void MatchesAllTypes_ReturnsFalseWhenExpected(string value) + { + // Arrange + var mediaType = new MediaType(value); + + // Act + var result = mediaType.MatchesAllTypes; + + // Assert + Assert.False(result); + } + + [Theory] + [InlineData("*/*")] + [InlineData("text/*")] + public void MatchesAllSubtypes_ReturnsTrueWhenExpected(string value) + { + // Arrange + var mediaType = new MediaType(value); + + // Act + var result = mediaType.MatchesAllSubTypes; + + // Assert + Assert.True(result); + } + + [Fact] + public void MatchesAllSubtypes_ReturnsFalseWhenExpected() + { + // Arrange + var mediaType = new MediaType("text/plain"); + + // Act + var result = mediaType.MatchesAllSubTypes; + + // Assert + Assert.False(result); + } + + [Theory] + [MemberData(nameof(MediaTypesWithParameters))] + [InlineData("application/json;format=pretty;q=0.9;charset=utf-8;q=0.8")] + [InlineData("application/json;format=pretty;q=0.9;charset=utf-8;q=0.8;version=3")] + public void CreateMediaTypeSegmentWithQuality_FindsQValue(string value) + { + // Arrange & Act + var mediaTypeSegment = MediaType.CreateMediaTypeSegmentWithQuality(value, start: 0); + + // Assert + Assert.Equal(0.8d, mediaTypeSegment.Quality); + } } }