-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[SecurityBundle] Don't break security.http_utils
service decoration
#60803
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
Conversation
I don't understand what this means, can you expand a bit please? |
It's about this line. The decoration does not work here. |
So this PR won't fix your issue, isn't it?
Indeed. So let's close here? |
I confirm it won't fix. I did not chose decoration, but inheritance to fix it. We could close it, but I think the current code is wrong because it you decorate the service, everything is badly wired. This patch fix it |
Why don't you decorate the url-generator BTW? |
Of course I did. But I had to change the http utils because of this, may be the error is there. I don't understand this comment TBH |
Let's do this on 7.4 then since that's not really a bugfix but more of an improved behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me on 7.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for 7.4 as this is likely to break some not-that-edge cases
896828e
to
a099ba2
Compare
I updated the base branch and rebased |
Thank you @lyrixx. |
Actually, this service is really hard to override. My use case: I want to preserve a parameter in the URL. I ended with the following code, and I'm not happy with it
This is not a real decorator. And I cannot do that, since the original service call itself
Caution
I believe my patch is good, but with it I cannot achieve anymore what I need to do 😬
Instead, I could change the class with a compiler pass