test: cover function-based "trust proxy" hop behavior#7387
Open
BIGSUS24 wants to merge 1 commit into
Open
Conversation
Closes expressjs#6541. When "trust proxy" is set to a function, req.ip calls it once per forwarded hop as fn(addr, i) starting from the socket address, stopping at the first hop it doesn't trust. This was undocumented and untested, which was the actual bug in expressjs#6541 (the reported call pattern was working as intended, just surprising). Adds test/req.ip.js coverage for the call sequence, the resolved address, and the no-XFF-header case, plus a short doc comment on the ip getter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6541.
trust proxyas a function is undocumented and had zero test coverage, which is exactly why #6541 got filed as a bug when it isn't one.req.ipcalls the function once per forwarded hop viaproxyaddr,fn(addr, i), starting from the socket address (i === 0) and moving outward throughX-Forwarded-For, stopping at the first hop that returns false. That's the intended behavior per proxy-addr, just never pinned down anywhere.@dougwilson said in the issue thread that undocumented/untested behavior like this is exactly what he wants contributors to lock down, so here's that:
(addr, i)call sequencereq.ipresolves to the first untrusted hopX-Forwarded-Foripgetter describing the callback contractNo production logic changed, the getter itself is untouched. Ran the full suite locally (1263 passing) plus lint, both clean.