Skip to content

Commit

Permalink
Bug #89350: media type validation (#1462)
Browse files Browse the repository at this point in the history
* AB#89350 - media type param values should be accepted unquoted

* Added failing tests for single quotes around content types.

* Simplified AcceptContentFilterAttribute and its related tests by removing allowSingle and allowMultiple, as allowSingle was always true and allowMultiple was always false.

* Flipped media type check to account for media type ranges in the Accept header, like application/*. Also removed a now-redundant E2E test.
  • Loading branch information
mitchell-fox committed Apr 7, 2022
1 parent 1db363b commit ee93cf5
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 152 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@
using Microsoft.AspNetCore.Routing;
using Microsoft.Health.Dicom.Api.Features.Filters;
using Microsoft.Health.Dicom.Core.Web;
using Microsoft.Net.Http.Headers;
using Xunit;

namespace Microsoft.Health.Dicom.Api.UnitTests.Features.Filters;

/// <summary>
/// These tests leverage the existing ASP.NET Core tests at: https://github.com/dotnet/aspnetcore/blob/main/src/Http/Headers/test/MediaTypeHeaderValueTest.cs
/// </summary>
public class AcceptContentFilterAttributeTests
{
private AcceptContentFilterAttribute _filter;
Expand All @@ -26,159 +30,116 @@ public AcceptContentFilterAttributeTests()
_context = CreateContext();
}

[Theory]
[InlineData("application/dicom+json; transfer-syntax=\"*\"", null)]
[InlineData("applicAtion/dICOM+Json", null)]
[InlineData("multipart/related; type=\"application/dicom+json\"", null)]
[InlineData("multipart/related; type=\"application/dicom\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related; type=\"blah\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related;", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/dicom+json+something", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/dicom", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/xml", (int)HttpStatusCode.NotAcceptable)]
[InlineData("", (int)HttpStatusCode.NotAcceptable)]
public void GivenARequestWithAValidAcceptHeader_WhenValidatingTheContentTypeAgainstSingleAndMultipartHeaderFilter_ThenCorrectStatusCodeShouldBeReturned(string acceptHeaderMediaType, int? expectedStatusCode)
[Fact]
public void GivenARequestWithNoAcceptHeader_ThenNotAcceptableStatusCodeShouldBeReturned()
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" }, true, true);

_context.HttpContext.Request.Headers.Add("Accept", acceptHeaderMediaType);
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicom, KnownContentTypes.ApplicationOctetStream });

_filter.OnActionExecuting(_context);

Assert.Equal(expectedStatusCode, (_context.Result as StatusCodeResult)?.StatusCode);
Assert.Equal((int)HttpStatusCode.NotAcceptable, (_context.Result as StatusCodeResult)?.StatusCode);
}

[Theory]
[InlineData(null, "application/dicom+json", "image/jpg")]
[InlineData((int)HttpStatusCode.NotAcceptable, "image/png", "image/jpg")]
[InlineData(null, "multipart/related; type=\"application/dicom+json\"", "image/jpg")]
[InlineData(null, "multipart/related; type=\"application/dicom+json\"", "multipart/related; type=\"image/jpg\"")]
[InlineData((int)HttpStatusCode.NotAcceptable, "multipart/related; type=\"image/png\"", "multipart/related; type=\"image/jpg\"")]
[InlineData(null, "multipart/related; type=\"image/jpg\"", "application/dicom+json")]
public void GivenARequestWithMultipleAcceptHeaders_WhenValidatingTheContentTypeAgainstSingleAndMultipartHeaderFilter_ThenCorrectStatusCodeShouldBeReturned(int? expectedStatusCode, params string[] acceptHeaderMediaType)
[InlineData("*/*")]
[InlineData("application/*")]
[InlineData("application/json")]
[InlineData("application/xml")]
[InlineData("application/dicom+json")]
[InlineData("applicAtion/dICOM+Json")]
[InlineData("application/dicom+xml")]
[InlineData("applicAtion/DICOM+XmL")]
[InlineData("application/dicom+json; transfer-syntax=*")]
[InlineData("application/dicom+json; transfer-syntax=\"*\"")]
public void GivenARequestWithAValidAcceptHeader_WhenMediaTypeMatches_ThenSuccess(string acceptHeaderMediaType)
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" }, true, true);
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" });

