From 8ec2ba79de0c0f0fa43c8210b286e86ca33eb4ca Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <odigiman@gmail.com>
Date: Thu, 21 May 2020 11:38:02 +0100
Subject: [PATCH] Unhackify Registry member cloning, fixes #3519

---
 src/block/VanillaBlocks.php                   |  8 +--
 src/item/VanillaItems.php                     |  8 +--
 src/utils/CloningRegistryTrait.php            | 32 +++++++++++
 src/utils/RegistryTrait.php                   | 11 +++-
 .../utils/CloningRegistryTraitTest.php        | 55 +++++++++++++++++++
 tests/phpunit/utils/TestCloningRegistry.php   | 54 ++++++++++++++++++
 6 files changed, 158 insertions(+), 10 deletions(-)
 create mode 100644 src/utils/CloningRegistryTrait.php
 create mode 100644 tests/phpunit/utils/CloningRegistryTraitTest.php
 create mode 100644 tests/phpunit/utils/TestCloningRegistry.php

diff --git a/src/block/VanillaBlocks.php b/src/block/VanillaBlocks.php
index 5da766624..3371a7d25 100644
--- a/src/block/VanillaBlocks.php
+++ b/src/block/VanillaBlocks.php
@@ -23,8 +23,8 @@ declare(strict_types=1);
 
 namespace pocketmine\block;
 
+use pocketmine\utils\CloningRegistryTrait;
 use pocketmine\utils\RegistryTrait;
-use pocketmine\utils\Utils;
 use function assert;
 
 /**
@@ -665,7 +665,7 @@ use function assert;
  * @method static Wool YELLOW_WOOL()
  */
 final class VanillaBlocks{
-	use RegistryTrait;
+	use CloningRegistryTrait;
 
 	private function __construct(){
 		//NOOP
@@ -678,14 +678,14 @@ final class VanillaBlocks{
 	public static function fromString(string $name) : Block{
 		$result = self::_registryFromString($name);
 		assert($result instanceof Block);
-		return clone $result;
+		return $result;
 	}
 
 	/**
 	 * @return Block[]
 	 */
 	public static function getAll() : array{
-		return Utils::cloneObjectArray(self::_registryGetAll());
+		return self::_registryGetAll();
 	}
 
 	protected static function setup() : void{
diff --git a/src/item/VanillaItems.php b/src/item/VanillaItems.php
index d8bb6b655..a95604b8e 100644
--- a/src/item/VanillaItems.php
+++ b/src/item/VanillaItems.php
@@ -23,8 +23,8 @@ declare(strict_types=1);
 
 namespace pocketmine\item;
 
+use pocketmine\utils\CloningRegistryTrait;
 use pocketmine\utils\RegistryTrait;
-use pocketmine\utils\Utils;
 use function assert;
 
 /**
@@ -292,7 +292,7 @@ use function assert;
  * @method static Skull ZOMBIE_HEAD()
  */
 final class VanillaItems{
-	use RegistryTrait;
+	use CloningRegistryTrait;
 
 	private function __construct(){
 		//NOOP
@@ -305,14 +305,14 @@ final class VanillaItems{
 	public static function fromString(string $name) : Item{
 		$result = self::_registryFromString($name);
 		assert($result instanceof Item);
-		return clone $result;
+		return $result;
 	}
 
 	/**
 	 * @return Item[]
 	 */
 	public static function getAll() : array{
-		return Utils::cloneObjectArray(self::_registryGetAll());
+		return self::_registryGetAll();
 	}
 
 	protected static function setup() : void{
diff --git a/src/utils/CloningRegistryTrait.php b/src/utils/CloningRegistryTrait.php
new file mode 100644
index 000000000..af815658e
--- /dev/null
+++ b/src/utils/CloningRegistryTrait.php
@@ -0,0 +1,32 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\utils;
+
+trait CloningRegistryTrait{
+	use RegistryTrait;
+
+	protected static function preprocessMember(object $member) : object{
+		return clone $member;
+	}
+}
diff --git a/src/utils/RegistryTrait.php b/src/utils/RegistryTrait.php
index 547b08c57..682a11a71 100644
--- a/src/utils/RegistryTrait.php
+++ b/src/utils/RegistryTrait.php
@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\utils;
 
+use function array_map;
 use function count;
 use function get_class;
 use function implode;
@@ -77,7 +78,11 @@ trait RegistryTrait{
 		if(!isset(self::$members[$name])){
 			throw new \InvalidArgumentException("No such registry member: " . self::class . "::" . $name);
 		}
-		return self::$members[$name];
+		return self::preprocessMember(self::$members[$name]);
+	}
+
+	protected static function preprocessMember(object $member) : object{
+		return $member;
 	}
 
 	/**
@@ -103,7 +108,9 @@ trait RegistryTrait{
 	 */
 	private static function _registryGetAll() : array{
 		self::checkInit();
-		return self::$members;
+		return array_map(function(object $o) : object{
+			return self::preprocessMember($o);
+		}, self::$members);
 	}
 
 	/**
diff --git a/tests/phpunit/utils/CloningRegistryTraitTest.php b/tests/phpunit/utils/CloningRegistryTraitTest.php
new file mode 100644
index 000000000..8151481a3
--- /dev/null
+++ b/tests/phpunit/utils/CloningRegistryTraitTest.php
@@ -0,0 +1,55 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\utils;
+
+use PHPUnit\Framework\TestCase;
+use pocketmine\block\VanillaBlocks;
+
+final class CloningRegistryTraitTest extends TestCase{
+
+	/**
+	 * @phpstan-return \Generator<int, array{\Closure() : \stdClass}, void, void>
+	 */
+	public function cloningRegistryMembersProvider() : \Generator{
+		yield [function() : \stdClass{ return TestCloningRegistry::TEST1(); }];
+		yield [function() : \stdClass{ return TestCloningRegistry::TEST2(); }];
+		yield [function() : \stdClass{ return TestCloningRegistry::TEST3(); }];
+	}
+
+	/**
+	 * @dataProvider cloningRegistryMembersProvider
+	 * @phpstan-param \Closure() : \stdClass $provider
+	 */
+	public function testEachMemberClone(\Closure $provider) : void{
+		self::assertNotSame($provider(), $provider(), "Cloning registry should never return the same object twice");
+	}
+
+	public function testGetAllClone() : void{
+		$list1 = TestCloningRegistry::getAll();
+		$list2 = TestCloningRegistry::getAll();
+		foreach($list1 as $k => $member){
+			self::assertNotSame($member, $list2[$k], "VanillaBlocks ought to clone its members");
+		}
+	}
+}
diff --git a/tests/phpunit/utils/TestCloningRegistry.php b/tests/phpunit/utils/TestCloningRegistry.php
new file mode 100644
index 000000000..5683fd11b
--- /dev/null
+++ b/tests/phpunit/utils/TestCloningRegistry.php
@@ -0,0 +1,54 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\utils;
+
+/**
+ * This doc-block is generated automatically, do not modify it manually.
+ * This must be regenerated whenever registry members are added, removed or changed.
+ * @see RegistryTrait::_generateMethodAnnotations()
+ *
+ * @method static \stdClass TEST1()
+ * @method static \stdClass TEST2()
+ * @method static \stdClass TEST3()
+ */
+final class TestCloningRegistry{
+	use CloningRegistryTrait;
+
+	/**
+	 * @return \stdClass[]
+	 */
+	public static function getAll() : array{
+		return self::_registryGetAll();
+	}
+
+	public static function fromString(string $s) : \stdClass{
+		return self::_registryFromString($s);
+	}
+
+	protected static function setup() : void{
+		self::_registryRegister("test1", new \stdClass());
+		self::_registryRegister("test2", new \stdClass());
+		self::_registryRegister("test3", new \stdClass());
+	}
+}