Handle relative URI's

[wayfarer3130]

This change supports handling relative URI's as clarified in the CP "Handle Relative URI's" for bulkdata requests. No other request types are modified.

[wayfarer3130]

The CP for relative URI, describing why the base relative URI's are already permitted by the DICOMweb standard, as well as defining the base path for series/instance metadata is available here: https://docs.google.com/document/d/1D_qweX55vymlgNqJimJzizoT1qg0XesA/edit?usp=sharing&ouid=106394421000961430700&rtpof=true&sd=true

[pieper]

@wayfarer3130 are Bulkdata and Series really allowed to be capitalized here?

[wayfarer3130]

@wayfarer3130 are Bulkdata and Series really allowed to be capitalized here?

Good catch - the Bulkdata one can be capitalized as it is an implementation specific version, while the Series must be series. Those were auto-capitalized by word.

[wayfarer3130]

I've seen absolute path relative URI's from at least one other vendor, but I don't remember which one it was. I'm not saying that normatively the bulk data needs to be in a particular place to make the relative URL's work, but if you do put them relative to the studies/<studyUID> directory, then you can access them relatively. Actually, you can regardless, just that you dont need any .. or anything like that to access them with a relative path relative URI. The embedded base URI is implicit, and just makes the relative path URI's consistent - but if you don't otherwise specify the base URI embedded in content (implicitly), then the path changes between study, series and instance level metadata, which I find ugly. Bill

…

On Tue, 12 Apr 2022 at 10:58, David Clunie ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/api.js <#54 (comment)> : > - const url = options.BulkDataURI; + // Allow relative URI's, assume it is relative to the studyUID directory Only that I am not sure anyone ever mentioned the possibility of using anything other than absolute URLs during the development of WADO-RS, but I could have missed the discussion. Has any use of them been observed in the wild by other implementers? Personally I would not use relative URLs if I could avoid them, especially if there is any potential ambiguity about what the base for them to be relative to that could lead to a different interpretation by origin server and user agent. I have strong objections to: - normatively specifying that the bulk data needs to be in a particular place just to make the use of relative URLs work - - introducing "embedded base URIs", which are not AFAIK mentioned in any of the existing PS3.18 documentation and perhaps not fully specified in the RFC - if using these is going to be suggested, then where these go in the request headers and payload needs to be spelled out in the CP, IMHO the only rule necessary in the standard is that if the origin server writes a relative rather than absolute URL in the metadata, then following the RFC rules to figure out what it is relative, i.e., the URL of the metadata resource itself, should always work, preferably without introducing anything funky like 5.1.1 Base URI Embedded in Content <https://www.rfc-editor.org/rfc/rfc3986#section-5.1.1> or 5.1.2 Base URI from the Encapsulating Entity <https://www.rfc-editor.org/rfc/rfc3986#section-5.1.2> from the RFC, which might make more work from the client, assuming these are even fully specified somewhere. — Reply to this email directly, view it on GitHub <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGT56XNI7HCOI6WZJYQEE5DVEWFSJANCNFSM5SXMQE7A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[dclunie]

Can you clarify "the embedded base URI is implicit"? Where does this "embedded base URI" live?

[wayfarer3130]

https://datatracker.ietf.org/doc/html/rfc3986#section-5.1.1 defines a base URI embedded in content. That is simply a method to get the full URI given the document. For example, in html, it can be explicitly set using a meta-tag, and that includes the full path or a partial (relative path): <base href = "https://www.tutorialspoint.com" /> <base href = "/dicomweb/" /> The format/method of extraction is left up to any defining documents. The relativeURI CP proposes to define one for dicom+json/xml based on the StudyInstanceUID tag value and the location of the studies part of the URL that the resource was fetched from, being: <base-wado-rs-path>/studies/<studyInstanceUID> so that it exactly matches regardless if you fetch the metadata from any of: <base-wado-rs-path>/studies/<studyInstanceUID>/metadata (note this is the same relative URI path for either the new proposal or the existing one as specified by 5.1.3) <base-wado-rs-path>/studies/<studyInstanceUID>/series/<seriesUID>/metadata <base-wado-rs-path>/studies/<studyInstanceUID>/series/<seriesUID>/instances/<instanceUID>/metadata otherwise keeping track of where a metadata file came from, because the fetch URI for the last two are different from the fetch URI for the first one. Basically, I'm proposing that we pretend they all came from the same base URI. My thought is that it makes things cleaner, which is unlikely to affect anyone already using relative URIs. Bill

