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

support X-Forwarded-Context header when xheaders are trusted #2601

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minrk
Copy link
Contributor

@minrk minrk commented Mar 1, 2019

allows proxies that forward /prefix/foo -> /foo to be handled under their original url

I can imagine wanting this to be a separate opt-in flag from the existing xheaders since it affects request routing, which hasn't been affected previously. It also seems reasonable to perhaps want one behavior and not the other. Any ideas on how best to specify it as an extra option, if that makes sense?

allows proxies that forward /prefix/foo -> /foo to be handled under their original url
@bdarnell
Copy link
Member

bdarnell commented Mar 1, 2019

I've wanted xheader configuration to be more flexible for a while (#763), especially since we support multiple names for the forwarded-IP header which makes it easily spoofed (unless you configure your proxy to strip the others out). There's also the trusted_downstream option which was added in #1864 without much consideration of a holistic solution (and I think it could also be useful to have the ability to trust some number of proxy layers without knowing their exact addresses).

The simple approach suggested in #763 was to allow xheaders to be a set of strings for the header names that would be examined. That would still leave trusted_downstream and other modifiers separate. To bring those in we could make xheaders a dict, although that gets more awkward in the common case.

Even if we make it more configurable, I think it might be better for xheaders=True to enable all xheaders instead of the set of things that happen to be supported today. But maybe x-forwarded-context really is different - it undoes what the proxy does so it's more of a workaround for a proxy config that's out of your control than something you'd expect for all users of a proxy.

Is there a reason apply_forwarded_context is on HTTPServer, instead of on the context object like apply_xheaders? I think I'd rather make apply_xheaders the sole entry point for this from the HTTPServer.

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

Successfully merging this pull request may close these issues.

2 participants