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

[SecurityBundle] Remove autoconfiguration forProcessorInterface#45422

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

Closed

Conversation

@derrabus
Copy link
Member

@derrabusderrabus commentedFeb 14, 2022
edited by carsonbot
Loading

QA
Branch?6.1
Bug fix?no
New feature?no
Deprecations?no
TicketsN/A
LicenseMIT
Doc PRN/A

This PR removes autoconfiguration logic for aSymfony\Bridge\Monolog\Processor\ProcessorInterface that does not exist. As far as I can tell, this has always been unused code, so we might as well remove it.

@carsonbotcarsonbot added this to the6.1 milestoneFeb 14, 2022
@carsonbotcarsonbot changed the titleRemove autoconfiguration forProcessorInterface[SecurityBundle] Remove autoconfiguration forProcessorInterfaceFeb 14, 2022
@derrabusderrabusforce-pushed theremove/processor-interface branch fromfb94f3a to33553eeCompareFebruary 14, 2022 17:09
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedFeb 14, 2022
edited
Loading

I guess that's a wrong namespace, maybe related to history/renames?
Seehttps://github.com/Seldaek/monolog/blob/main/src/Monolog/Processor/ProcessorInterface.php
Git blame will point at me for the removed lines, and I'm sure I added them for a reason (can't recall right now.)

@derrabus
Copy link
MemberAuthor

I guess that's a wrong namespace, maybe related to history/renames?

The pass was added in#33663 and those lines haven't changed since then. I've browsed the source tree that very commit but the bridge did not have that interface at that time either.

I dug deeper and apparently, the interface did exist for a brief moment: Introduced in#27801, reverted in#28845.

I assume that the lines are an artifact of an earlier iteration of that PR. Since they effectively never did anything, the fix for me is to remove them.

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedFeb 14, 2022
edited
Loading

The purpose of those lines was to wire processors automatically with the untracked token storage, so that they could log things found in the session without triggeringCache-Control: private on the response.

We should fix this behavior of it's broken.

@derrabus
Copy link
MemberAuthor

Should we build that logic in MonologBundle instead?

@nicolas-grekas
Copy link
Member

I was having a look, monolog bundle already registers the tag for autoconfiguration:
https://github.com/symfony/monolog-bundle/blob/fde12fc628162787a4e53877abadc30047fd868b/DependencyInjection/MonologExtension.php#L131-L134

What this does is the binding. And we could argue that the binding is for SecurityBundle.

To me, we should fix the namespace in the lowest affected branch.

nicolas-grekas added a commit that referenced this pull requestFeb 18, 2022
…terface (nicolas-grekas)This PR was merged into the 4.4 branch.Discussion----------[SecurityBundle] fix autoconfiguring Monolog's ProcessorInterface| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#45422| License       | MIT| Doc PR        | -Commits-------7712d80 [SecurityBundle] fix autoconfiguring Monolog's ProcessorInterface
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@wouterjwouterjAwaiting requested review from wouterjwouterj is a code owner

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

3 participants

@derrabus@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp