Skip to content
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

dark: fix clone serialization issue in php8.2 #12

Merged
merged 3 commits into from
Sep 21, 2023
Merged

dark: fix clone serialization issue in php8.2 #12

merged 3 commits into from
Sep 21, 2023

Conversation

azuradara
Copy link
Collaborator

@azuradara azuradara commented Sep 21, 2023

Notes

For some reason serializing a cloned object in PHP 8.2 breaks the serialization recursion, resulting in the creation of two different serializables (different references). The serializer only keeps track of the deepest serializable in the chain, meaning that handled serializable properties can only be accessed by doing the following:

// uninitialized, the serializer does not keep a reference of this serializable
$serializable->property;

// works (if the serializer proper were public)
$serializable->serializer->serializable->property;

Reproduction steps

$cloner = new Cloner([$wrapper]);

$serialized = serialize(clone $cloner);

$deserialized = unserialize($serialized);

$wrapper = $deserialized->data[0];

$srProp = (new \ReflectionObject($wrapper))
    ->getProperty('serializer');

$srProp->setAccessible(true);
$serializer = $srProp->getValue($wrapper);

$sbProp = (new \ReflectionObject($serializer))
    ->getProperty('serializable');

$sbProp->setAccessible(true);
$serializable = $sbProp->getValue($serializer);

dump('DEPTH 1 ID: ' . spl_object_id($wrapper));
dump('DEPTH 1 DESERIALIZED: '.  isset($wrapper->something));

dump('DEPTH 3: ' . spl_object_id($serializable));
dump('DEPTH 3 DESERIALIZED: ' . isset($serializable->something));

// OUTPUT: different serializables, highest depth one isn't serialized
//         the serializer only kept track of the depth 3 one's reference

^ "DEPTH 1 ID: 1522"
^ "DEPTH 1 DESERIALIZED: "
^ "DEPTH 3: 1521"
^ "DEPTH 3 DESERIALIZED: 1"

For reference, here are the serialized values from the Cloner test class introduced in this PR without the __clone() fix:

// without clone
serialize($cloner);

O:20:"Tests\Helpers\Cloner":1:{
  s:4:"data";a:1:{
    i:0;O:21:"Tests\Helpers\Wrapper":1:{
      s:10:"serializer";O:28:"YouCanShop\Cereal\Serializer":3:{
        s:44:"\x00YouCanShop\Cereal\Serializer\x00serializations";a:1:{
          s:9:"something";s:3:"one";
          s:42:"\x00YouCanShop\Cereal\Serializer\x00serializable";r:3; // serializer isn't defined on the serialized serializable
          s:42:"\x00YouCanShop\Cereal\Serializer\x00factoryClass";s:45:"YouCanShop\Cereal\SerializationHandlerFactory";
        }
      }
    }
  }
}

// with clone
serialize(clone $cloner);

O:20:"Tests\Helpers\Cloner":1:{
  s:4:"data";a:1:{
    i:0;O:21:"Tests\Helpers\Wrapper":1:{
      s:10:"serializer";O:28:"YouCanShop\Cereal\Serializer":3:{
        s:44:"\x00YouCanShop\Cereal\Serializer\x00serializations";a:1:{
          s:9:"something";s:3:"one";
        }
        s:42:"\x00YouCanShop\Cereal\Serializer\x00serializable";O:21:"Tests\Helpers\Wrapper":1:{
          s:10:"serializer";O:28:"YouCanShop\Cereal\Serializer":3:{ // serializer is redefined
            s:44:"\x00YouCanShop\Cereal\Serializer\x00serializations";a:1:{
              s:9:"something";s:3:"one";
            }
            s:42:"\x00YouCanShop\Cereal\Serializer\x00serializable";r:7;
            s:42:"\x00YouCanShop\Cereal\Serializer\x00factoryClass";s:45:"YouCanShop\Cereal\SerializationHandlerFactory";
          }
        }
        s:42:"\x00YouCanShop\Cereal\Serializer\x00factoryClass";s:45:"YouCanShop\Cereal\SerializationHandlerFactory";
      }
    }
  }
}

@azuradara azuradara self-assigned this Sep 21, 2023
@azuradara azuradara changed the title cloning bug dark: fix clone serialization issue in php8.2 Sep 21, 2023
@azuradara azuradara added the bug Something isn't working label Sep 21, 2023
@azuradara azuradara merged commit 4aa12fe into main Sep 21, 2023
@azuradara azuradara deleted the clone-bug branch September 21, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants