Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error deleting a dicom study or retrieving a study using REST API #1396

Closed
mhassaninmsft opened this issue Mar 4, 2022 · 3 comments
Closed

Comments

@mhassaninmsft
Copy link

Describe the bug
The application does not respect the HTTP 'Accept' headers in the request. When we provide -H 'accept:multipart/related; type=application/dicom' The application only receives. application/dicom and not the multipart/related part of the header which causes some checks later in the request to fail (the checks enforce both application/dicom as the only supported media type and also expect the multipart/related section.

  • The headers are conformat to the spec as specified here
  • After debugging, it seems there is a bug with ASP.NET Typed headers as used in \dicom-server\src\Microsoft.Health.Dicom.Api\Extensions\HttpRequestExtensions.cs The call to the method httpRequest.GetTypedHeaders().Accept returns a list of MediaTypeHeaderValue which seems strange, As per the class doc on Microsoft, the class is used for content type and not accept headers. The call to httpRequest.GetTypedHeaders().Accept only returns application/dicom and not the multipart/related section. However, a call to httpRequest.Headers.GetCommaSeparatedValues(HeaderNames.Accept) returns the correct headers.
  • Ideally there should be a bug request for ASP.NET to fix how accept headers are handled. or in the meantime, the dicom code can change from the MediaTypeHeaderValue for accept header to a custom implemnted header handler.
  • I tried forcing the MediaTypeHeaderValue to accept the correct headler by calling new MediaTypeHeaderValue(new StringSegment(x, 0, x.Length)) where x = "multipart/related; type=application/dicom", but that threw a formatting exception confirming it is not the correct abstraction.

To Reproduce
Steps to reproduce the behavior:

  1. Enable the openAPI Spec (optional).
  2. Follow the instructions in the README to upload a dicom file using fiddler or postman
  3. Try to delete or get that study using the swagger button which correpsonds to
    curl -X 'GET' 'https://localhost:63838/v1/studies/1.2.826.0.1.3680043.8.498.13230779778012324449356534479549187420' -H 'accept:multipart/related; type=application/dicom' -H 'content-type: application/dicom' substitue the study instace id with the apporpiate study instance if of the file you uploaded in step 2.
    You will get an exception:
    "Headers are not supported"

Expected behavior
Successful Response with the study file or an empoty response when deleting the study

Actual behavior
Headers not supported exxeption

@jovinson-ms
Copy link
Contributor

Hi @mhassaninmsft, can you clarify what tool you're using to send the GET request? If you enabled the Swagger UI, that's not currently supported due to accessibility issues.

I did just check your scenario against the Postman collection, and I was able to retrieve a study as a multipart response successfully. Looking at your request, there are two things that stand out:

  • When specifying the content type in either the Accept or Content-Type header, you need to include surrounding quotes, like Accept:multipart/related; type="application/dicom". According to the spec, the media type is surrounded by quotes when a multipart response is requested.
  • You don't include a Content-Type header in a WADO-RS request since there's no body.

Please let me know if this works for you!

@jovinson-ms
Copy link
Contributor

jovinson-ms commented Mar 8, 2022

And the spec does say that the media type param value can be quoted or unquoted - that's a bug on us.

edit: this is generally true, but not in this case, see below

@jovinson-ms
Copy link
Contributor

Correction of above - our current handling of quotes is correct according to the Dicom standard:

  • according to 8.7, in general, media type parameter values can be quoted or unquoted (i.e. text/html; q=0.3 and text/html; q="0.3" are both valid)
  • according to 8.7.1, in the special case where the media type is multipart/related there must be a type parameter present, and it must be quoted (i.e. multipart/related; type=application/dicom is not valid)

Closing this issue - thanks for raising it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants