[v5] Lock down valid status code ranges to 1xx through 5xx

[jonchurch]

Node's http module accepts status codes from 100 up to 999.

Do we want to lock this down further in express to only accept status codes in the 1xx to 5xx ranges? Proposed behavior is to throw a RangeError when encountering one outside the accepted range.

Im for it.

[UlisesGascon]

The status code of a response is a three-digit integer code that describes the result of the request and the semantics of the response, including whether the request was successful and what content is enclosed (if any). All valid status codes are within the range of 100 to 599, inclusive. RFC9110 Reference

The specs are clear on this, I am +1

[jonchurch]

Removed help wanted labels bc I have a PR open which does this in https://github.com/expressjs/express/pull/4212

[wesleytodd]

Sorry I was just going randomly based on which notification I clicked first. Any reason not to have the discussion just in the PR then?

[jonchurch]

It's semantics, but I see issues as long lived to decide what to code, and PRs to talk about how it is implemented.

@wesleytodd This issue's goal is to discuss what ranges are valid.

Should throw when someone does res.status(999)?

The PR itself would handle how we implement throwing on invalid status codes. Which includes using strings and floats.

That PR could always be closed and nuked, so all that conversation would be separated from whatever PR ends up implementing. (I actually force pushed it after rebasing away from v4 to 5.x branch, so some of the convo in there isn't that relevant anyways). That PR is also like 4 years old now.

[jonchurch]

@blakeembrey brought up a good point, do we want to take this opinionated approach which would prevent anyone from building something on top of express which defines custom http status codes? I don't really know, bc I am not aware of anyone who does that.

edit: that's not how this works actually, these would be strings, which we've already said in the past we wanted to make res.status only take integers anyways. So this example is moot. ~The benefit I see is that it would prevent folks doing something weird like if (err) { res.status(err.code ?? 500)~

But that's not a huge benefit really, compared to the reduction in flexibility.

I see express as an HTTP framework, and 999 as an invalid HTTP status code. It feels natural to me to take the HTTP spec's definition of a valid status code.

If folks complain, we could add res.statusNoThrow(999) at the request of real users know who what they're doing

[blakeembrey]

There's modules written jokingly using express 7xx: https://github.com/devmaximilian/http-700

There also appears to be some valid reason for a CDN to support your server returning a non-standard code: https://www.fastly.com/documentation/reference/http/http-statuses/

I'm in favor of not constraining it, unless it turned out this was a large source of error for users. Both sides seem kind of weak so I'm more in favor of staying un-opinionated.

I'd love to take a strong stance either way, not a weak stance adding two different methods or flags anywhere. The only way I'd be in favor of something like that is to potentially have a "HTTP status code" map and if it's not in the map throw. E.g. if (!res.supportedStatuses[x]) throw ... as this does not impact performance or make support more complex.

[UlisesGascon]

I agree with @blakeembrey. I've seen cases where people use status codes outside of the standard for ad-hoc integrations and edge cases.

Perhaps we could add this and other similar spec "opinionated" features into a separate middleware? This could follow the helmet approach for security but focus on a more strict/opinionated way of using Express. Just to clarify, I love that Express is not opinionated, but maybe we could provide easy plugins for end users 🙂

[jonchurch]

Sounds like a plan! Let's not limit the range of status codes in v5.

To be clear, we are in agreement still that we want to throw a RangeError if someone attempts to set a status code outside of what Node expects?

That was the original feature in #4212 , Node throws if given res.status(1000 and you can't really handle it in Express currently. Hence throwing from express internals

[blakeembrey]

To be clear, we are in agreement still that we want to throw a RangeError if someone attempts to set a status code outside of what Node expects?

SGTM, in favor of accepting only integers >= 100 and < 1000.

[wesleytodd]

Totally 👍 to this decision. I think we could add a feature to enable "strict status codes" or something but that would be a minor addition and could land any time.