_context.HttpContext.Request.Headers.Add("Accept", acceptHeaderMediaType);
_context.HttpContext.Request.Headers.TryAdd(HeaderNames.Accept, acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal(expectedStatusCode, (_context.Result as StatusCodeResult)?.StatusCode);
Assert.Null((_context.Result as StatusCodeResult)?.StatusCode);
}

[Fact]
public void GivenARequestWithNoAcceptHeader_WhenValidatingTheContentTypeAgainstSingleAndMultipartHeaderFilter_ThenNotAcceptableStatusCodeShouldBeReturned()
[Theory]
[InlineData("application/dicom+json+something")]
[InlineData("application/dicom")]
[InlineData("")]
[InlineData(null)]
public void GivenARequestWithAValidAcceptHeader_WhenMediaTypeDoesntMatch_ThenFailure(string acceptHeaderMediaType)
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" }, true, true);
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" });

_context.HttpContext.Request.Headers.Add(HeaderNames.Accept, acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal((int)HttpStatusCode.NotAcceptable, (_context.Result as StatusCodeResult)?.StatusCode);
}

[Theory]
[InlineData("application/dicom+json", null)]
[InlineData("applicAtion/dICOM+Json", null)]
[InlineData("multipart/related; type=\"application/dicom+json\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related; type=\"blah\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related;", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/dicom+json+something", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/dicom", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/xml", (int)HttpStatusCode.NotAcceptable)]
[InlineData("", (int)HttpStatusCode.NotAcceptable)]
public void GivenARequestWithAValidAcceptHeader_WhenValidatingTheContentTypeAgainstSingleHeaderFilter_ThenCorrectStatusCodeShouldBeReturned(string acceptHeaderMediaType, int? expectedStatusCode)
[InlineData("application/dicom+json", "image/jpg")]
[InlineData("application/dicom+xml", "image/png")]
[InlineData("application/dicom+json", "application/dicom+xml")]
[InlineData("image/png", "application/dicom+xml")]
[InlineData("application/dicom", "application/xml")]
public void GivenARequestWithMultipleAcceptHeaders_WhenAnyMediaTypeMatches_ThenSuccess(params string[] acceptHeaderMediaType)
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" }, true, false);
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" });

_context.HttpContext.Request.Headers.Add("Accept", acceptHeaderMediaType);
_context.HttpContext.Request.Headers.Add(HeaderNames.Accept, acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal(expectedStatusCode, (_context.Result as StatusCodeResult)?.StatusCode);
Assert.Null((_context.Result as StatusCodeResult)?.StatusCode);
}

[Theory]
[InlineData(null, "application/dicom+json", "image/jpg")]
[InlineData((int)HttpStatusCode.NotAcceptable, "image/png", "image/jpg")]
[InlineData((int)HttpStatusCode.NotAcceptable, "multipart/related; type=\"application/dicom+json\"", "image/jpg")]
[InlineData(null, "multipart/related; type=\"image/jpg\"", "application/dicom+json")]
public void GivenARequestWithMultipleAcceptHeaders_WhenValidatingTheContentTypeAgainstSingleHeaderFilter_ThenCorrectStatusCodeShouldBeReturned(int? expectedStatusCode, params string[] acceptHeaderMediaType)
[InlineData("image/png", "image/jpg")]
[InlineData("application/dicom", "image/png")]
public void GivenARequestWithMultipleAcceptHeaders_WhenNoMediaTypeMatches_ThenFailure(params string[] acceptHeaderMediaType)
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" }, true, false);
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" });

_context.HttpContext.Request.Headers.Add("Accept", acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal(expectedStatusCode, (_context.Result as StatusCodeResult)?.StatusCode);
}

[Fact]
public void GivenARequestWithNoAcceptHeader_WhenValidatingTheContentTypeAgainstSingleHeaderFilter_ThenNotAcceptableStatusCodeShouldBeReturned()
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" }, true, false);
_context.HttpContext.Request.Headers.Add(HeaderNames.Accept, acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal((int)HttpStatusCode.NotAcceptable, (_context.Result as StatusCodeResult)?.StatusCode);
}

[Theory]
[InlineData("application/dicom", (int)HttpStatusCode.NotAcceptable)]
[InlineData("applicAtion/dICOM", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related; type=\"application/dicom+json\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related; type=\"application/octet_stream\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related; type=\"application/octetStream\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related; type=\"application/dicom\"", null)]
[InlineData("multipart/relATed; type=\"applICation/DICom\"", null)]
[InlineData("multipart/related; type=\"application/octet-stream\"", null)]
[InlineData("multipart/related; type=\"blah\"", (int)HttpStatusCode.NotAcceptable)]
[InlineData("multipart/related;", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/dicom+something", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/dic", (int)HttpStatusCode.NotAcceptable)]
[InlineData("application/xml", (int)HttpStatusCode.NotAcceptable)]
[InlineData("", (int)HttpStatusCode.NotAcceptable)]
public void GivenARequestWithAValidAcceptHeader_WhenValidatingTheContentTypeAgainstMultipartHeaderFilter_ThenCorrectStatusCodeShouldBeReturned(string acceptHeaderMediaType, int? expectedStatusCode)
[InlineData("image/png, image/jpg, application/dicom+xml")]
[InlineData("application/dicom+json, application/xml")]
public void GivenARequestWithOneAcceptHeaderWithMultipleTypes_WhenAnyMediaTypeMatches_ThenSuccess(string acceptHeaderMediaType)
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicom, KnownContentTypes.ApplicationOctetStream }, false, true);
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" });

_context.HttpContext.Request.Headers.Add("Accept", acceptHeaderMediaType);
_context.HttpContext.Request.Headers.Add(HeaderNames.Accept, acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal(expectedStatusCode, (_context.Result as StatusCodeResult)?.StatusCode);
Assert.Null((_context.Result as StatusCodeResult)?.StatusCode);
}

[Theory]
[InlineData((int)HttpStatusCode.NotAcceptable, "image/png", "image/jpg")]
[InlineData(null, "multipart/related; type=\"application/dicom\"", "image/jpg")]
[InlineData(null, "multipart/related; type=\"application/dicom\"", "multipart/related; type=\"image/jpg\"")]
[InlineData((int)HttpStatusCode.NotAcceptable, "multipart/related; type=\"image/png\"", "multipart/related; type=\"image/jpg\"")]
[InlineData((int)HttpStatusCode.NotAcceptable, "multipart/related; type=\"image/jpg\"", "application/dicom+json")]
public void GivenARequestWithMultipleAcceptHeaders_WhenValidatingTheContentTypeAgainstMultipartHeaderFilter_ThenCorrectStatusCodeShouldBeReturned(int? expectedStatusCode, params string[] acceptHeaderMediaType)
[InlineData("image/png, image/jpg, application/dicom")]
[InlineData("application/dicom, application/pdf")]
public void GivenARequestWithOneAcceptHeaderWithMultipleTypes_WhenNoMediaTypeMatches_ThenFailure(string acceptHeaderMediaType)
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicom, KnownContentTypes.ApplicationOctetStream }, false, true);

_context.HttpContext.Request.Headers.Add("Accept", acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal(expectedStatusCode, (_context.Result as StatusCodeResult)?.StatusCode);
}
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicomJson, "application/dicom+xml" });

[Fact]
public void GivenARequestWithNoAcceptHeader_WhenValidatingTheContentTypeAgainstMultipartHeaderFilter_ThenNotAcceptableStatusCodeShouldBeReturned()
{
_filter = CreateFilter(new[] { KnownContentTypes.ApplicationDicom, KnownContentTypes.ApplicationOctetStream }, false, true);
_context.HttpContext.Request.Headers.Add(HeaderNames.Accept, acceptHeaderMediaType);

_filter.OnActionExecuting(_context);

Assert.Equal((int)HttpStatusCode.NotAcceptable, (_context.Result as StatusCodeResult)?.StatusCode);
}

