Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Security] Adding __toString() method to Role class#10179
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
This will produce a fatal error if$this->role isnull. It is allowed by theRoleInterface.
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.
good spot, will fix and add a test
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.
Added tests to ensure that the__toString() behaves as expected, although it was never possible for it to be null because the role was immutable and cast to a string in the constructor
jameshalsall commentedApr 13, 2014
@fabpot is this okay to be merged? |
Tobion commentedApr 13, 2014
It cannot be merged into 2.3. Also the toString doesn't help much when it's not part of the interface. So it would need to be added there as well which makes it a BC break. Could be worth, not sure. |
jameshalsall commentedApr 14, 2014
@Tobion I'm not sure the interface would actually require the It certainly eases things when you're trying to compare an array of |
Tobion commentedOct 9, 2014
fabpot commentedFeb 5, 2015
Closing in favor of what I explained in#8313 (comment) |
This PR proposes adding the
toString()method to the\Symfony\Component\Security\Core\Role\Roleclass. This should ease the comparison of role strings and instances ofRoleitself. For example, in theFOSUserBundletheUsermodel contains roles which are represented as strings. This makes comparisons between arrays ofRoleobjects and role strings difficult without creating your ownRoleclass with a__toString(), and this isn't always desirable.I found some conversation aroundadding the
\Seriazableinterface to theRoleclass, and I realise that this might also be considered a BC break if people are relying on the fact that it will not cast to a string.