Sorry to dredge this up, I know there's a lot of changes been made on CORS recently.
Say I make an cross-domain XHR request to a route with CORS switched on (origin allowed) and I include a custom header (e.g. x-custom-header) but that header is _not_ whitelisted in config.cors.headers or config.cors.additionalHeaders, the following will happen:
Once the above happens, the browser will show an error like:
Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource.
This message is concealing the real issue that the custom header isn't allowed. If this check wasn't there at all:
access-control-allow-headers header missing the custom headerAlso a more relevant error would be shown by the browser that would help with debugging:
Request header field x-custom-header is not allowed by Access-Control-Allow-Headers in preflight response.`
Do we need to validate headers here at all? Is that the browser's job?
+1
Perhaps a 400 response would be more suited, as I took the 404 as it being a case that I had to write a handler for an OPTIONS request (which is a perfectly logical assumption) and thats why it wasn't found.
Why even a 400? Why not just a 200 with the appropriate headers? There's nothing wrong with the OPTIONS request.
If I understand the spec correctly, a non-2xx response on a preflight is treated as though there was a network issue during preflight, which does not involve taking into account the preflight response headers. A 2xx response kicks the browser into validating the original request using the preflight response headers. So perhaps it should be a 200 response.
But yes, I believe the server ought to validate the headers listed in Access-Control-Allow-Headers just like it currently does.
To clarify, yes I would prefer a 200 if the request is valid, the 400 was just a preference over a page not found which is totally incorrect as the route is present, I just didnt like the indication the route was missing (and that it might need to be added).
Although if @devinivy is correct then the suggestion is moot!
The spec isn't clear on this. It just say "terminate these steps". I think a 404 is much safer option than 200 without the CORS headers. 400 is not appropriate here as there is nothing wrong with the request.
Is there any other system we can look at to see how they handle this? Also, if the request is missing some of the required request headers we can't even look up if the route exists and a 200 is misleading.
@hueniverse which part of the spec are you referencing there so I can go read up on it?
It looks like Express CORS will do a 200 without CORS headers (using this code https://gist.github.com/mtharrison/65ba1e7f6856df2ba8fa). What would the issue be with doing a 200 _with_ the CORS headers and then the browser knows exactly why the request can't happen?
http://www.w3.org/TR/cors/#resource-preflight-requests
Under http://www.w3.org/TR/cors/#cross-origin-request-with-preflight
If the response has an HTTP status code that is not in the 2xx range
- Apply the network error steps.
Providing a 4xx will have a similar effect, but the browser isn't given the chance to even use the preflight response headers. So hapi is putting the onus on itself to understand the browser spec for a "failed" preflight.
Yes. That's the crux of it I think. So my original question was supposed to mean should we be doing that? Or should we be letting the browser receive those preflight response headers and then decide what happens?
To be clear, a 200 response would include no CORS headers. That part is clear from the spec.
Here is my proposal: if the request is missing Origin or Access-Control-Request-Method, return 404 as this means the request is not a CORS request and we don't have an OPTIONS handler for those. If the request includes those two, but it fails the tests, return a 200 without any CORS headers.
To be clear, a 200 response would include no CORS headers. That part is clear from the spec.
I'm still not totally convinced on that. From what I understand it's supposed to be the browser doing the validation of the preflight, not the server. So it seems reasonable that the browser should get the CORS headers back to validate the request.
I've done a test, serving a file from Google Cloud Storage and they indeed send back a 200 _with_ CORS headers in response to a request with a non-whitlisted header. (You can try it out here: http://mattharrison.s3.amazonaws.com/cors/google-cloud-storage.html).
The browser error message in this case is the helpful one indicating exactly what went wrong:
Request header field x-custom-header is not allowed by Access-Control-Allow-Headers in preflight response.
With both the current and suggested implementation in hapi, it seems like it wouldn't be possible to get that error, because in response to an invalid header, hapi won't provide the CORS response headers in the response.
The spec is pretty clear on that part. The section I linked to before is for the server: http://www.w3.org/TR/cors/#resource-preflight-requests. There is another section for the browser which is completely irrelevant here.
The bottom line is that I don't really care what other servers are doing. My reading of the spec is that when the required headers are missing or not matching, "terminate this set of steps" means don't do anything else form this spec such as including any CORS headers.
"If any of the header field-names is not a ASCII case-insensitive match for any of the values in list of headers do not set any additional headers and terminate this set of steps."
That's pretty clear.
Yes, that is very clear! Maybe I've been thinking about this in reverse or something. hapi seems pretty close to the spec.
Thanks for your patience, anyway.
Most helpful comment
To be clear, a 200 response would include no CORS headers. That part is clear from the spec.
Here is my proposal: if the request is missing Origin or Access-Control-Request-Method, return 404 as this means the request is not a CORS request and we don't have an OPTIONS handler for those. If the request includes those two, but it fails the tests, return a 200 without any CORS headers.