From 0494eb3a54321808985ab7f275777633e8178f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 1 Aug 2022 22:57:54 +0200 Subject: [PATCH 1/2] Verify that the engine doesn't change in construct_twice.phpt --- ext/random/tests/03_randomizer/construct_twice.phpt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ext/random/tests/03_randomizer/construct_twice.phpt b/ext/random/tests/03_randomizer/construct_twice.phpt index ccb831dfd4ab5..848e8029720e0 100644 --- a/ext/random/tests/03_randomizer/construct_twice.phpt +++ b/ext/random/tests/03_randomizer/construct_twice.phpt @@ -31,10 +31,21 @@ try { echo $e->getMessage() . PHP_EOL; } +try { + $r = new \Random\Randomizer(new \Random\Engine\Xoshiro256StarStar()); + $r->__construct(new \UserEngine()); +} catch (\BadMethodCallException $e) { + echo $e->getMessage(), PHP_EOL; +} + +var_dump($r->engine::class); + die('success'); ?> --EXPECT-- Cannot call constructor twice Cannot call constructor twice Cannot call constructor twice +Cannot call constructor twice +string(32) "Random\Engine\Xoshiro256StarStar" success From c114995f504bbab0eed965129e6e5cf93ea1da36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 1 Aug 2022 22:58:42 +0200 Subject: [PATCH 2/2] Clean up the implementation of Randomizer::__construct() Instead of manually checking whether the constructor was already called, we rely on the `readonly` modifier of the `$engine` property. Additionally use `object_init_ex()` instead of manually calling `->create_object()`. --- ext/random/randomizer.c | 30 +++++++++---------- .../tests/03_randomizer/construct_twice.phpt | 16 +++++----- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 2672477feccb8..c0d981aa24e39 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -62,32 +62,30 @@ static inline void randomizer_common_init(php_random_randomizer *randomizer, zen PHP_METHOD(Random_Randomizer, __construct) { php_random_randomizer *randomizer = Z_RANDOM_RANDOMIZER_P(ZEND_THIS); - zend_object *engine_object = NULL; - zval zengine_object; + zval engine; + zval *param_engine = NULL; ZEND_PARSE_PARAMETERS_START(0, 1) Z_PARAM_OPTIONAL - Z_PARAM_OBJ_OF_CLASS_OR_NULL(engine_object, random_ce_Random_Engine); + Z_PARAM_OBJECT_OF_CLASS_OR_NULL(param_engine, random_ce_Random_Engine); ZEND_PARSE_PARAMETERS_END(); - if (randomizer->algo) { - zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Cannot call constructor twice"); - RETURN_THROWS(); + if (param_engine != NULL) { + ZVAL_COPY(&engine, param_engine); + } else { + /* Create default RNG instance */ + object_init_ex(&engine, random_ce_Random_Engine_Secure); } - /* Create default RNG instance */ - if (!engine_object) { - engine_object = random_ce_Random_Engine_Secure->create_object(random_ce_Random_Engine_Secure); - - /* No need self-refcount */ - GC_DELREF(engine_object); - } + zend_update_property(random_ce_Random_Randomizer, Z_OBJ_P(ZEND_THIS), "engine", strlen("engine"), &engine); - ZVAL_OBJ(&zengine_object, engine_object); + OBJ_RELEASE(Z_OBJ_P(&engine)); - zend_update_property(random_ce_Random_Randomizer, Z_OBJ_P(ZEND_THIS), "engine", strlen("engine"), &zengine_object); + if (EG(exception)) { + RETURN_THROWS(); + } - randomizer_common_init(randomizer, engine_object); + randomizer_common_init(randomizer, Z_OBJ_P(&engine)); } /* }}} */ diff --git a/ext/random/tests/03_randomizer/construct_twice.phpt b/ext/random/tests/03_randomizer/construct_twice.phpt index 848e8029720e0..ed2a667e2af11 100644 --- a/ext/random/tests/03_randomizer/construct_twice.phpt +++ b/ext/random/tests/03_randomizer/construct_twice.phpt @@ -13,28 +13,28 @@ final class UserEngine implements \Random\Engine try { (new \Random\Randomizer())->__construct(); -} catch (\BadMethodCallException $e) { +} catch (\Error $e) { echo $e->getMessage() . PHP_EOL; } try { $r = new \Random\Randomizer(new \Random\Engine\Xoshiro256StarStar()); $r->__construct(new \Random\Engine\PcgOneseq128XslRr64()); -} catch (\BadMethodCallException $e) { +} catch (\Error $e) { echo $e->getMessage() . PHP_EOL; } try { $r = new \Random\Randomizer(new \UserEngine()); $r->__construct(new \UserEngine()); -} catch (\BadMethodCallException $e) { +} catch (\Error $e) { echo $e->getMessage() . PHP_EOL; } try { $r = new \Random\Randomizer(new \Random\Engine\Xoshiro256StarStar()); $r->__construct(new \UserEngine()); -} catch (\BadMethodCallException $e) { +} catch (\Error $e) { echo $e->getMessage(), PHP_EOL; } @@ -43,9 +43,9 @@ var_dump($r->engine::class); die('success'); ?> --EXPECT-- -Cannot call constructor twice -Cannot call constructor twice -Cannot call constructor twice -Cannot call constructor twice +Cannot modify readonly property Random\Randomizer::$engine +Cannot modify readonly property Random\Randomizer::$engine +Cannot modify readonly property Random\Randomizer::$engine +Cannot modify readonly property Random\Randomizer::$engine string(32) "Random\Engine\Xoshiro256StarStar" success