MaxSem has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/394736 )
Change subject: WIP: prevent (de)serialization ...................................................................... WIP: prevent (de)serialization Change-Id: I91fc3b0e2d82e11d3f9b43b60eeed515152038d9 --- M src/ScopedCallback.php M tests/ScopedCallbackTest.php 2 files changed, 35 insertions(+), 7 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/libs/ScopedCallback refs/changes/36/394736/1 diff --git a/src/ScopedCallback.php b/src/ScopedCallback.php index ab9e1be..ce79472 100644 --- a/src/ScopedCallback.php +++ b/src/ScopedCallback.php @@ -28,10 +28,8 @@ * Class for asserting that a callback happens when a dummy object leaves scope */ class ScopedCallback { - /** @var callable */ + /** @var \Closure */ protected $callback; - /** @var array */ - protected $params; /** * @param callable|null $callback @@ -42,8 +40,19 @@ if ( $callback !== null && !is_callable( $callback ) ) { throw new \InvalidArgumentException( "Provided callback is not valid." ); } - $this->callback = $callback; - $this->params = $params; + $this->callback = self::makeCallback( $callback, $params ); + } + + /** + * Helper function for closure creation. Without it, the closure will contain a reference to + * $this and the destructor will never get called. + */ + private static function makeCallback( $callback, array $params ) { + return $callback ? function () use ( $callback, $params ) { + if ( $callback !== null ) { + call_user_func_array( $callback, $params ); + } + } : null; } /** @@ -72,8 +81,15 @@ * Trigger the callback when it leaves scope. */ function __destruct() { - if ( $this->callback !== null ) { - call_user_func_array( $this->callback, $this->params ); + if ( $this->callback === null ) { + return; } + if ( !$this->callback instanceof \Closure ) { + throw new \UnexpectedValueException( 'A callback in ' . __CLASS__ + . ' was expected to be a closure. Bogus deserialization?' ); + } + + $callback = $this->callback; + $callback(); } } diff --git a/tests/ScopedCallbackTest.php b/tests/ScopedCallbackTest.php index f4c1f72..68d459d 100644 --- a/tests/ScopedCallbackTest.php +++ b/tests/ScopedCallbackTest.php @@ -66,4 +66,16 @@ new ScopedCallback( 'not a valid callback' ); } + /** + * @expectedException Exception + */ + public function testSerializationProhibition() { + $sc = new ScopedCallback( 'print', [ '' ] ); + serialize( $sc ); + } + + public function testDeserializationProhibition() { + $serialized = sprintf( 'O:%d:"%s":0:{s:11:"callback";O:7:"Closure"}', strlen( ScopedCallback::class ), ScopedCallback::class ); + var_dump( unserialize( $serialized ) ); + } } -- To view, visit https://gerrit.wikimedia.org/r/394736 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I91fc3b0e2d82e11d3f9b43b60eeed515152038d9 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/libs/ScopedCallback Gerrit-Branch: master Gerrit-Owner: MaxSem <maxsem.w...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits