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

Using express vhost behind a reverse proxy #20

Open
domharrington opened this issue Dec 7, 2016 · 3 comments
Open

Using express vhost behind a reverse proxy #20

domharrington opened this issue Dec 7, 2016 · 3 comments
Assignees

Comments

@domharrington
Copy link

domharrington commented Dec 7, 2016

I'm using express vhost behind a reverse proxy and i want to use this module based on the x-forwarded-host header as opposed to the host header that is hardcoded here:

vhost/index.js

Line 77 in 12565d1

var host = req.headers.host

Express 4 supports this by accessing req.hostname directly: http://expressjs.com/en/4x/api.html#req.hostname

Express 3 supports this at req.host:
http://expressjs.com/en/3x/api.html#req.host

Is this module still intended to support express v3? Would you accept a pull request that replaces:

vhost/index.js

Line 77 in 12565d1

var host = req.headers.host

with:

var host = req.hostname || req.host

to keep support with both express v3 and v4.

EDIT:
I've just re-read the readme again and this would be the only part of this module which adds a direct express dependency. Maybe the code could be:

var host = req.hostname || req.host || req.headers.host

To support express 3/4 and connect servers.

domharrington pushed a commit to domharrington/vhost that referenced this issue Dec 7, 2016
…ixes expressjs#20

This is so that the module works when express has `trust proxy` enabled
and the `Host` header is expected to come from `X-Forwarded-Host`
@dougwilson dougwilson self-assigned this Feb 26, 2017
@roccomuso
Copy link

Thanks +1

@Bramzor
Copy link

Bramzor commented Mar 5, 2019

We really need to get the pull request in because people are hitting this issue a lot and the fix is so easy...

@ty-fleming
Copy link

I agree, please get to this pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants