CORS requirements can be misread

[orryverducci]

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.

[garethsb]

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 AMWA-TV/nmos-testing#724)?

[orryverducci]

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.

[peterbrightwell]

Architecture Review Group review: place on backlog

[garethsb]

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