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

[Security] Deprecate callable firewall listeners#60614

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

Conversation

@MatTheCat
Copy link
Contributor

@MatTheCatMatTheCat commentedJun 1, 2025•
edited
Loading

QA
Branch?7.4
Bug fix?no
New feature?no
Deprecations?yes
IssuesN/A
LicenseMIT

After spending some time in the Security component it occurred to me callable firewall listeners are obsolete now that we got theFirewallListenerInterface. Their deprecation has already been suggested (like in#34627 (comment) or#38751 (review)), so this PR does it.

Copy link
Contributor

@SpomkySpomky left a comment

Choose a reason for hiding this comment

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

Looks good. Just one remark about the changlog.

@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch 2 times, most recently fromf2b8c5e to311c89fCompareJune 5, 2025 08:40
@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch from311c89f to253ed34CompareJune 11, 2025 08:34
@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch 3 times, most recently from225e787 tofd98438CompareJune 12, 2025 21:11
@MatTheCat
Copy link
ContributorAuthor

Just noticed that theLazyFirewallContext can also run the firewall listeners so I added the deprecation in there. I also moved the changelog entry to thesecurity-http’s instead of thesecurity-bundle’s.

@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch fromfd98438 to2b96194CompareJune 12, 2025 21:24
@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch 4 times, most recently from7dd75b6 to2b4d065CompareJune 15, 2025 11:59
@MatTheCat
Copy link
ContributorAuthor

With this PRAbstractListener::__invoke shouldn’t be called anymore, should I deprecate it as well? That would only leave itsgetPriority method so maybe theAbstractListener can be deprecated entirely?

@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch from2b4d065 to3b041a7CompareJune 17, 2025 14:05
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.

Let's keep AbstractListener, it's not a big deal - but let's deprecate its __invoke method.

}

privatefunctiongetListenerPriority(object$logoutListener):int
privatefunctiongetListenerPriority(object$listener):int

Choose a reason for hiding this comment

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

let's remember to inline this method on 8.0

@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch 4 times, most recently fromc1a12a9 to11f9c3fCompareJune 23, 2025 10:00
@MatTheCat
Copy link
ContributorAuthor

Let's keep AbstractListener, it's not a big deal - but let's deprecate its __invoke method.

Done; I also deprecatedLazyFirewallContext::__invoke() for the same reason.

@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch 3 times, most recently from41ffe67 toa516e13CompareJune 23, 2025 10:29
@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch froma516e13 to5234217CompareJune 23, 2025 16:02
@MatTheCatMatTheCatforce-pushed thedeprecate-callable-firewall-listener branch from5234217 to510e506CompareJune 23, 2025 16:04
@MatTheCat
Copy link
ContributorAuthor

Just to be sure:WrappedListener doesn’t need to be deprecated because it’s@internal?

nicolas-grekas reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

Thank you@MatTheCat.

MatTheCat reacted with hooray emoji

@nicolas-grekasnicolas-grekas merged commit868f3dc intosymfony:7.4Jun 23, 2025
10 of 11 checks passed
@MatTheCatMatTheCat deleted the deprecate-callable-firewall-listener branchJune 23, 2025 16:19
nicolas-grekas added a commit that referenced this pull requestJun 24, 2025
…(MatTheCat)This PR was merged into the 8.0 branch.Discussion----------[Security] Remove callable firewall listeners support| Q             | A| ------------- | ---| Branch?       | 8.0| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | N/A| License       | MITFollow-up of#60614Now that firewall listeners must implement `FirewallListenerInterface`, their traceable version’s stub can be their class name again.Commits-------fcadbf0 Remove callable firewall listeners support
This was referencedOct 27, 2025
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

@SpomkySpomkySpomky approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

6 participants

@MatTheCat@nicolas-grekas@Spomky@chalasr@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp