Proposal to add route option for CORS preflight status code

[devinivy]

Support plan

Community

• is this issue currently blocking your project? (yes/no): no
• is this issue affecting a production system? (yes/no): no

Context

• node version: v12+
• module version: v19+
• environment (e.g. node, browser, native): node
• used with (e.g. hapi application, another framework, standalone, ...): hapi application
• any other relevant information: PR fix(cors): emptyStatusCode not being taken into account for OPTIONS requests #4024 was originally opened to address the same underlying issue.

What problem are you trying to solve?

Users would like to be able to control the status code used to respond to CORS preflights. Currently the only way to do this is to configure the global server setting routes.response.emptyStatusCode. This has two drawbacks:

1. It is not necessarily desirable to couple the response code of preflights with the empty response code used by other endpoints. One reason for this is that browser quirks over time have shifted what an ideal preflight response code should be (200 vs 204). The client for preflights is the browser, and the client for other endpoints is likely the user's application.
2. It is not possible to configure this on a per-route basis. The main downside here is that it is confusing that routes.response.emptyStatusCode configured on the server takes effect, but the analogous route-level setting options.response.emptyStatusCode does not.

Do you have a new or modified API suggestion to solve the problem?

In order to address both points above my recommendation is to introduce a new route option cors.preflightStatusCode to control the preflight status code, which is respected both by global server settings and on a per-route basis. This would be a breaking change, as users are currently using the global routes.response.emptyStatusCode setting to control this, a workaround provided by @kanongil in #4024. I propose the two be totally decoupled, however there are backwards-compatible ways to introduce this feature prior to decoupling the two options.

[kanongil]

I don't see any reason for preflight responses to ever return code 204, so I would much rather just change it to 200 than introduce another option. The only cost of this is 19 extra transferred bytes for the Content-Length: 0 header (and less with http2 & header compression).

I would expect such a change to be labelled as a bugfix, and not a breaking change.

This was only an "issue" when Chrome could erroneously log a CORB warning for such responses until it was fixed in v69.

[devinivy]

Can you help me understand the merits of a 200 versus a 204? I'm not terribly worried about the 19 bytes, but I do wonder if we'd be going against the grain, e.g. against MDN's recommendations (updated after Chrome 69 was released), the direction kong is heading, and how express/cors has essentially always handled these responses without much issue (though they also have an option). Is the idea that 200 offers better support for certain legacy clients, or is it something else?

I definitely would prefer not having to implement an option for this. However a. I do think we ought to decouple CORS responses from emptyStatusCode, which I would probably list as a breaking change and b. I think if there's any merit to introducing an option it's that it becomes the user's issue to reconfigure their server before it's hapi's issue to change this finicky behavior in case the situation continues to shift in the future.

[kanongil]

The point is that 200 works everywhere, and is likely to continue doing so forever, while 204 can break in some configurations.

I don't have much regard for your MDN "recommendation", which can just be a single persons opinion that no one had a strong enough opinion to object (its a wiki). In this case a person with only that specific agenda: https://wiki.developer.mozilla.org/en-US/profiles/gshutler.

The Kong change seems to be motivated by MDN, and has not been completed and is questioned.

I don't know about express, but the decision already includes a compatibility hack!

I'm all for the decoupling. As I don't see 200 ever not working, or any actual benefit of a 204 (only a nerdy feeling of "doing it right"), I'd rather this just be hardcoded. You are welcome to do this as a breaking change, but we are well within semver guarantees to do it as a bugfix, as the docs provide no mention on what kind of status code is used for preflight responses. In reality it is not going to break anything (besides maybe an unit test or two). But please challenge me on this, if you feel I'm wrong.

[devinivy]

The point is that 200 works everywhere, and is likely to continue doing so forever, while 204 can break in some configurations.

Ahh I see— I haven't been able to find clarity on that 😭. If this is true: "hapi isn't supporting some known cases by serving a 204, but could support all cases by serving a 200" then I am all-in on your suggestion. The lack of an authority on the question only causes confusion for us poor web framework collaborators doesn't it 😆 . In my experience the knowledge about this is scattered throughout issues trackers across the web, mostly with incomplete details, partial solutions, and almost never with any clarity.

My impression from all that has been: "204s have worked on modern clients for a while so tools have been moving back that direction, and 200s work but have been a moving target by causing intermittent warnings even in modern clients." This lead me to believe there wasn't a one-size-fits-all solution, which is why I proposed adding an option. From what you're saying it seems like that may not be correct, though! I have no interest in serving a 204 in order to "do it right"— I similarly just want to land on a solution that will allow CORS to always work without warnings or other issues. Also I hadn't noticed that compatibility hack in express for 204s, ouch!

Finally, the more I think about it, the more I agree that decoupling emptyStatusCode behavior from this doesn't need to be a breaking change. I see your perspective about the docs, but for me it's less because of the docs and more because: if anyone is using that functionality then they are just using it to serve a 200 anyway. And I agree that changing the default status code for preflights is within our purview.

TLDR: I am down with your suggestion as long as yourself and others have a high confidence we can serve a 200 that will work without causing issues or warnings. In making a change to the status code it would also be useful if we could cite a concrete case where the current behavior serving a 204 causes issues. Then we could file a bug report and address it.

[devinivy]

Resolved with v21 #4386