From 05d9298958606144a07c39062d3e27d79a3c5623 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 19 Mar 2023 16:33:59 +0000 Subject: [PATCH 01/19] PHPStan 1.10.7 --- composer.json | 2 +- composer.lock | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index 23bf5f1267..00673dd2cc 100644 --- a/composer.json +++ b/composer.json @@ -56,7 +56,7 @@ "webmozart/path-util": "^2.3" }, "require-dev": { - "phpstan/phpstan": "1.10.6", + "phpstan/phpstan": "1.10.7", "phpstan/phpstan-phpunit": "^1.1.0", "phpstan/phpstan-strict-rules": "^1.2.0", "phpunit/phpunit": "^9.2" diff --git a/composer.lock b/composer.lock index 4f149eee92..bf8ede4887 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1d0c1d2fe668d85ae87110a1e3cfac05", + "content-hash": "6622f1adc1d8ce4c025c367f8825f936", "packages": [ { "name": "adhocore/json-comment", @@ -1884,16 +1884,16 @@ }, { "name": "phpstan/phpstan", - "version": "1.10.6", + "version": "1.10.7", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan.git", - "reference": "50d089a3e0904b0fe7e2cf2d4fd37d427d64235a" + "reference": "b10ceb526d9607903c5b2673f1fc8775dbe48975" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan/zipball/50d089a3e0904b0fe7e2cf2d4fd37d427d64235a", - "reference": "50d089a3e0904b0fe7e2cf2d4fd37d427d64235a", + "url": "https://api.github.com/repos/phpstan/phpstan/zipball/b10ceb526d9607903c5b2673f1fc8775dbe48975", + "reference": "b10ceb526d9607903c5b2673f1fc8775dbe48975", "shasum": "" }, "require": { @@ -1922,8 +1922,11 @@ "static analysis" ], "support": { + "docs": "https://phpstan.org/user-guide/getting-started", + "forum": "https://github.com/phpstan/phpstan/discussions", "issues": "https://github.com/phpstan/phpstan/issues", - "source": "https://github.com/phpstan/phpstan/tree/1.10.6" + "security": "https://github.com/phpstan/phpstan/security/policy", + "source": "https://github.com/phpstan/phpstan-src" }, "funding": [ { @@ -1939,7 +1942,7 @@ "type": "tidelift" } ], - "time": "2023-03-09T16:55:12+00:00" + "time": "2023-03-16T15:24:20+00:00" }, { "name": "phpstan/phpstan-phpunit", From a619fd2be688ca63d20e82bc72a9d146404d83cc Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 19 Mar 2023 16:37:38 +0000 Subject: [PATCH 02/19] Scrub PHPStan baselines --- tests/phpstan/configs/actual-problems.neon | 5 ----- tests/phpstan/configs/phpstan-bugs.neon | 20 -------------------- 2 files changed, 25 deletions(-) diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 90ec78b196..ac9093da1e 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -490,11 +490,6 @@ parameters: count: 1 path: ../../../src/command/defaults/TimingsCommand.php - - - message: "#^Call to function is_resource\\(\\) with resource will always evaluate to true\\.$#" - count: 2 - path: ../../../src/console/ConsoleReader.php - - message: "#^Parameter \\#1 \\$path of static method pocketmine\\\\utils\\\\Filesystem\\:\\:cleanPath\\(\\) expects string, mixed given\\.$#" count: 1 diff --git a/tests/phpstan/configs/phpstan-bugs.neon b/tests/phpstan/configs/phpstan-bugs.neon index 3ee56e8be5..99cf2aaa76 100644 --- a/tests/phpstan/configs/phpstan-bugs.neon +++ b/tests/phpstan/configs/phpstan-bugs.neon @@ -5,11 +5,6 @@ parameters: count: 1 path: ../../../src/block/BaseBanner.php - - - message: "#^Comparison operation \"\\<\" between int\\<1, max\\> and 1 is always false\\.$#" - count: 1 - path: ../../../src/console/ConsoleCommandSender.php - - message: "#^Call to function assert\\(\\) with false and 'unknown hit type' will always evaluate to false\\.$#" count: 1 @@ -35,21 +30,11 @@ parameters: count: 1 path: ../../../src/network/mcpe/raklib/SnoozeAwarePthreadsChannelWriter.php - - - message: "#^Comparison operation \"\\<\" between int\\<1, max\\> and 1 is always false\\.$#" - count: 1 - path: ../../../src/player/Player.php - - message: "#^Dead catch \\- RuntimeException is never thrown in the try block\\.$#" count: 1 path: ../../../src/plugin/PluginManager.php - - - message: "#^Offset \\(int\\|string\\) on array\\ in isset\\(\\) always exists and is not nullable\\.$#" - count: 1 - path: ../../../src/plugin/PluginManager.php - - message: "#^Static property pocketmine\\\\scheduler\\\\AsyncTask\\:\\:\\$threadLocalStorage \\(ArrayObject\\\\>\\|null\\) does not accept non\\-empty\\-array\\\\>\\|ArrayObject\\\\>\\.$#" count: 1 @@ -65,11 +50,6 @@ parameters: count: 1 path: ../../../src/thread/Worker.php - - - message: "#^Call to function is_resource\\(\\) with resource will always evaluate to true\\.$#" - count: 2 - path: ../../../src/world/format/io/region/RegionLoader.php - - message: "#^Casting to int something that's already int\\.$#" count: 1 From 01d557062a5f51ac8b153f4c6700284f052c8a0b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 19 Mar 2023 16:41:01 +0000 Subject: [PATCH 03/19] Remove dead baseline --- phpstan.neon.dist | 1 - tests/phpstan/configs/runtime-type-checks.neon | 12 ------------ 2 files changed, 13 deletions(-) delete mode 100644 tests/phpstan/configs/runtime-type-checks.neon diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 6fc15b07a9..d184a0fd04 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -5,7 +5,6 @@ includes: - tests/phpstan/configs/impossible-generics.neon - tests/phpstan/configs/php-bugs.neon - tests/phpstan/configs/phpstan-bugs.neon - - tests/phpstan/configs/runtime-type-checks.neon - tests/phpstan/configs/spl-fixed-array-sucks.neon - vendor/phpstan/phpstan-phpunit/extension.neon - vendor/phpstan/phpstan-phpunit/rules.neon diff --git a/tests/phpstan/configs/runtime-type-checks.neon b/tests/phpstan/configs/runtime-type-checks.neon deleted file mode 100644 index fb1e456b78..0000000000 --- a/tests/phpstan/configs/runtime-type-checks.neon +++ /dev/null @@ -1,12 +0,0 @@ -parameters: - ignoreErrors: - - - message: "#^Casting to int something that's already int\\.$#" - count: 2 - path: ../../../src/item/Item.php - - - - message: "#^Instanceof between pocketmine\\\\nbt\\\\tag\\\\CompoundTag and pocketmine\\\\nbt\\\\tag\\\\CompoundTag will always evaluate to true\\.$#" - count: 1 - path: ../../../src/network/mcpe/handler/InGamePacketHandler.php - From 52ea4feac00a116c53fc5f98602d9704ff8b8164 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 19 Mar 2023 16:53:02 +0000 Subject: [PATCH 04/19] Updated pocketmine/locale-data --- composer.json | 2 +- composer.lock | 14 +++++++------- src/lang/KnownTranslationFactory.php | 12 ++++++++++++ src/lang/KnownTranslationKeys.php | 3 +++ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index 00673dd2cc..4f7afd1485 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "pocketmine/classloader": "^0.2.0", "pocketmine/color": "^0.3.0", "pocketmine/errorhandler": "^0.6.0", - "pocketmine/locale-data": "~2.18.0", + "pocketmine/locale-data": "~2.19.0", "pocketmine/log": "^0.4.0", "pocketmine/log-pthreads": "^0.4.0", "pocketmine/math": "^0.4.0", diff --git a/composer.lock b/composer.lock index bf8ede4887..3305ea5b1a 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "6622f1adc1d8ce4c025c367f8825f936", + "content-hash": "7c02da0a4bfc5f59effdf8e55b085c08", "packages": [ { "name": "adhocore/json-comment", @@ -589,16 +589,16 @@ }, { "name": "pocketmine/locale-data", - "version": "2.18.3", + "version": "2.19.5", "source": { "type": "git", "url": "https://github.com/pmmp/Language.git", - "reference": "da25bfe9ee4822a84feb9b7e620c56ad4000aed0" + "reference": "71af5f9bd23b4e4bad8920dac7f4fe08e5205f7d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/pmmp/Language/zipball/da25bfe9ee4822a84feb9b7e620c56ad4000aed0", - "reference": "da25bfe9ee4822a84feb9b7e620c56ad4000aed0", + "url": "https://api.github.com/repos/pmmp/Language/zipball/71af5f9bd23b4e4bad8920dac7f4fe08e5205f7d", + "reference": "71af5f9bd23b4e4bad8920dac7f4fe08e5205f7d", "shasum": "" }, "type": "library", @@ -606,9 +606,9 @@ "description": "Language resources used by PocketMine-MP", "support": { "issues": "https://github.com/pmmp/Language/issues", - "source": "https://github.com/pmmp/Language/tree/2.18.3" + "source": "https://github.com/pmmp/Language/tree/2.19.5" }, - "time": "2023-01-17T21:43:36+00:00" + "time": "2023-03-19T16:45:15+00:00" }, { "name": "pocketmine/log", diff --git a/src/lang/KnownTranslationFactory.php b/src/lang/KnownTranslationFactory.php index 07a8b17b46..ea8c2952e5 100644 --- a/src/lang/KnownTranslationFactory.php +++ b/src/lang/KnownTranslationFactory.php @@ -1606,6 +1606,14 @@ final class KnownTranslationFactory{ return new Translatable(KnownTranslationKeys::POCKETMINE_DISCONNECT_BAN_NOREASON, []); } + public static function pocketmine_disconnect_clientDisconnect() : Translatable{ + return new Translatable(KnownTranslationKeys::POCKETMINE_DISCONNECT_CLIENTDISCONNECT, []); + } + + public static function pocketmine_disconnect_clientReconnect() : Translatable{ + return new Translatable(KnownTranslationKeys::POCKETMINE_DISCONNECT_CLIENTRECONNECT, []); + } + public static function pocketmine_disconnect_error(Translatable|string $error, Translatable|string $errorId) : Translatable{ return new Translatable(KnownTranslationKeys::POCKETMINE_DISCONNECT_ERROR, [ "error" => $error, @@ -1633,6 +1641,10 @@ final class KnownTranslationFactory{ return new Translatable(KnownTranslationKeys::POCKETMINE_DISCONNECT_ERROR_RESPAWN, []); } + public static function pocketmine_disconnect_error_timeout() : Translatable{ + return new Translatable(KnownTranslationKeys::POCKETMINE_DISCONNECT_ERROR_TIMEOUT, []); + } + public static function pocketmine_disconnect_incompatibleProtocol(Translatable|string $param0) : Translatable{ return new Translatable(KnownTranslationKeys::POCKETMINE_DISCONNECT_INCOMPATIBLEPROTOCOL, [ 0 => $param0, diff --git a/src/lang/KnownTranslationKeys.php b/src/lang/KnownTranslationKeys.php index 3221ecbdc4..c834527306 100644 --- a/src/lang/KnownTranslationKeys.php +++ b/src/lang/KnownTranslationKeys.php @@ -349,12 +349,15 @@ final class KnownTranslationKeys{ public const POCKETMINE_DISCONNECT_BAN_HARDCORE = "pocketmine.disconnect.ban.hardcore"; public const POCKETMINE_DISCONNECT_BAN_IP = "pocketmine.disconnect.ban.ip"; public const POCKETMINE_DISCONNECT_BAN_NOREASON = "pocketmine.disconnect.ban.noReason"; + public const POCKETMINE_DISCONNECT_CLIENTDISCONNECT = "pocketmine.disconnect.clientDisconnect"; + public const POCKETMINE_DISCONNECT_CLIENTRECONNECT = "pocketmine.disconnect.clientReconnect"; public const POCKETMINE_DISCONNECT_ERROR = "pocketmine.disconnect.error"; public const POCKETMINE_DISCONNECT_ERROR_AUTHENTICATION = "pocketmine.disconnect.error.authentication"; public const POCKETMINE_DISCONNECT_ERROR_BADPACKET = "pocketmine.disconnect.error.badPacket"; public const POCKETMINE_DISCONNECT_ERROR_INTERNAL = "pocketmine.disconnect.error.internal"; public const POCKETMINE_DISCONNECT_ERROR_LOGINTIMEOUT = "pocketmine.disconnect.error.loginTimeout"; public const POCKETMINE_DISCONNECT_ERROR_RESPAWN = "pocketmine.disconnect.error.respawn"; + public const POCKETMINE_DISCONNECT_ERROR_TIMEOUT = "pocketmine.disconnect.error.timeout"; public const POCKETMINE_DISCONNECT_INCOMPATIBLEPROTOCOL = "pocketmine.disconnect.incompatibleProtocol"; public const POCKETMINE_DISCONNECT_INVALIDSESSION = "pocketmine.disconnect.invalidSession"; public const POCKETMINE_DISCONNECT_INVALIDSESSION_BADSIGNATURE = "pocketmine.disconnect.invalidSession.badSignature"; From 097632902ab1a02ec366ff2ecde65b63ba162923 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:02:32 +0000 Subject: [PATCH 05/19] InGamePacketHandler: fixed crash condition in drop item handler --- src/network/mcpe/handler/InGamePacketHandler.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 67d7ebb65a..c2fa13d466 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -387,15 +387,18 @@ class InGamePacketHandler extends PacketHandler{ } foreach($data->getActions() as $networkInventoryAction){ - if($networkInventoryAction->sourceType === NetworkInventoryAction::SOURCE_WORLD){ + if($networkInventoryAction->sourceType === NetworkInventoryAction::SOURCE_WORLD && $networkInventoryAction->inventorySlot == NetworkInventoryAction::ACTION_MAGIC_SLOT_DROP_ITEM){ //drop item - we don't need to validate this, we only care about the count //if the resulting actions don't match the client for some reason, it will trigger an automatic //prediction rollback anyway. //it's technically possible to see this more than once, but a normal client should never do that. + $droppedItemStack = $networkInventoryAction->newItem->getItemStack(); + if($droppedItemStack->getCount() <= 0){ + throw new PacketHandlingException("Expected positive count for dropped item"); + } $inventory = $this->player->getInventory(); $heldItemStack = TypeConverter::getInstance()->coreItemStackToNet($inventory->getItemInHand()); - $droppedItemStack = $networkInventoryAction->newItem->getItemStack(); //because the client doesn't tell us the expected itemstack ID, we have to deep-compare our known //itemstack info with the one the client sent. This is costly, but we don't have any other option :( if($heldItemStack->getCount() < $droppedItemStack->getCount() || !$heldItemStack->equalsWithoutCount($droppedItemStack)){ From ccd288d7fa38ab24f0b17bab0af64ba2581f01a2 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:04:29 +0000 Subject: [PATCH 06/19] Avoid repeated calls to getItemInHand() in drop item handler --- src/network/mcpe/handler/InGamePacketHandler.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index c2fa13d466..6dbeaa8f4e 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -398,18 +398,19 @@ class InGamePacketHandler extends PacketHandler{ } $inventory = $this->player->getInventory(); - $heldItemStack = TypeConverter::getInstance()->coreItemStackToNet($inventory->getItemInHand()); + $heldItem = $inventory->getItemInHand(); + $heldItemStack = TypeConverter::getInstance()->coreItemStackToNet($heldItem); //because the client doesn't tell us the expected itemstack ID, we have to deep-compare our known //itemstack info with the one the client sent. This is costly, but we don't have any other option :( if($heldItemStack->getCount() < $droppedItemStack->getCount() || !$heldItemStack->equalsWithoutCount($droppedItemStack)){ return false; } - $newHeldItem = $inventory->getItemInHand(); - $droppedItem = $newHeldItem->pop($droppedItemStack->getCount()); + //this modifies $heldItem + $droppedItem = $heldItem->pop($droppedItemStack->getCount()); $builder = new TransactionBuilder(); - $builder->getInventory($inventory)->setItem($inventory->getHeldItemIndex(), $newHeldItem); + $builder->getInventory($inventory)->setItem($inventory->getHeldItemIndex(), $heldItem); $builder->addAction(new DropItemAction($droppedItem)); $transaction = new InventoryTransaction($this->player, $builder->generateActions()); From 955f7944bb5dc3278c42c2467ffc29d2f1afd373 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:06:33 +0000 Subject: [PATCH 07/19] ItemStackRequestExecutor: fixed another possible crash condition --- src/network/mcpe/handler/ItemStackRequestExecutor.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index 15f1776f04..11bfeb8414 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -141,6 +141,9 @@ final class ItemStackRequestExecutor{ private function removeItemFromSlot(ItemStackRequestSlotInfo $slotInfo, int $count) : Item{ $this->requestSlotInfos[] = $slotInfo; [$inventory, $slot] = $this->getBuilderInventoryAndSlot($slotInfo); + if($count === 0){ + throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Cannot take 0 items from a stack"); + } $existingItem = $inventory->getItem($slot); if($existingItem->getCount() < $count){ From f90315c4a234b1de6c707ad18b7c632e004a96fa Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:13:21 +0000 Subject: [PATCH 08/19] ItemStackRequestExecutor: harden against invalid item counts these cases should all be impossible, but that's assuming that the core code doesn't start using them for a different purpose in the future. --- .../mcpe/handler/ItemStackRequestExecutor.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index 11bfeb8414..df66f49389 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -141,8 +141,9 @@ final class ItemStackRequestExecutor{ private function removeItemFromSlot(ItemStackRequestSlotInfo $slotInfo, int $count) : Item{ $this->requestSlotInfos[] = $slotInfo; [$inventory, $slot] = $this->getBuilderInventoryAndSlot($slotInfo); - if($count === 0){ - throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Cannot take 0 items from a stack"); + if($count < 1){ + //this should be impossible at the protocol level, but in case of buggy core code this will prevent exploits + throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Cannot take less than 1 items from a stack"); } $existingItem = $inventory->getItem($slot); @@ -162,6 +163,10 @@ final class ItemStackRequestExecutor{ private function addItemToSlot(ItemStackRequestSlotInfo $slotInfo, Item $item, int $count) : void{ $this->requestSlotInfos[] = $slotInfo; [$inventory, $slot] = $this->getBuilderInventoryAndSlot($slotInfo); + if($count < 1){ + //this should be impossible at the protocol level, but in case of buggy core code this will prevent exploits + throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Cannot take less than 1 items from a stack"); + } $existingItem = $inventory->getItem($slot); if(!$existingItem->isNull() && !$existingItem->canStackWith($item)){ @@ -232,6 +237,10 @@ final class ItemStackRequestExecutor{ } private function takeCreatedItem(ItemStackRequestSlotInfo $destination, int $count) : void{ + if($count < 1){ + //this should be impossible at the protocol level, but in case of buggy core code this will prevent exploits + throw new ItemStackRequestProcessException("Cannot take less than 1 created item"); + } $createdItem = $this->nextCreatedItem; if($createdItem === null){ throw new ItemStackRequestProcessException("No created item is waiting to be taken"); From e57fbff28c1175d4536e487a1c1f877436b3f0c2 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:16:03 +0000 Subject: [PATCH 09/19] ItemStackRequestExecutor: added a sanity check for recipe repetitions --- src/network/mcpe/handler/ItemStackRequestExecutor.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index df66f49389..3aa751f27d 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -211,6 +211,12 @@ final class ItemStackRequestExecutor{ if($repetitions < 1){ //TODO: upper bound? throw new ItemStackRequestProcessException("Cannot craft a recipe less than 1 time"); } + if($repetitions > 256){ + //TODO: we can probably lower this limit to 64, but I'm unsure if there are cases where the client may + //request more than 64 repetitions of a recipe. + //It's already hard-limited to 256 repetitions in the protocol, so this is just a sanity check. + throw new ItemStackRequestProcessException("Cannot craft a recipe more than 256 times"); + } $craftingManager = $this->player->getServer()->getCraftingManager(); $recipe = $craftingManager->getCraftingRecipeFromIndex($recipeId); if($recipe === null){ From 08e8ef275f01507628a6fdd6670a4de392706096 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:17:24 +0000 Subject: [PATCH 10/19] remove comment --- src/network/mcpe/handler/ItemStackRequestExecutor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index 3aa751f27d..6ac67b5dc0 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -208,7 +208,7 @@ final class ItemStackRequestExecutor{ if($this->specialTransaction !== null){ throw new ItemStackRequestProcessException("Another special transaction is already in progress"); } - if($repetitions < 1){ //TODO: upper bound? + if($repetitions < 1){ throw new ItemStackRequestProcessException("Cannot craft a recipe less than 1 time"); } if($repetitions > 256){ From c8d9477da122c4afe83660e6cf0c4806c16f7156 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:22:21 +0000 Subject: [PATCH 11/19] ItemStackRequestExecutor: make non-final, and make some stuff protected this allows for plugin extension, for example to implement anvils. --- .../mcpe/handler/ItemStackRequestExecutor.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index 6ac67b5dc0..002988bfff 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -58,7 +58,7 @@ use function array_key_first; use function count; use function spl_object_id; -final class ItemStackRequestExecutor{ +class ItemStackRequestExecutor{ private TransactionBuilder $builder; /** @var ItemStackRequestSlotInfo[] */ @@ -81,7 +81,7 @@ final class ItemStackRequestExecutor{ $this->builder = new TransactionBuilder(); } - private function prettyInventoryAndSlot(Inventory $inventory, int $slot) : string{ + protected function prettyInventoryAndSlot(Inventory $inventory, int $slot) : string{ if($inventory instanceof TransactionBuilderInventory){ $inventory = $inventory->getActualInventory(); } @@ -111,7 +111,7 @@ final class ItemStackRequestExecutor{ * * @throws ItemStackRequestProcessException */ - private function getBuilderInventoryAndSlot(ItemStackRequestSlotInfo $info) : array{ + protected function getBuilderInventoryAndSlot(ItemStackRequestSlotInfo $info) : array{ $windowId = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId()); $windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $info->getSlotId()); if($windowAndSlot === null){ @@ -129,7 +129,7 @@ final class ItemStackRequestExecutor{ return [$this->builder->getInventory($inventory), $slot]; } - private function transferItems(ItemStackRequestSlotInfo $source, ItemStackRequestSlotInfo $destination, int $count) : void{ + protected function transferItems(ItemStackRequestSlotInfo $source, ItemStackRequestSlotInfo $destination, int $count) : void{ $removed = $this->removeItemFromSlot($source, $count); $this->addItemToSlot($destination, $removed, $count); } @@ -138,7 +138,7 @@ final class ItemStackRequestExecutor{ * Deducts items from an inventory slot, returning a stack containing the removed items. * @throws ItemStackRequestProcessException */ - private function removeItemFromSlot(ItemStackRequestSlotInfo $slotInfo, int $count) : Item{ + protected function removeItemFromSlot(ItemStackRequestSlotInfo $slotInfo, int $count) : Item{ $this->requestSlotInfos[] = $slotInfo; [$inventory, $slot] = $this->getBuilderInventoryAndSlot($slotInfo); if($count < 1){ @@ -160,7 +160,7 @@ final class ItemStackRequestExecutor{ /** * Adds items to the target slot, if they are stackable. */ - private function addItemToSlot(ItemStackRequestSlotInfo $slotInfo, Item $item, int $count) : void{ + protected function addItemToSlot(ItemStackRequestSlotInfo $slotInfo, Item $item, int $count) : void{ $this->requestSlotInfos[] = $slotInfo; [$inventory, $slot] = $this->getBuilderInventoryAndSlot($slotInfo); if($count < 1){ @@ -182,7 +182,7 @@ final class ItemStackRequestExecutor{ /** * @throws ItemStackRequestProcessException */ - private function setNextCreatedItem(?Item $item, bool $creative = false) : void{ + protected function setNextCreatedItem(?Item $item, bool $creative = false) : void{ if($item !== null && $item->isNull()){ $item = null; } @@ -204,7 +204,7 @@ final class ItemStackRequestExecutor{ /** * @throws ItemStackRequestProcessException */ - private function beginCrafting(int $recipeId, int $repetitions) : void{ + protected function beginCrafting(int $recipeId, int $repetitions) : void{ if($this->specialTransaction !== null){ throw new ItemStackRequestProcessException("Another special transaction is already in progress"); } @@ -242,7 +242,7 @@ final class ItemStackRequestExecutor{ } } - private function takeCreatedItem(ItemStackRequestSlotInfo $destination, int $count) : void{ + protected function takeCreatedItem(ItemStackRequestSlotInfo $destination, int $count) : void{ if($count < 1){ //this should be impossible at the protocol level, but in case of buggy core code this will prevent exploits throw new ItemStackRequestProcessException("Cannot take less than 1 created item"); @@ -282,7 +282,7 @@ final class ItemStackRequestExecutor{ /** * @throws ItemStackRequestProcessException */ - private function processItemStackRequestAction(ItemStackRequestAction $action) : void{ + protected function processItemStackRequestAction(ItemStackRequestAction $action) : void{ if( $action instanceof TakeStackRequestAction || $action instanceof PlaceStackRequestAction From 1a9322c00a10b6cb06a43696bd1cd0b0b70ae554 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:23:31 +0000 Subject: [PATCH 12/19] ItemStackRequestExecutor: added some missing @throws --- src/network/mcpe/handler/ItemStackRequestExecutor.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index 002988bfff..bdc2937a5e 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -129,6 +129,9 @@ class ItemStackRequestExecutor{ return [$this->builder->getInventory($inventory), $slot]; } + /** + * @throws ItemStackRequestProcessException + */ protected function transferItems(ItemStackRequestSlotInfo $source, ItemStackRequestSlotInfo $destination, int $count) : void{ $removed = $this->removeItemFromSlot($source, $count); $this->addItemToSlot($destination, $removed, $count); @@ -159,6 +162,7 @@ class ItemStackRequestExecutor{ /** * Adds items to the target slot, if they are stackable. + * @throws ItemStackRequestProcessException */ protected function addItemToSlot(ItemStackRequestSlotInfo $slotInfo, Item $item, int $count) : void{ $this->requestSlotInfos[] = $slotInfo; @@ -242,6 +246,9 @@ class ItemStackRequestExecutor{ } } + /** + * @throws ItemStackRequestProcessException + */ protected function takeCreatedItem(ItemStackRequestSlotInfo $destination, int $count) : void{ if($count < 1){ //this should be impossible at the protocol level, but in case of buggy core code this will prevent exploits From 66a4c4c88b1d70d2705bf32e51d80802bdc651ae Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:26:19 +0000 Subject: [PATCH 13/19] Release 4.18.0-ALPHA2 --- changelogs/4.18-alpha.md | 46 ++++++++++++++++++++++++++++++++++++++++ src/VersionInfo.php | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/changelogs/4.18-alpha.md b/changelogs/4.18-alpha.md index d2c5998fc9..fde35a1da7 100644 --- a/changelogs/4.18-alpha.md +++ b/changelogs/4.18-alpha.md @@ -43,3 +43,49 @@ Released 16th March 2023. - `StandardPacketBroadcaster` is now locked to a single `PacketSerializer` context, reducing complexity. - Introduced `NetworkBroadcastUtils::broadcastPackets()`, replacing `Server->broadcastPackets()`. - `Server->broadcastPackets()` has been deprecated. It will be removed in a future version. + +# 4.18.0-ALPHA2 +Released 21st March 2023. + +## General +- Included more sections of the network system in Player Network Send timings. +- Changed the names of some timings to make them more user-friendly. +- Removed packet IDs from `receivePacket` and `sendPacket` timings, as they were not very useful. +- Added new specialized timers for the following: + - Item entity base ticking (merging) + - Player movement processing + - Entity movement processing (collision checking section) + - Projectile movement (all) + - Projectile movement processing (ray tracing section) + +## API +### `pocketmine\crafting` +- The following new API methods have been added: + - `CraftingManager->getCraftingRecipeIndex() : array` - returns a list of all crafting recipes + - `CraftingManager->getCraftingRecipeFromIndex(int $index) : ?CraftingRecipe` - returns the crafting recipe at the given index, or null if it doesn't exist + +### `pocketmine\inventory\transaction` +- The following API methods have changed signatures: + - `CraftingTransaction->__construct()` now accepts additional arguments `?CraftingRecipe $recipe = null, ?int $repetitions = null` +- The following new API methods have been added: + - `TransactionBuilderInventory->getActualInventory() : Inventory` - returns the actual inventory that this inventory is a proxy for + +## Internals +### Network +- Introduced support for the `ItemStackRequest` Minecraft: Bedrock network protocol. + - This fixes a large number of inventory- and crafting-related bugs. + - This also improves server security by closing off many code pathways that might have been used for exploits. `TypeConverter->netItemStackToCore()` is no longer used in server code, and remains for tool usage only. + - This system is also significantly more bandwidth-efficient and has lower overhead than the legacy system. + - This now opens the gateway to easily implement lots of gameplay features which have been missing for a long time, such as enchanting, anvils, looms, and more. + - Significant changes have been made to `pocketmine\network\mcpe\InventoryManager` internals. These shouldn't affect plugins, but may affect plugins which use internal network API. + - **No changes have been made to the plugin `InventoryTransaction` API**. + - This system has been implemented as a shim for the existing PocketMine-MP transaction system to preserve plugin compatibility. Plugins using `InventoryTransactionEvent` should continue to work seamlessly. + - The `InventoryTransaction` API will be redesigned in a future major version to make use of the new information provided by the `ItemStackRequest` system. + - `InventoryTransactionPacket` is no longer sent by the client for "regular" inventory actions. However, it is still sent when dropping items, interacting with blocks, and using items. +- Inventory slot and content syncing is now buffered until the end of the tick. This reduces outbound network usage when the client performs multiple transactions in a single tick (e.g. crafting a stack of items). +- Renamed some `InventoryManager` internal properties to make them easier to understand. +- `TypeConverter->createInventoryAction()` has been removed. +- Packet batch limit has been lowered to `100` packets. With the introduction of `ItemStackRequest`, this is more than sufficient for normal gameplay. + +### Other +- Use `Vector3::zero()` instead of `new Vector3()` in some places. \ No newline at end of file diff --git a/src/VersionInfo.php b/src/VersionInfo.php index b99f51fc9d..194d834d87 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -32,7 +32,7 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; public const BASE_VERSION = "4.18.0-ALPHA2"; - public const IS_DEVELOPMENT_BUILD = true; + public const IS_DEVELOPMENT_BUILD = false; public const BUILD_CHANNEL = "alpha"; private function __construct(){ From 043e81e737f165cf6c39b0edb789393d14fe6625 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 00:26:19 +0000 Subject: [PATCH 14/19] 4.18.0-ALPHA3 is next --- src/VersionInfo.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 194d834d87..779298f360 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,8 +31,8 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "4.18.0-ALPHA2"; - public const IS_DEVELOPMENT_BUILD = false; + public const BASE_VERSION = "4.18.0-ALPHA3"; + public const IS_DEVELOPMENT_BUILD = true; public const BUILD_CHANNEL = "alpha"; private function __construct(){ From ea386c42d305448f95b2e6511f1a240b127258a1 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Tue, 21 Mar 2023 14:45:18 +0000 Subject: [PATCH 15/19] InGamePacketHandler: fixed dropping items from unselected hotbar slots --- .../mcpe/handler/InGamePacketHandler.php | 64 +++++++++++-------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 6dbeaa8f4e..213ce2b8c5 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -386,39 +386,51 @@ class InGamePacketHandler extends PacketHandler{ throw new PacketHandlingException("Expected exactly 2 actions for dropping an item"); } + $sourceSlot = null; + $clientItemStack = null; + $droppedCount = null; + foreach($data->getActions() as $networkInventoryAction){ if($networkInventoryAction->sourceType === NetworkInventoryAction::SOURCE_WORLD && $networkInventoryAction->inventorySlot == NetworkInventoryAction::ACTION_MAGIC_SLOT_DROP_ITEM){ - //drop item - we don't need to validate this, we only care about the count - //if the resulting actions don't match the client for some reason, it will trigger an automatic - //prediction rollback anyway. - //it's technically possible to see this more than once, but a normal client should never do that. - $droppedItemStack = $networkInventoryAction->newItem->getItemStack(); - if($droppedItemStack->getCount() <= 0){ + $droppedCount = $networkInventoryAction->newItem->getItemStack()->getCount(); + if($droppedCount <= 0){ throw new PacketHandlingException("Expected positive count for dropped item"); } - $inventory = $this->player->getInventory(); - - $heldItem = $inventory->getItemInHand(); - $heldItemStack = TypeConverter::getInstance()->coreItemStackToNet($heldItem); - //because the client doesn't tell us the expected itemstack ID, we have to deep-compare our known - //itemstack info with the one the client sent. This is costly, but we don't have any other option :( - if($heldItemStack->getCount() < $droppedItemStack->getCount() || !$heldItemStack->equalsWithoutCount($droppedItemStack)){ - return false; - } - - //this modifies $heldItem - $droppedItem = $heldItem->pop($droppedItemStack->getCount()); - - $builder = new TransactionBuilder(); - $builder->getInventory($inventory)->setItem($inventory->getHeldItemIndex(), $heldItem); - $builder->addAction(new DropItemAction($droppedItem)); - - $transaction = new InventoryTransaction($this->player, $builder->generateActions()); - return $this->executeInventoryTransaction($transaction, $itemStackRequestId); + }elseif($networkInventoryAction->sourceType === NetworkInventoryAction::SOURCE_CONTAINER && $networkInventoryAction->windowId === ContainerIds::INVENTORY){ + //mobile players can drop an item from a non-selected hotbar slot + $sourceSlot = $networkInventoryAction->inventorySlot; + $clientItemStack = $networkInventoryAction->oldItem->getItemStack(); + }else{ + throw new PacketHandlingException("Unexpected action type in drop item transaction"); } } + if($sourceSlot === null || $clientItemStack === null || $droppedCount === null){ + throw new PacketHandlingException("Missing information in drop item transaction, need source slot, client item stack and dropped count"); + } - throw new PacketHandlingException("Legacy 'normal' transactions should only be used for dropping items"); + $inventory = $this->player->getInventory(); + + if(!$inventory->slotExists($sourceSlot)){ + return false; //TODO: size desync?? + } + + $sourceSlotItem = $inventory->getItem($sourceSlot); + $serverItemStack = TypeConverter::getInstance()->coreItemStackToNet($sourceSlotItem); + //because the client doesn't tell us the expected itemstack ID, we have to deep-compare our known + //itemstack info with the one the client sent. This is costly, but we don't have any other option :( + if(!$serverItemStack->equals($clientItemStack)){ + return false; + } + + //this modifies $sourceSlotItem + $droppedItem = $sourceSlotItem->pop($droppedCount); + + $builder = new TransactionBuilder(); + $builder->getInventory($inventory)->setItem($sourceSlot, $sourceSlotItem); + $builder->addAction(new DropItemAction($droppedItem)); + + $transaction = new InventoryTransaction($this->player, $builder->generateActions()); + return $this->executeInventoryTransaction($transaction, $itemStackRequestId); } private function handleUseItemTransaction(UseItemTransactionData $data) : bool{ From b11457d605272dc9610be95dd5e81efd48d13b41 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 22 Mar 2023 22:24:25 +0000 Subject: [PATCH 16/19] Fixed uncaught exception when retrieving a packet from the pool --- src/network/mcpe/NetworkSession.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 071c754b7f..ffd47c201d 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -120,6 +120,7 @@ use pocketmine\player\XboxLivePlayerInfo; use pocketmine\Server; use pocketmine\timings\Timings; use pocketmine\utils\AssumptionFailedError; +use pocketmine\utils\BinaryDataException; use pocketmine\utils\BinaryStream; use pocketmine\utils\ObjectSet; use pocketmine\utils\TextFormat; @@ -414,7 +415,7 @@ class NetworkSession{ throw PacketHandlingException::wrap($e, "Error processing " . $packet->getName()); } } - }catch(PacketDecodeException $e){ + }catch(PacketDecodeException|BinaryDataException $e){ $this->logger->logException($e); throw PacketHandlingException::wrap($e, "Packet batch decode error"); } From db59f71130c0f0d2cdbeb6f20502c32c4bb1d46a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 22 Mar 2023 22:29:00 +0000 Subject: [PATCH 17/19] attempt to fix ghcr.io docker image push --- .github/workflows/build-docker-image.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/build-docker-image.yml b/.github/workflows/build-docker-image.yml index 4a4b0d35af..918ec2cb11 100644 --- a/.github/workflows/build-docker-image.yml +++ b/.github/workflows/build-docker-image.yml @@ -20,6 +20,13 @@ jobs: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} + - name: Login to GitHub Container Registry + uses: docker/login-action@v2 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Clone pmmp/PocketMine-Docker repository uses: actions/checkout@v3 with: From 00286e761c7461aa64950efa23f6492d29ea7e84 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 22 Mar 2023 22:35:24 +0000 Subject: [PATCH 18/19] Release 4.17.1 --- changelogs/4.17.md | 21 ++++++++++++++++++++- src/VersionInfo.php | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/changelogs/4.17.md b/changelogs/4.17.md index d35cc77d90..58669cc250 100644 --- a/changelogs/4.17.md +++ b/changelogs/4.17.md @@ -11,4 +11,23 @@ Released 14th March 2023. ## General - Added support for Minecraft: Bedrock Edition 1.19.70. -- Removed support for older versions. \ No newline at end of file +- Removed support for older versions. + +# 4.17.1 +Released 22nd March 2023. + +## General +- Docker images for PocketMine-MP are now published on [GitHub Container Registry](https://github.com/pmmp/PocketMine-MP/pkgs/container/pocketmine-mp). The Docker Hub images will stop being maintained in the future. +- Updated translations. + +## Fixes +- Fixed server crash on empty packets in certain cases. +- Fixed mushroom blocks dropping the wrong items when broken with a silk-touch tool. +- Fixed mushroom blocks giving the wrong items when block-picked. +- Fixed missing ability flag `PRIVILEGED_BUILDER`. + +## Internals +- `update-updater-api.yml` workflow now uses `github.repository_owner` to make it easier to test the workflow on forks. +- Added version-specific channels to `update.pmmp.io`, such as `4`, `4.18-beta`, `4.17`, etc. +- Replaced deprecated `::set-output` commands in GitHub Actions workflows. +- `build/make-release.php` no longer automatically pushes changes, to avoid accidents when testing release workflows on forks. diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 880fda6981..0a9abc9f84 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -32,7 +32,7 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; public const BASE_VERSION = "4.17.1"; - public const IS_DEVELOPMENT_BUILD = true; + public const IS_DEVELOPMENT_BUILD = false; public const BUILD_CHANNEL = "stable"; private function __construct(){ From 0b8193aeb35f9394d0fccf82f58a47d9c604effb Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 22 Mar 2023 22:35:25 +0000 Subject: [PATCH 19/19] 4.17.2 is next --- src/VersionInfo.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 0a9abc9f84..87214e6919 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,8 +31,8 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "4.17.1"; - public const IS_DEVELOPMENT_BUILD = false; + public const BASE_VERSION = "4.17.2"; + public const IS_DEVELOPMENT_BUILD = true; public const BUILD_CHANNEL = "stable"; private function __construct(){