Skip to content

Conversation

ddeboer
Copy link
Contributor

@ddeboer ddeboer commented Dec 17, 2013

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

parent::serialize() and parent::unserialize(), which are used in the AbstractToken are problematic in PHP >= 5.4. Cloning the object before serialization seems to fix this.

return serialize(array($this->user, $this->authenticated, $this->roles, $this->attributes));
return serialize(
array(
\is_object($this->user) ? clone $this->user : $this->user,
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm why not just is_object()?

@ddeboer
Copy link
Contributor Author

ddeboer commented Dec 18, 2013

@cordoval Agreed, I got rid of the leading backslashes.

fabpot added a commit that referenced this pull request Dec 23, 2013
This PR was squashed before being merged into the 2.2 branch (closes #9806).

Discussion
----------

[Security] Fix parent serialization of user object

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

`parent::serialize()` and `parent::unserialize()`, which are used in the `AbstractToken` are [problematic](https://bugs.php.net/bug.php?id=62836) in PHP >= 5.4. [Cloning the object](https://gist.github.com/aurelijus/4713758) before serialization seems to fix this.

Commits
-------

2e4670d [Security] Fix parent serialization of user object
fabpot added a commit that referenced this pull request Dec 23, 2013
This PR was submitted for the 2.2 branch but it was merged into the 2.3 branch instead (closes #9806).

Discussion
----------

[Security] Fix parent serialization of user object

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

`parent::serialize()` and `parent::unserialize()`, which are used in the `AbstractToken` are [problematic](https://bugs.php.net/bug.php?id=62836) in PHP >= 5.4. [Cloning the object](https://gist.github.com/aurelijus/4713758) before serialization seems to fix this.

Commits
-------

e0bb891 [Security] Fix parent serialization of user object
@fabpot fabpot closed this Dec 23, 2013
@g-g
Copy link

g-g commented Jan 14, 2014

Guys, this breaks my code :-(
Updating to 2.4.1 breaks my security code with all kind of weird unserialize problems, token::getUser does not return a user, but a token.
I think this comes from PHP bug 64146: https://bugs.php.net/bug.php?id=64146
I am generally not happy by this process: changing the framework because of a PHP bug, and basing the change on a single opinion that this might help.
The original (2.4.0) code works fine on PHP 5.5.3, but the PR introduces new problems.

@leftouterjoins
Copy link

This breaks my code as well. We have special and necessary behavior built in to our Entity's clone behavior so when this went live in 2.3.9 it broke our code. Cloning an entire object seems like a very expensive and unexpected behavior in this situation. I'm seeing if there is perhaps another way around the PHP bug.

@sstok
Copy link
Contributor

sstok commented Mar 12, 2014

Is it possible to solve this using a __clone() method providing some special operation?

@g-g
Copy link

g-g commented Mar 12, 2014

This is a weird problem. While doing some tests on my systems Ubuntu 13.10, PHP 5.5.3 i got strange results (with a rather complicated token, as it contains another token in itself):
serialize w/o clone works
serialize w clone w/o __clone() does not
serialize w clone w __clone() does not
serialize w clone w __clone() where in __clone the object is just unserialized from the old object doesn't work either :-(
Result: I patch the symfony src and remove the clone(), which is of course bad for the production system :-(

@ddeboer
Copy link
Contributor Author

ddeboer commented Mar 12, 2014

@MacinJosh Please let us know if you find a better workaround.

@sstok What do you mean exactly?

@g-g Do your nested tokens also contain full user objects? My fix was necessary for correct serialization of the SwitchUserRole, where a token contains a set of roles and each role can itself contain a token. When those nested tokens contain user objects, the PHP serialization bug occurs.

@g-g
Copy link

g-g commented Mar 12, 2014

Yes, the nested tokens may have user objects. But all user objects implement Serializable, and this works well for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants