config to remove " " for the Content Type in accept headers

[sedghi]

Currently, dicomwebClient adds "" around the type in the accept header. However, having "" makes a request a "not simple request" resulting in a preflight request to be sent to the server first. You can see below

With quotation

This is basically twice the amount of request hence twice the travel to the server, making the load time significantly slower (on 600 images around 2 seconds).

Apparently in the original RFC for the standard there is no need for quotation, but in the examples there are quotations. I'm opening this issue to check on the feasibility for removing this quotation (or maybe see if there's a better way to do it.)

The RFC is https://www.ietf.org/rfc/rfc2387.txt and the text that is difinitive for the mime type is:

related-param := [ ";" "start" "=" cid ]
[ ";" "start-info" "="
( cid-list / value ) ]
[ ";" "type" "=" type "/" subtype ]
; order independent

where type and subtype are both tokens. Tokens are not permitted to have quotes around them unless otherwise specified in the syntax.

I'm CC'ing the people who I think might want to participate in this discussion

@hackermd @dclunie @pieper @swederik @chafey @wayfarer3130 @Zaid-Safadi @fedorov

[hackermd]

Thanks @sedghi for reporting this interesting observation and for the technical deep dive.

DICOM PS3.18 Section 8.7.1 Multipart Media Types states

The "type" parameter is required. It contains the media type of the "root" body part. It always contains the special character "/" and thus requires quote marks.

RFC 2387 Section 3.4 Syntax further states:

Note that the parameter values will usually require quoting.

That's why we used " " for the value of the type parameter.

Does anyone else have any experience with this? @jasonklotzer @gunterze @StevenBorg do you have any thoughts on this?

[wayfarer3130]

The " around is in a note in the RFC, thus not normative. The normative section is the syntax definition, and that part does not permit quotes - and is defined in exactly teh same way other content parts are defined, so I suspect it is correct there.

[chafey]

Some quick testing shows that this quote behavior is consistent across Chrome, Firefox and Safari. Given that, I suggest we disable quoting by default and allow it to be turned on if the server requires it

[Zaid-Safadi]

This is great information and thanks to everyone for explaining. I wanted to share my opinion as I got burned by this in the past due to the inconsistency of interpreting the standards.

Assuming the intent of this open-source project is helping everyone, including the average Joe decades from now, to pull the library and start using it with no issues out of the box, I believe we should have the default configuration follow what the standard says. Subject matter experts who know how to interpret the standards, and wanting to get extra performance will have no problem digging into the details and finding about flipping the quotation configuration if that really helps.

The first version of the DICOMweb standard didn't have the "quotation" requirement, when I implemented DICOMcloud and DICOMweb-is, I spent significant time trying to figure out why my JS client fails to request images from the server, and after tons of research, I found out in the RFC that I had to include "quotation" in my request, and once I did, my requests finally worked. This is due to the .NET Framework not accepting the "type" parameter without these quotes. Eventually, the next version of the DICOMweb standard came out and fixed this by requiring the quotation and I created an issue to have this library fixed accordingly. I would really hate for this library to cause someone else the same pain I had because by default we are not following the standard.

My suggestion, if this is indeed an issue, let's first get it updated in the DICOM standard after clearing the RFC requirements, then we can change the default (and maybe the NET Framework if needed).

[dclunie]

This has a long and sordid history in DICOM PS3.18, and I am still not sure what the "right" answer is (if you will excuse the quotes), but @wayfarer3130 has promised to reactivate CP 1776 and push it through the DICOM process in WG 27 and WG 6.

[sedghi]

Thank you all for your responses. Looking at the web standard here https://www.w3.org/TR/2020/SPSD-cors-20200602/#simple-header tells me that it should be unquoted, but I don't care about what should be the default value.

Some sort of configuration is needed for backward compatibility even if the dicom standard gets updated at some point, are you (@hackermd) open to add such config at the library level? I can make the PR for it

[pieper]

+1 for removing the quotes but providing a config option to turn it on if a particular server requires it. +100 for clarifying the standard and any implementations that behave inconsistently.

[gunterze]

WildFly/RESTEasy content type parser is agnostic concerning quotes around parameter values, so DICOMWeb servers relying on it (as dcm4chee-arc 5.x) should handle quoted and unquoted content type parameter values the same way.

[pieper]

I can make the PR for it

It sounds to me like we are ready to move to that stage.

[hackermd]

Thanks everyone for providing additional background and feedback.

Given all the great input, I suggest adding a parameter to the constructor of the DICOMwebClient class to control the behavior (e.g., quoteTypeParameterValue or something along those lines). Given that the current edition of the standard still requires quotes and the client currently achieves interoperability with a wide range of origin servers, I would suggest keeping the default behavior for the time being (i.e., set { quoteTypeParameterValue: true }). We can monitor the behavior of different servers when omitting the quotes and re-consider the default argument moving forward.

What do you think?

[pieper]

I like your proposal @hackermd because as I understand it changing the default behavior would break some use cases and would only improve performance in others. Also we can expect that a user will know the capabilities of the server if they choose to change the default behavior to improve performance for a specific server. It also seems to me that if someone really wanted to we could add a { quoteTypeParameterValue: 'auto' } option that probed the server to see what it accepts.

[swederik]

Thanks everyone for providing additional background and feedback.

