Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[FrameworkBundle] mitigate BC break with empty trusted_proxies#23049

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:3.3fromxabbuh:issue-22238
Jun 3, 2017

Conversation

@xabbuh
Copy link
Member

QA
Branch?3.3
Bug fix?kind of
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#22238 (comment)
LicenseMIT
Doc PR

theofidry reacted with thumbs up emoji
@syrm
Copy link

syrm commentedJun 3, 2017

One step closer to respect semver :-)

sstok and scaytrase reacted with laugh emoji

->arrayNode('trusted_proxies')// @deprecated in version 3.3, to be removed in 4.0
->beforeNormalization()
->always()
->ifTrue(function ($v) {returnnull ===$v ||array() ===$v; })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

if (empty($v)) ? would be aligned with if !empty below, isn't it?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

indeed

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

@sstok
Copy link
Contributor

Looks good to me 👍 maybe add an exception-message expectation?

@fabpot
Copy link
Member

Thank you@xabbuh.

@fabpotfabpot merged commitff055ef intosymfony:3.3Jun 3, 2017
fabpot added a commit that referenced this pull requestJun 3, 2017
…xies (xabbuh)This PR was merged into the 3.3 branch.Discussion----------[FrameworkBundle] mitigate BC break with empty trusted_proxies| Q             | A| ------------- | ---| Branch?       | 3.3| Bug fix?      | kind of| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#22238 (comment)| License       | MIT| Doc PR        |Commits-------ff055ef mitigate BC break with empty trusted_proxies
@xabbuhxabbuh deleted the issue-22238 branchJune 3, 2017 15:56
fabpot added a commit that referenced this pull requestJun 5, 2017
… BC break (nicolas-grekas)This PR was merged into the 3.3 branch.Discussion----------[HttpFoundation][FrameworkBundle] Revert "trusted proxies" BC break| Q             | A| ------------- | ---| Branch?       | 3.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -Basically reverts#22238 + cleanups some comments + adds missing syncing logic in setTrustedHeaderName.The reason for this proposal is that the BC break can go un-noticed until prod, *even if you have proper CI*. That's because your CI may not replicate exactly what your prod have (ie a reverse proxy), so that maybe only prod has a trusted-proxies configuration. I realized this while thinking about#23049: it made this situation even more likely, by removing an opportunity for you to notice the break before prod.The reasons for the BC break are still valid and all of this is security-related. But the core security issue is already fixed. The remaining issue still exists (an heisenbug related to some people having both Forwarded and X-Forwarded-* set for some reason), but deprecating might still be enough.WDYT? (I'm sure everyone is going to be happy with the BC break reversal, but I'm asking for feedback from people who actually could take the time to *understand* and *balance* the rationales here, thanks :) )Commits-------2132333 [HttpFoundation][FrameworkBundle] Revert "trusted proxies" BC break
@fabpotfabpot mentioned this pull requestJun 5, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

6 participants

@xabbuh@syrm@sstok@fabpot@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp