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

'x-forwarded-host' should be added to proxyHeadersIgnore by default #456

Closed
hassanselim0 opened this issue Dec 15, 2020 · 3 comments · Fixed by elirehema/nbs-web#85 or elirehema/authx#9

Comments

@hassanselim0
Copy link
Contributor

The X-Forwarded-Host header is a very common header used by Reverse Proxies like Nginx and Cloudflare.
If I have both Nuxt and my API behind a reverse proxy, Nuxt gets a header like X-Forwarded-Host: my-nuxt-app.example.com so when making an API call this axios module happily copies that header over to the API Request.
When Nginx gets X-Forwarded-Host in a request it causes a lot of confusion which leads to it rewriting the Host header to be like Host: my-nuxt-app.example.com even though the request gets routed to the API Server.
This in turn confuses a lot of API Servers because they get an unexpected Host header value. Django for example has an ALLOWED_HOSTS config and uses it to secure against fake host headers.

So to summarize, I don't think there will ever be a case were a developer would want the X-Forwarded-Host header proxied by Axios, it's logically incorrect and causes a lot of confusion. This is why I think it should be part of the proxyHeadersIgnore by default and save other people the 5 hours I spent trying to debug the issue I had.

Copy link
Member

Atinux commented Dec 15, 2020

I think it’s good for me, PR welcome!

CC @pi0

@hassanselim0
Copy link
Contributor Author

@pi0 You previously added a note in the docs for proxyHeaders saying to disable it when using CloudFlare (here e9e225f), but I think now that we have the cf-* headers and soon the x-forwarded-host header ignored by default then this note is not necessary anymore, right?
Maybe it can be replaced by another note explaining the importance of having these 3 headers ignored by default so people don't accidentally fall into this issue when replacing the array with their own one.
I haven't tested this under cloudflare yet so I'm not 100% sure about it though.

@hassanselim0
Copy link
Contributor Author

@Atinux @pi0 Here is the PR: #462

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