express
express copied to clipboard
expressjs
express checks referrer first, then referer next
Looking at this, https://github.com/expressjs/express/blob/master/lib/request.js#L79
It seems like express checks referrer (with two "r"s) first, then referer next.
Is there a particular reason why the double-r is checked first?
It seems like single-r should be checked first because it's standard.
Another reason to check single-r first is because it's easier to spoof double-r than it is to spoof single-r (with a web browser, anyway). Some browsers block attempts to modify the single-r header. But I don't think any browser blocks attempts to spoof double-r.
If it were me, I'd even go as far as to not check double-r at all. But that would be a breaking change and one can easily do this by just accessing req.headers.referer and not using req.header().
I'm just curious why req.header() was made to behave this way.
Hi @AnyhowStep thanks for your question. So the logic was added here: https://github.com/expressjs/express/commit/b65b0636aa2890f86b4ec7baddd289c9fb1ca043
At the time, there was a general effort floating around to "fix" the spelling of the header, and servers would back this effort by looking for the correctly-spelled header, which Express did.
Since that time, it never really took off and then with the rewriting of the RFC into the 72xx series, the misspelled name was solidified further.
I believe the special case can just be removed these days, only checking the misspelled, standard name. This can easily be landed in the 5.0 branch.
Should we add a 5.x tag to keep track of this issue?
@dougwilson, I can make a PR to strip the "referrer" special casing out of https://github.com/expressjs/express/blob/5.0/lib/request.js if you want that changed. Let me know if you'd prefer it gone :)
@rileyjshaw, I think it would be best to start with printing a deprecation warning when trying to access req.headers.referrer if it was set. That would probably be what we would merge in 5.0 with a removal in 6.0.
@wesleytodd sounds good! Something like:
console.warn('Deprecation warning: the "Referrer" header is non-standard, and special-casing for it will be dropped in version 6.0. See https://github.com/expressjs/express/issues/3951 for further details.');
?
Preferably more like this:
https://github.com/expressjs/express/blob/master/lib/application.js#L512
@wesleytodd sounds good! Sorry for the delay, I just added a PR to the 5.0 branch (https://github.com/expressjs/express/pull/4106). Let me know if you'd like to see any changes.