is-04 icon indicating copy to clipboard operation
is-04 copied to clipboard

CORS requirements can be misread

Open orryverducci opened this issue 2 years ago • 4 comments

This is a follow up to a conversation in an issue logged with the NMOS API Testing Tool (AMWA-TV/nmos-testing#722).

Currently the Server Side Implementation Notes state that "all NMOS APIs MUST implement valid CORS HTTP headers in responses to all requests". As currently worded this can be interpereted to mean that all APIs must always send CORS headers, regardless of whether or not the client is making a cross origin request that requires CORS headers.

However the Fetch specification, which specifies CORS, only requires servers to send CORS headers if the client is making a cross origin request and participating in the CORS protocol. This is indicated by the client sending an Origin header, or in the case of a preflight request an Access-Control-Request-Method header.

I expect the intention here was not for NMOS API servers to respond with CORS headers at all times, but rather for servers to respond with the appropriate CORS headers to allow a cross origin request when the client makes a CORS request as per the Fetch specification. As such the NMOS specifications should be reworded to clarify this.

The issue with the current wording is some NMOS client implementations expect server to always send CORS headers. However some servers and development frameworks (e.g. ASP.NET) by default only send CORS headers when they receive an appropriate header (e.g. Origin) in the request from the client, which is valid behaviour per the Fetch specification.

orryverducci avatar Nov 15 '22 13:11 orryverducci

Thanks for the follow up. I agree with your assessment of the specs.

some NMOS client implementations expect server to always send CORS headers.

Without naming names, are you aware of controllers that expect these headers, not just the testing tool (previous to the fix by https://github.com/AMWA-TV/nmos-testing/pull/724)?

garethsb avatar Nov 15 '22 13:11 garethsb

Admittedly the testing tool is the only client I'm aware of that is explicitly checking for CORS headers.

I am aware of a commercially available controller that is developed on top of .NET / ASP.NET and always sends CORS server headers based on their interpretation of the current specification (and likely previous testing tool behaviour). As mentioned previously I know from experience that is not default behaviour for the framework, and so the developers would have had to go though some hoops to override that.

orryverducci avatar Nov 23 '22 14:11 orryverducci

Architecture Review Group review: place on backlog

peterbrightwell avatar Jun 23 '23 13:06 peterbrightwell

I will review latest Fetch spec and see what the simplest change to the NMOS specs (Server Side Implementation sections) would be.

  • Fetch spec - https://fetch.spec.whatwg.org/#http-cors-protocol
  • IS-04 - https://github.com/AMWA-TV/is-04/blob/v1.3.x/docs/APIs%20-%20Server%20Side%20Implementation%20Notes.md#cross-origin-resource-sharing-cors
  • IS-05 - https://github.com/AMWA-TV/is-05/blob/v1.1.x/docs/APIs%20-%20Server%20Side%20Implementation.md#cross-origin-resource-sharing-cors
  • IS-06 - https://github.com/AMWA-TV/is-06/blob/v1.0.x/docs/APIs%20-%20Server%20Side%20Implementation.md#cross-origin-resource-sharing-cors
  • IS-07 - https://github.com/AMWA-TV/is-07/blob/v1.0.x/docs/Event%20and%20tally%20rest%20api.md#6-cross-origin-resource-sharing-cors
  • IS-08 - https://github.com/AMWA-TV/is-08/blob/v1.0.x/docs/APIs%20-%20Server%20Side%20Implementation.md#cross-origin-resource-sharing-cors
  • IS-09 - https://github.com/AMWA-TV/is-09/blob/v1.0.x/docs/APIs%20-%20Server%20Side%20Implementation%20Notes.md#cross-origin-resource-sharing-cors
  • IS-10 - has references to CORS in the RAML but doesn't have the Server Side Implementation or other "core" sections that most of these specs have, though does have this: https://github.com/AMWA-TV/is-10/blob/v1.0.x/docs/APIs.md#options-requests
  • IS-11 - https://github.com/AMWA-TV/is-11/blob/v1.0.x/docs/APIs%20-%20Server%20Side%20Implementation.md#cross-origin-resource-sharing-cors
  • IS-12 - is WebSocket not HTTP
  • IS-13 - https://github.com/AMWA-TV/is-13/blob/v1.0-dev/docs/APIs%20-%20Server%20Side%20Implementation.md#cross-origin-resource-sharing-cors
  • IS-14 - has references to CORS in the RAML but doesn't have the Server Side Implementation or other "core" sections that most of these specs have, yet... @jonathan-r-thorpe

garethsb avatar Jun 12 '24 18:06 garethsb