…

On Tue, 12 Apr 2022 at 14:04, David Clunie ***@***.***> wrote: Can you clarify "the embedded base URI is implicit"? Where does this "embedded base URI" live? — Reply to this email directly, view it on GitHub <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGT56XPKHPSEMXAQWIUZ2CDVEW3MJANCNFSM5SXMQE7A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[dclunie]

The most important thing is not to introduce any breaking changes either in terms of:

• change in existing interpretation, which I would take to be relative to the requested resource per the RFC (regardless of the implementation impact of that as @hackermd points out in terms of timing of retrieval +/- need to preserve state, that's the way it is now)
• new additional means of specifying the path to which relative URLs are relative that existing clients may not know about and hence not follow and hence fail

I.e., the CP must not break existing valid clients.

[wayfarer3130]

What I mean by keeping things as consistent as possible is to keep the paths compatible with https://datatracker.ietf.org/doc/html/rfc3986#section-5.1 where it explains about how to get the rest of the path for relative URI's, with the default behaviour of /studies/<studyUID>/metadata being to fetch bulkdata from /studies/<studyUID>. That would imply that /studies/<studyUID>/series/<seriesUID>/metadata bulkdata references would come from /studies/<studyUID>/series, but I'm suggesting that we change the definition on the bulkdataURI below the studies level to comes from /studies/<studyUID>/ as well. Bill

…

On Wed, 13 Apr 2022 at 21:24, Markus D. Herrmann ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/api.js <#54 (comment)> : > - const url = options.BulkDataURI; + // Allow relative URI's, assume it is relative to the studyUID directory I have repeatedly run into issues with absolute URLs provided via BulkDataURI when one or more reverse proxy servers are placed before a DICOMweb server. One then either has to configure the archive accordingly or rewrite message payloads in each proxy, but I have encountered several deployments where the URLs are wrong (inaccessible). However, absolute URLs have advantages, because applications do not need to keep track from which server a given metadata resource was retrieved (or which metadata resource was retrieved). At first glance this appears trivial, but applications may interact with multiple servers and keeping track of the mapping can quickly get complicated. cc @fedorov <https://github.com/fedorov> @igoroctaviano <https://github.com/igoroctaviano> Also, if a server would switch from absolute to relative URLs (encouraged by the change in the standard), it would probably break many existing applications. I would assume that most applications were not designed to handle relative URLs. Therefore, this CP could have significant negative consequences. @dclunie <https://github.com/dclunie> I would suggest discussing this also in the next WG-26 meeting. — Reply to this email directly, view it on GitHub <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGT56XKPVRSJ5F2LLXF3PTDVE5XTDANCNFSM5SXMQE7A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[dclunie]

That would break existing implementations.

[pieper]

@wayfarer3130 @dclunie so did we decide this change is not correct and we should close the PR?

[wayfarer3130]

@pieper - the relative URI change is valid for the standard, the only bit that isn't valid is that I assumed the base URL rather than having it provided - that is the bit of the URL in between the http://host/dicomweb/studies bit and the relative URI being specified. Let me make that explicit in the call, and then I think we should merge it. I will add tests for that too. OHIF worked around it by manually modifying the URL to include the prefix portion of the URI, but it would be nice for the rest of the services to have support.

[wayfarer3130]

I think this is standards compliant now - the retrieve needs to either have the path that the original metadata fetch used, or a study uid and optionally series uid to construct the intermediate path.

[pieper]

Thanks for following up on this @wayfarer3130.

@dclunie do you have any more comments?