private AcceptContentFilterAttribute CreateFilter(string[] supportedMediaTypes, bool allowSingle, bool allowMultiple)
private AcceptContentFilterAttribute CreateFilter(string[] supportedMediaTypes)
{
return new AcceptContentFilterAttribute(supportedMediaTypes, allowSingle, allowMultiple);
return new AcceptContentFilterAttribute(supportedMediaTypes);
}

private static ActionExecutingContext CreateContext()
Expand Down
12 changes: 6 additions & 6 deletions src/Microsoft.Health.Dicom.Api/Controllers/QueryController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public QueryController(IMediator mediator, ILogger<QueryController> logger)
}

[HttpGet]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType((int)HttpStatusCode.NoContent)]
Expand All @@ -69,7 +69,7 @@ public async Task<IActionResult> QueryForStudyAsync([FromQuery] QueryOptions opt
}

[HttpGet]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType((int)HttpStatusCode.NoContent)]
Expand All @@ -92,7 +92,7 @@ public async Task<IActionResult> QueryForSeriesAsync([FromQuery] QueryOptions op
}

[HttpGet]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType((int)HttpStatusCode.NoContent)]
Expand All @@ -115,7 +115,7 @@ public async Task<IActionResult> QueryForSeriesInStudyAsync([FromRoute] string s
}

[HttpGet]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType((int)HttpStatusCode.NoContent)]
Expand All @@ -138,7 +138,7 @@ public async Task<IActionResult> QueryForInstancesAsync([FromQuery] QueryOptions
}

[HttpGet]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType((int)HttpStatusCode.NoContent)]
Expand All @@ -161,7 +161,7 @@ public async Task<IActionResult> QueryForInstancesInStudyAsync([FromRoute] strin
}

[HttpGet]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType((int)HttpStatusCode.NoContent)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public async Task<IActionResult> GetStudyAsync(string studyInstanceUid)
return CreateResult(response);
}

[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType(typeof(string), (int)HttpStatusCode.BadRequest)]
Expand Down Expand Up @@ -117,7 +117,7 @@ public async Task<IActionResult> GetStudyMetadataAsync([FromHeader(Name = IfNone
return CreateResult(response);
}

[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType(typeof(string), (int)HttpStatusCode.BadRequest)]
Expand Down Expand Up @@ -164,7 +164,7 @@ public async Task<IActionResult> GetSeriesMetadataAsync([FromHeader(Name = IfNon
return CreateResult(response);
}

[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType(typeof(IEnumerable<DicomDataset>), (int)HttpStatusCode.OK)]
[ProducesResponseType(typeof(string), (int)HttpStatusCode.BadRequest)]
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Health.Dicom.Api/Controllers/StoreController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public StoreController(IMediator mediator, ILogger<StoreController> logger)
_logger = logger;
}

[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[Consumes(KnownContentTypes.ApplicationDicom, KnownContentTypes.MultipartRelated)]
[ProducesResponseType(typeof(DicomDataset), (int)HttpStatusCode.OK)]
Expand All @@ -62,7 +62,7 @@ public async Task<IActionResult> PostInstanceAsync()
return await PostAsync(null);
}

[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[Consumes(KnownContentTypes.ApplicationDicom, KnownContentTypes.MultipartRelated)]
[ProducesResponseType(typeof(DicomDataset), (int)HttpStatusCode.OK)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public partial class WorkitemController
/// DICOM PS 3.19 XML metadata is not supported.
/// </remarks>
/// <returns></returns>
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson }, allowSingle: true, allowMultiple: false)]
[AcceptContentFilter(new[] { KnownContentTypes.ApplicationDicomJson })]
[Produces(KnownContentTypes.ApplicationDicomJson)]
[Consumes(KnownContentTypes.ApplicationDicomJson)]
[ProducesResponseType((int)HttpStatusCode.OK)]
Expand Down
Loading

0 comments on commit ee93cf5

Please sign in to comment.