Given all the great input, I suggest adding a parameter to the constructor of the DICOMwebClient class to control the behavior (e.g., quoteTypeParameterValue or something along those lines). Given that the current edition of the standard still requires quotes and the client currently achieves interoperability with a wide range of origin servers, I would suggest keeping the default behavior for the time being (i.e., set { quoteTypeParameterValue: true }). We can monitor the behavior of different servers when omitting the quotes and re-consider the default argument moving forward.

What do you think?

Sounds great to me! For OHIF we will just set it false for our demos so they are fast, and this way we won't impact any servers that can't work without the quotes.

[sedghi]

Sounds like we have a conclusion. Thanks everyone

[hackermd]

@sedghi would you like to draft a pull request?

[hackermd]

I have run some tests against a range of different archives and the requests appear to handled just fine when the quotes are omitted. Therefore, I would be open to changing the default behavior of the library and send requests without quotes by default.

[pieper]

@Zaid-Safadi did I understand correctly that quotes are required for the .NET based server? If that's the case I'd still vote for leaving the default as is (or implement the auto option).

[hackermd]

I like the auto option @pieper. If the current behavior in fact violates the HTTP standard, then I tend argue that we should not keep it as default.

[Zaid-Safadi]

@Zaid-Safadi did I understand correctly that quotes are required for the .NET based server? If that's the case I'd still vote for leaving the default as is (or implement the auto option).

That is correct @pieper , .NET framework doesn't recognize the "type" parameter if it is not quoted. If the quotes are omitted by default, the library becomes not usable as-is with any DICOMweb server built in .NET including MS DICOMweb server, DICOMcloud or any other server built using the .NET based DICOM toolkit fo-dicom

The only two examples in the RFC showing how a multipart/related content-type header is formatted, are both quoted as well: Section 5.1

 Content-Type: Multipart/Related; boundary=example-1
         start="<[email protected]>";
         type="Application/X-FixedRecord"
         start-info="-o ps" 

and Section 5.2

     Content-Type: Multipart/Related; boundary=example-2;
             start="<[email protected]>"
             type="Text/x-Okie"

[dclunie]

Perhaps @StevenBorg Steven Borg [email protected], who has an interest in DICOMweb for WSI, may be able to help with this.

[StevenBorg]

        I’ll take a look!---- On Thu, 20 Oct 2022 14:43:58 -0700  David ***@***.***> wrote ---- 

Perhaps @StevenBorg Steven Borg @.***, who has an interest in DICOMweb for WSI, may be able to help with this.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[hackermd]

RFC1872 The MIME Multipart/Related Content-type Section 3.4 Syntax states:

Note that the parameter values will usually require quoting.

As pointed out by @Zaid-Safadi, all examples in the RFC for the Content-type header field quote the value of the type parameter.

RFC9119 HTTP Semantics Section 8.3.3 Multipart Types states:

All multipart types share a common syntax, as defined in Section 5.1.1 of [RFC2046]

RFC2046 Section 5.1.1 Common Syntax doesn't explicitly comment on the quoting of the type parameter, but makes the following statement regarding the boundary parameter:

The grammar for parameters on the Content-type field is such that it is often necessary to enclose the boundary parameter values in quotes on the Content-type line. This is not always necessary, but never hurts.

RFC9119 HTTP Semantics Section 5.6.6 Parameters states:

A parameter value that matches the token production can be transmitted either as a token or within a quoted-string.

RFC9119 HTTP Semantics Section 5.6.2 Tokens states:

Tokens are short textual identifiers that do not include whitespace or delimiters. [...] Delimiters are chosen from the set of US-ASCII visual characters not allowed in a token (DQUOTE and "(),/:;<=>?@[]{}")

Since the value of the type parameter includes the / character, it should not be considered a token but a quoted-string.

My conclusion is that quoting of the type parameter value is required by HTTP.

[sedghi]

Thanks @hackermd for the research, we also opted in to have it as default true (with "") in OHIF https://github.com/OHIF/Viewers/blob/v3-stable/platform/viewer/public/config/default.js#L9

[hackermd]

It appears the preflight requests are performed because the " character in the Accept header field is considered a CORS-unsafe request-header byte.

However, simply omitting the character and thereby breaking HTTP syntax rules is not a good way to work around the problem in my opinion.

[Zaid-Safadi]

RFC1872 The MIME Multipart/Related Content-type Section 3.4 Syntax states:

Note that the parameter values will usually require quoting.

As pointed out by @Zaid-Safadi, all examples in the RFC for the Content-type header field quote the value of the type parameter.

RFC9119 HTTP Semantics Section 8.3.3 Multipart Types states:

All multipart types share a common syntax, as defined in Section 5.1.1 of [RFC2046]

RFC2046 Section 5.1.1 Common Syntax doesn't explicitly comment on the quoting of the type parameter, but makes the following statement regarding the boundary parameter:

The grammar for parameters on the Content-type field is such that it is often necessary to enclose the boundary parameter values in quotes on the Content-type line. This is not always necessary, but never hurts.

RFC9119 HTTP Semantics Section 5.6.6 Parameters states:

A parameter value that matches the token production can be transmitted either as a token or within a quoted-string.

RFC9119 HTTP Semantics Section 5.6.2 Tokens states:

Tokens are short textual identifiers that do not include whitespace or delimiters. [...] Delimiters are chosen from the set of US-ASCII visual characters not allowed in a token (DQUOTE and "(),/:;<=>?@[]{}")

Since the value of the type parameter includes the / character, it should not be considered a token but a quoted-string.

My conclusion is that quoting of the type parameter value is required by HTTP.

These are great details @hackermd , thank you for the effort!