-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Error in PostAuthenticationGuardToken::serialize because of bug in PHP 7.3 #29951
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
Comments
I'm not sure we can do anything in Symfony itself, this is a PHP 7.3 bug. |
Another reason to deprecate |
I don't believe we can do anything on the PHP side either, short of introducing a new serialization mechanism. I believe that the only change here in PHP 7.3 is that it has some stricter validation, which makes unserialization fail rather than silently returning bogus data. If you use the Serialization interface, you need to write your code in a way that invokes serialization / unserialization in the same order. That generally means that |
Basically what you need to do is to make sure that you first assembly all the data you want to serialize and then do a single call to I have no idea whether you can do such a change in this case, or whether this runs afoul of BC guarantees. |
Thanks for the details, let's see what we can do then. That's highly unexpected but I understand there are valid technical reasons. |
An easy solution would be to make the properties in |
@toooni Adding a method that just returns the data to be serialised would be better to not make all the data modifiable. But if I don't miss anything, this still means that the serialised representation would change with the following update of any application thus invalidating all existing serialised tokens. |
@nicolas-grekas Okay, I'll try to create an implementation and RFC for a new serialization mechanism sometime soon. |
@nikic awesome thanks, keep me posted if you want any feedback! |
"An easy solution would be to make the properties in Symfony\Component\Security\Core\Authentication\Token\AbstractToken protected instead of private and use them in PostAuthenticationGuardToken to serialize/unserialize only once." Thank you so much, that worked! Seriously I've been stumped for a week over this. I think I'll post my exact code. That way anyone else can just copy and paste. 1. In AbstractToken.php lines 27-30 make these protected. protected $user;
protected $roles = array();
protected $authenticated = false;
protected $attributes = array(); 2. In PostAuthenticationGuardToken.php locate the serialize() function and replace it with the code below. public function serialize() {
return serialize(array(
$this->providerKey,
\is_object($this->user) ? clone $this->user : $this->user,
$this->authenticated,
array_map(function ($role) { return clone $role; }, $this->roles),
$this->attributes ) );
} 3. Again in PostAuthenticationGuardToken.php locate the unserialize() function and replace it with the following code. public function unserialize($serialized) {
list($this->providerKey, $this->user, $this->authenticated, $this->roles, $this->attributes) = unserialize($serialized);
} 4. Thats it. Just be sure to clear your cache in your console.
|
After some history, this bug is "known" since 2013, but hidden in #9806 |
As another possibility for working around this: public function serialize()
{
$s1 = serialize($this->providerKey);
$s2 = serialize(parent::serialize());
return bin2hex($s1) . ',' . bin2hex($s2);
}
public function unserialize($serialized)
{
list($s1, $s2) = explode(',', $serialized);
$this->providerKey = unserialize(hex2bin($s1));
parent::unserialize(hex2bin($s2));
} This avoids the nested serialization and allows the serialize and unserialize calls to happen in the same order. (This is actually somewhat similar to what internal classes using Serializable do.) |
I've just prototyped a working patch based on an optional argument When set, the serialize method returns an array. "unserialize" then just checks with Works like a charm. PR coming soon. |
Here is the PR implementing the strategy I mentioned: #30006 |
…rekas, Renan) This PR was merged into the 3.4 branch. Discussion ---------- [Security] don't do nested calls to serialize() | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29951 | License | MIT | Doc PR | n/a The problem (originally reported as `Symfony\Component\Security\Core\Authentication\Token\AbstractToken` issue), may occur also in classes extending `Symfony\Component\Security\Core\Exception\AuthenticationException` Tasks: - [x] Skip native serializer (workaround itself) - [x] Token test - [x] Exception test Commits ------- 10256fc skip native serialize among child and parent serializable objects 41000f1 [Security] dont do nested calls to serialize()
Symfony version(s) affected: 2.8.0-4.2.2 (ONLY with PHP 7.3+)
Description
A bug in PHP7.3 (https://bugs.php.net/bug.php?id=77302) which was also discussed in #29459 creates an issue where an object (sometimes?) cannot be serialized twice.
This is what happens in
PostAuthenticationGuardToken::serialize
/PostAuthenticationGuardToken::unserialize
:Which results in an error
Notice: unserialize(): Error at offset x of y bytes
when using symfony guard.How to reproduce
See https://bugs.php.net/bug.php?id=77302
Possible solution
I don't know how to fix this without a BC break. Since it's basically a PHP bug only affecting some users I also don't know how to proceed.
The text was updated successfully, but these errors were encountered: