Skip to content

[HttpFoundation] reintroduce set trusted header name in request #32961

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[HttpFoundation] reintroduce set trusted header name in request #32961

wants to merge 1 commit into from

Conversation

brambaud
Copy link

@brambaud brambaud commented Aug 5, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #26333
License MIT
Doc PR TODO

I think it could be great to reintroduce in a simple way the ability to support custom header names .

Indeed, not everyone can use "standard" header name (see fideloper/TrustedProxy#108 for instance).
Some people start using workaround like https://github.com/jdavidbakr/CloudfrontProxies.

I personally need it for some Drupal sites.

@brambaud brambaud force-pushed the issue/26333/custom-headers branch from 55e3355 to d9c3cf4 Compare August 5, 2019 18:57
@nicolas-grekas nicolas-grekas added this to the next milestone Aug 6, 2019
@nicolas-grekas
Copy link
Member

I pretty much like what https://github.com/jdavidbakr/CloudfrontProxies does.
It does more than just turning cloudfront-forwarded-proto into x-forwarded-proto, since it also loads the list of trusted proxies.
Not sure we need more, don't you think?

@brambaud
Copy link
Author

brambaud commented Aug 7, 2019

https://github.com/jdavidbakr/CloudfrontProxies does more its true, that was just an example.

Indeed the workaround is to do something like
$_SERVER['HEADER_X_FORWARDED_PROTO'] = $_SERVER['HEADER_X_CUSTOM_FORWARDED_PROTO'];

It feels a bit "hacky" to me nevertheless I agree that it could be enough for the majority.

@nicolas-grekas
Copy link
Member

@brambaud would you mind opening a doc issue or PR, then close this issue please if you're OK to?

@brambaud
Copy link
Author

brambaud commented Aug 7, 2019

I'm ok :)
I'll open a PR shortly to update the documentation then I'll close this one. Thanks!

@brambaud brambaud mentioned this pull request Aug 7, 2019
Merged
@brambaud
Copy link
Author

brambaud commented Aug 7, 2019

Closing in favor of symfony/symfony-docs#12117

@brambaud brambaud closed this Aug 7, 2019
OskarStark added a commit to symfony/symfony-docs that referenced this pull request Aug 16, 2019
@OskarStark
minor #12117 add support for custom headers when using a proxy (bramb…
a6f4536 
…aud)

This PR was merged into the 3.4 branch.

Discussion
----------

add support for custom headers when using a proxy

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/roadmap for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->

See symfony/symfony#32961 and symfony/symfony#26333.

We should document how to use custom headers when using reverse proxies since `Request::setTrustedHeaderName()` has been deprecated.

Commits
-------

a63c5a6 add support for custom headers when using a proxy
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature HttpFoundation Status: Needs Review
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
3 participants
@brambaud @nicolas-grekas @carsonbot