Skip to content

[Security] Improve deprecation logic to minimalize the changes required in 3.0 #15886

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

Closed
wants to merge 1 commit into from

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Sep 24, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

@@ -146,6 +139,6 @@ protected function isGranted($attribute, $object, $user = null)
*/
protected function voteOnAttribute($attribute, $object, TokenInterface $token)
{
return false;
return $this->isGranted($attribute, $object, $token->getUser());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not move the trigger_error here as well? Then we don't need reflection at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm classes that don't support any attributes would not trigger a warning then.

@Tobion
Copy link
Contributor

Tobion commented Sep 24, 2015

👍

1 similar comment
@nicolas-grekas
Copy link
Member

👍

@stof
Copy link
Member

stof commented Sep 25, 2015

you should also add a deprecation warning in AbstractVoter::isGranted, in case someone overwritesvoteOnAttributebut still callsisGranted``

@wouterj
Copy link
Member Author

wouterj commented Sep 26, 2015

@stof I don't like that, as bundles supporting 2.7 and 2.8 will probably create both methods and call isGranted from voteOnAttribute, like the BC layer implemented in this PR.

@wouterj wouterj mentioned this pull request Sep 26, 2015
@wouterj wouterj closed this Sep 26, 2015
@wouterj wouterj deleted the patch-15 branch September 26, 2015 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants