Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proxy xforward option will set bad headers in some cases #1477

Closed
nlf opened this issue Mar 10, 2014 · 6 comments
Closed

proxy xforward option will set bad headers in some cases #1477

nlf opened this issue Mar 10, 2014 · 6 comments
Assignees
Labels
Milestone

Comments

@nlf
Copy link
Member

@nlf nlf commented Mar 10, 2014

Due to lib/proxy.js lines 59-61, if the mapUri method returns x-forwarded-for, x-forwarded-proto, or x-forwarded-port headers, their values can end up corrupt in many situations.

When using server.inject the x-forwarded-for and x-forwarded-port headers will end with a trailing comma, due to request.info.remoteAddress and request.info.remotePort containing invalid values.

If the x-forwarded-proto header is set in mapUri, then the resulting x-forwarded-proto header will end with ',undefined' since settings.protocol cannot be specified if mapUri is, and as such will be undefined

@nlf nlf added the bug label Mar 10, 2014
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Mar 11, 2014

Can you add a test showing this?

@nlf
Copy link
Member Author

@nlf nlf commented Mar 11, 2014

I actually have one in the test_coverage branch, here

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Mar 11, 2014

What's your ETA about a PR? I want to do 3.0 push today.

@nlf
Copy link
Member Author

@nlf nlf commented Mar 11, 2014

Hoping to get it done this morning, I've got 4 lines left missing coverage.

Two of them are console.error calls, which are annoying since I feel like that means I need to mock globals.console so I can be sure that the error method runs. The other two are just complicated ternary if statements that I haven't deciphered yet.

I'm hung up for the next half-hour to an hour, so if you beat me to it my feelings won't be hurt.

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Mar 11, 2014

want to PR what you have so far?

@nlf
Copy link
Member Author

@nlf nlf commented Mar 11, 2014

done, see #1482

@hueniverse hueniverse added this to the 5.0.1 milestone May 24, 2014
@hueniverse hueniverse self-assigned this May 24, 2014
@lock lock bot locked as resolved and limited conversation to collaborators Jan 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants