From e463a4536cce2580ce7619027548b9350af2b4b9 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Mon, 29 Nov 2021 15:13:59 +0100 Subject: [PATCH 1/7] [3.6] Regression: ArrayObject with protected ARRAY_AS_PROPS cannot be serialized anymore Signed-off-by: Filippo Tessarotto --- test/ArrayObjectTest.php | 11 +++++++++++ test/CustomArrayObject.php | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 test/CustomArrayObject.php diff --git a/test/ArrayObjectTest.php b/test/ArrayObjectTest.php index 0f626847..9cf8cf73 100644 --- a/test/ArrayObjectTest.php +++ b/test/ArrayObjectTest.php @@ -401,4 +401,15 @@ public function testSerializationRestoresProperties(): void self::assertEquals($ar, unserialize(serialize($ar))); } + + public function testSerializationRestoresProtectedProperties(): void + { + $ar = new CustomArrayObject([], ArrayObject::ARRAY_AS_PROPS); + self::assertTrue($ar->isImmutable()); + + $serialized = serialize($ar); + $unserialized = unserialize($serialized); + + self::assertTrue($unserialized->isImmutable()); + } } diff --git a/test/CustomArrayObject.php b/test/CustomArrayObject.php new file mode 100644 index 00000000..5e85bc37 --- /dev/null +++ b/test/CustomArrayObject.php @@ -0,0 +1,17 @@ +isImmutable; + } +} From e3303337513bb2c12bf11e9bb757594f5bcc907d Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Mon, 29 Nov 2021 15:36:05 +0100 Subject: [PATCH 2/7] Tests green again Signed-off-by: Filippo Tessarotto --- src/ArrayObject.php | 75 ++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 41 deletions(-) diff --git a/src/ArrayObject.php b/src/ArrayObject.php index 0e706db0..015ea1cc 100644 --- a/src/ArrayObject.php +++ b/src/ArrayObject.php @@ -16,7 +16,6 @@ use function asort; use function class_exists; use function count; -use function get_class; use function get_object_vars; use function gettype; use function in_array; @@ -92,7 +91,7 @@ public function __isset($key) } if (in_array($key, $this->protectedProperties)) { - throw new Exception\InvalidArgumentException('$key is a protected property, use a different key'); + throw new Exception\InvalidArgumentException("$key is a protected property, use a different key"); } return isset($this->$key); @@ -113,7 +112,7 @@ public function __set($key, $value) } if (in_array($key, $this->protectedProperties)) { - throw new Exception\InvalidArgumentException('$key is a protected property, use a different key'); + throw new Exception\InvalidArgumentException("$key is a protected property, use a different key"); } $this->$key = $value; @@ -133,7 +132,7 @@ public function __unset($key) } if (in_array($key, $this->protectedProperties)) { - throw new Exception\InvalidArgumentException('$key is a protected property, use a different key'); + throw new Exception\InvalidArgumentException("$key is a protected property, use a different key"); } unset($this->$key); @@ -154,7 +153,7 @@ public function &__get($key) } if (in_array($key, $this->protectedProperties, true)) { - throw new Exception\InvalidArgumentException('$key is a protected property, use a different key'); + throw new Exception\InvalidArgumentException("$key is a protected property, use a different key"); } return $this->$key; @@ -462,43 +461,37 @@ public function __unserialize($data) { $this->protectedProperties = array_keys(get_object_vars($this)); + $flag = $data['flag']; + unset($data['flag']); + $this->setFlags((int) $flag); + + $storage = $data['storage']; + unset($data['storage']); + if (! is_array($storage) && ! is_object($storage)) { + throw new UnexpectedValueException(sprintf( + 'Cannot deserialize %s instance: corrupt storage data;' + . ' expected array or object, received %s', + self::class, + gettype($storage) + )); + } + $this->exchangeArray($storage); + + $iteratorClass = $data['iteratorClass']; + unset($data['iteratorClass']); + if (! is_string($iteratorClass)) { + throw new UnexpectedValueException(sprintf( + 'Cannot deserialize %s instance: invalid iteratorClass; expected string, received %s', + self::class, + is_object($iteratorClass) ? get_class($iteratorClass) : gettype($iteratorClass) + )); + } + $this->setIteratorClass($iteratorClass); + + unset($data['protectedProperties']); + foreach ($data as $k => $v) { - switch ($k) { - case 'flag': - $this->setFlags((int) $v); - break; - - case 'storage': - if (! is_array($v) && ! is_object($v)) { - throw new UnexpectedValueException(sprintf( - 'Cannot deserialize %s instance: corrupt storage data;' - . ' expected array or object, received %s', - self::class, - gettype($v) - )); - } - - $this->exchangeArray($v); - break; - - case 'iteratorClass': - if (! is_string($v)) { - throw new UnexpectedValueException(sprintf( - 'Cannot deserialize %s instance: invalid iteratorClass; expected string, received %s', - self::class, - is_object($v) ? get_class($v) : gettype($v) - )); - } - - $this->setIteratorClass($v); - break; - - case 'protectedProperties': - break; - - default: - $this->__set($k, $v); - } + $this->__set($k, $v); } } } From 1751b3f88cfd866ae0a60494039997029b325964 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Mon, 29 Nov 2021 15:47:34 +0100 Subject: [PATCH 3/7] CI happy again? Signed-off-by: Filippo Tessarotto --- psalm-baseline.xml | 72 +++++++++++++++++++++++++++++++++++--- src/ArrayObject.php | 1 + test/CustomArrayObject.php | 2 +- 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index eb2ca18d..84923cd9 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -41,9 +41,11 @@ $this->storage[$key] $this->storage[$key] - + + $flag $ret $ret + $v Iterator @@ -58,6 +60,14 @@ is_callable($function) is_callable($function) + + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + @@ -158,6 +168,14 @@ $this->values $this->values + + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + @@ -205,6 +223,9 @@ $this[$name] + + ReturnTypeWillChange + @@ -223,6 +244,14 @@ (int) $priority (int) $priority + + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + @@ -292,6 +321,10 @@ null !== $this->queue + + ReturnTypeWillChange + ReturnTypeWillChange + @@ -305,7 +338,6 @@ $data[] $item $item - $item @@ -314,6 +346,12 @@ $item $item + + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + @@ -321,6 +359,12 @@ $item $item + + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + ReturnTypeWillChange + @@ -377,9 +421,7 @@ - - $length - $this->getEncoding() + $this->getEncoding() $this->getEncoding() @@ -424,6 +466,12 @@ $ar['foo']['bar'] $ar['foo']['bar'] + + $unserialized + + + isImmutable + assertSame @@ -500,6 +548,20 @@ $this->helper + + + $isImmutable + + + bool + + + $this->isImmutable + + + CustomArrayObject + + 'ErrorException' diff --git a/src/ArrayObject.php b/src/ArrayObject.php index 015ea1cc..3a8a4b84 100644 --- a/src/ArrayObject.php +++ b/src/ArrayObject.php @@ -16,6 +16,7 @@ use function asort; use function class_exists; use function count; +use function get_class; use function get_object_vars; use function gettype; use function in_array; diff --git a/test/CustomArrayObject.php b/test/CustomArrayObject.php index 5e85bc37..32bf20d8 100644 --- a/test/CustomArrayObject.php +++ b/test/CustomArrayObject.php @@ -8,7 +8,7 @@ final class CustomArrayObject extends ArrayObject { - protected bool $isImmutable = true; + protected $isImmutable = true; public function isImmutable(): bool { From 91493260b05728c7f2a6d79fe73e9208fa6bd63a Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Mon, 29 Nov 2021 15:49:51 +0100 Subject: [PATCH 4/7] Finally Signed-off-by: Filippo Tessarotto --- test/CustomArrayObject.php | 1 + 1 file changed, 1 insertion(+) diff --git a/test/CustomArrayObject.php b/test/CustomArrayObject.php index 32bf20d8..40586be4 100644 --- a/test/CustomArrayObject.php +++ b/test/CustomArrayObject.php @@ -8,6 +8,7 @@ final class CustomArrayObject extends ArrayObject { + /** @var bool */ protected $isImmutable = true; public function isImmutable(): bool From 17ff7fceee2d4d94ce770429546fc098cf72730e Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 1 Dec 2021 07:29:07 +0100 Subject: [PATCH 5/7] Update src/ArrayObject.php Co-authored-by: Matthew Weier O'Phinney Signed-off-by: Filippo Tessarotto --- src/ArrayObject.php | 48 ++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/src/ArrayObject.php b/src/ArrayObject.php index 3a8a4b84..2a077e6d 100644 --- a/src/ArrayObject.php +++ b/src/ArrayObject.php @@ -462,35 +462,39 @@ public function __unserialize($data) { $this->protectedProperties = array_keys(get_object_vars($this)); - $flag = $data['flag']; - unset($data['flag']); - $this->setFlags((int) $flag); + // Unserialize protected internal properties first + if (array_key_exists('flag', $data)) { + $this->setFlags((int) $data['flag']); + unset($data['flag']); + } - $storage = $data['storage']; - unset($data['storage']); - if (! is_array($storage) && ! is_object($storage)) { - throw new UnexpectedValueException(sprintf( - 'Cannot deserialize %s instance: corrupt storage data;' - . ' expected array or object, received %s', - self::class, - gettype($storage) - )); + if (array_key_exists('storage', $data)) { + if (! is_array($data['storage']) && ! is_object($data['storage'])) { + throw new UnexpectedValueException(sprintf( + 'Cannot deserialize %s instance: corrupt storage data; expected array or object, received %s', + self::class, + gettype($data['storage']) + )); + } + $this->exchangeArray($data['storage']); + unset($data['storage']); } - $this->exchangeArray($storage); - $iteratorClass = $data['iteratorClass']; - unset($data['iteratorClass']); - if (! is_string($iteratorClass)) { - throw new UnexpectedValueException(sprintf( - 'Cannot deserialize %s instance: invalid iteratorClass; expected string, received %s', - self::class, - is_object($iteratorClass) ? get_class($iteratorClass) : gettype($iteratorClass) - )); + if (array_key_exists('iteratorClass', $data) { + if (! is_string($data['iteratorClass'])) { + throw new UnexpectedValueException(sprintf( + 'Cannot deserialize %s instance: invalid iteratorClass; expected string, received %s', + self::class, + is_object($data['iteratorClass']) ? get_class($data['iteratorClass']) : gettype($data['iteratorClass']) + )); + } + $this->setIteratorClass($data['iteratorClass']); + unset($data['iteratorClass']); } - $this->setIteratorClass($iteratorClass); unset($data['protectedProperties']); + // Unserialize array keys after resolving protected properties to ensure configuration is used. foreach ($data as $k => $v) { $this->__set($k, $v); } From d844fcfaa38cd96ea1ade7a804fd5ef1c0ea5ba5 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 1 Dec 2021 07:35:26 +0100 Subject: [PATCH 6/7] Fix syntax errors Signed-off-by: Filippo Tessarotto --- src/ArrayObject.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ArrayObject.php b/src/ArrayObject.php index 2a077e6d..93d412fe 100644 --- a/src/ArrayObject.php +++ b/src/ArrayObject.php @@ -12,6 +12,7 @@ use Serializable; use UnexpectedValueException; +use function array_key_exists; use function array_keys; use function asort; use function class_exists; @@ -480,7 +481,7 @@ public function __unserialize($data) unset($data['storage']); } - if (array_key_exists('iteratorClass', $data) { + if (array_key_exists('iteratorClass', $data)) { if (! is_string($data['iteratorClass'])) { throw new UnexpectedValueException(sprintf( 'Cannot deserialize %s instance: invalid iteratorClass; expected string, received %s', From 724894b35abdb17df2eec01b39e568e954daf413 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 1 Dec 2021 07:38:37 +0100 Subject: [PATCH 7/7] PHPCS Signed-off-by: Filippo Tessarotto --- src/ArrayObject.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ArrayObject.php b/src/ArrayObject.php index 93d412fe..6cc195dd 100644 --- a/src/ArrayObject.php +++ b/src/ArrayObject.php @@ -486,7 +486,9 @@ public function __unserialize($data) throw new UnexpectedValueException(sprintf( 'Cannot deserialize %s instance: invalid iteratorClass; expected string, received %s', self::class, - is_object($data['iteratorClass']) ? get_class($data['iteratorClass']) : gettype($data['iteratorClass']) + is_object($data['iteratorClass']) + ? get_class($data['iteratorClass']) + : gettype($data['iteratorClass']) )); } $this->setIteratorClass($data['iteratorClass']);