Merge branch 'stable' into minor-next

changelogs/4.20.md

@@ -34,3 +34,13 @@ Released 26th April 2023.
 ### `pocketmine\player`
 - The following API methods have been added:
   - `public Player->openSignEditor(Vector3 $position) : void` - opens the client-side sign editor GUI for the given position
+
+# 4.20.1
+Released 27th April 2023.
+
+## Fixes
+- Fixed server crash when firing a bow while holding arrows in the offhand slot.
+
+## Internals
+- `ItemStackContainerIdTranslator::translate()` now requires an additional `int $slotId` parameter and returns `array{int, int}` (translated window ID, translated slot ID) to be used with `InventoryManager->locateWindowAndSlot()`.
+- `InventoryManager->locateWindowAndSlot()` now checks if the translated slot actually exists in the requested inventory, and returns `null` if not. Previously, it would return potentially invalid slot IDs without checking them, potentially leading to crashes.

src/VersionInfo.php

@@ -31,7 +31,7 @@ use function str_repeat;
 
 final class VersionInfo{
 	public const NAME = "PocketMine-MP";
-	public const BASE_VERSION = "4.20.1";
+	public const BASE_VERSION = "4.20.2";
 	public const IS_DEVELOPMENT_BUILD = true;
 	public const BUILD_CHANNEL = "stable";
 

src/inventory/BaseInventory.php

@@ -219,6 +219,7 @@ abstract class BaseInventory implements Inventory{
 			$item = $this->getItem($i);
 			if($item->isNull()){
 				$emptySlots[] = $i;
+				continue;
 			}
 
 			if($slot->canStackWith($item) && $item->getCount() < $item->getMaxStackSize()){

src/network/mcpe/InventoryManager.php

@@ -205,11 +205,13 @@ class InventoryManager{
 			if($entry === null){
 				return null;
 			}
+			$inventory = $entry->getInventory();
 			$coreSlotId = $entry->mapNetToCore($netSlotId);
-			return $coreSlotId !== null ? [$entry->getInventory(), $coreSlotId] : null;
+			return $coreSlotId !== null && $inventory->slotExists($coreSlotId) ? [$inventory, $coreSlotId] : null;
 		}
-		if(isset($this->networkIdToInventoryMap[$windowId])){
+		$inventory = $this->networkIdToInventoryMap[$windowId] ?? null;
-			return [$this->networkIdToInventoryMap[$windowId], $netSlotId];
+		if($inventory !== null && $inventory->slotExists($netSlotId)){
+			return [$inventory, $netSlotId];
 		}
 		return null;
 	}

src/network/mcpe/handler/InGamePacketHandler.php

@@ -356,8 +356,8 @@ class InGamePacketHandler extends PacketHandler{
 		//rejects the transaction. The most common example of this is equipping armor by right-click, which doesn't send
 		//a legacy prediction action for the destination armor slot.
 		foreach($packet->requestChangedSlots as $containerInfo){
-			$windowId = ItemStackContainerIdTranslator::translate($containerInfo->getContainerId(), $this->inventoryManager->getCurrentWindowId());
+			foreach($containerInfo->getChangedSlotIndexes() as $netSlot){
-			foreach($containerInfo->getChangedSlotIndexes() as $slot){
+				[$windowId, $slot] = ItemStackContainerIdTranslator::translate($containerInfo->getContainerId(), $this->inventoryManager->getCurrentWindowId(), $netSlot);
 				$inventoryAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slot);
 				if($inventoryAndSlot !== null){ //trigger the normal slot sync logic
 					$this->inventoryManager->onSlotChange($inventoryAndSlot[0], $inventoryAndSlot[1]);

src/network/mcpe/handler/ItemStackContainerIdTranslator.php

@@ -33,15 +33,21 @@ final class ItemStackContainerIdTranslator{
 		//NOOP
 	}
 
-	public static function translate(int $containerInterfaceId, int $currentWindowId) : int{
+	/**
+	 * @return int[]
+	 * @phpstan-return array{int, int}
+	 * @throws PacketHandlingException
+	 */
+	public static function translate(int $containerInterfaceId, int $currentWindowId, int $slotId) : array{
 		return match($containerInterfaceId){
-			ContainerUIIds::ARMOR => ContainerIds::ARMOR,
+			ContainerUIIds::ARMOR => [ContainerIds::ARMOR, $slotId],
 
 			ContainerUIIds::HOTBAR,
 			ContainerUIIds::INVENTORY,
-			ContainerUIIds::COMBINED_HOTBAR_AND_INVENTORY => ContainerIds::INVENTORY,
+			ContainerUIIds::COMBINED_HOTBAR_AND_INVENTORY => [ContainerIds::INVENTORY, $slotId],
 
-			ContainerUIIds::OFFHAND => ContainerIds::OFFHAND,
+			//TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70 (though this doesn't really matter since the offhand has only 1 slot anyway)
+			ContainerUIIds::OFFHAND => [ContainerIds::OFFHAND, 0],
 
 			ContainerUIIds::ANVIL_INPUT,
 			ContainerUIIds::ANVIL_MATERIAL,
@@ -68,7 +74,7 @@ final class ItemStackContainerIdTranslator{
 			ContainerUIIds::TRADE2_INGREDIENT1,
 			ContainerUIIds::TRADE2_INGREDIENT2,
 			ContainerUIIds::TRADE_INGREDIENT1,
-			ContainerUIIds::TRADE_INGREDIENT2 => ContainerIds::UI,
+			ContainerUIIds::TRADE_INGREDIENT2 => [ContainerIds::UI, $slotId],
 
 			ContainerUIIds::BARREL,
 			ContainerUIIds::BLAST_FURNACE_INGREDIENT,
@@ -80,7 +86,7 @@ final class ItemStackContainerIdTranslator{
 			ContainerUIIds::FURNACE_RESULT,
 			ContainerUIIds::LEVEL_ENTITY, //chest
 			ContainerUIIds::SHULKER_BOX,
-			ContainerUIIds::SMOKER_INGREDIENT => $currentWindowId,
+			ContainerUIIds::SMOKER_INGREDIENT => [$currentWindowId, $slotId],
 
 			//all preview slots are ignored, since the client shouldn't be modifying those directly
 

src/network/mcpe/handler/ItemStackRequestExecutor.php

@@ -111,12 +111,7 @@ class ItemStackRequestExecutor{
 	 * @throws ItemStackRequestProcessException
 	 */
 	protected function getBuilderInventoryAndSlot(ItemStackRequestSlotInfo $info) : array{
-		$windowId = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId());
+		[$windowId, $slotId] = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId(), $info->getSlotId());
-		$slotId = $info->getSlotId();
-		if($info->getContainerId() === ContainerUIIds::OFFHAND && $slotId === 1){
-			//TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70
-			$slotId = 0;
-		}
 		$windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId);
 		if($windowAndSlot === null){
 			throw new ItemStackRequestProcessException("No open inventory matches container UI ID: " . $info->getContainerId() . ", slot ID: " . $info->getSlotId());

src/network/mcpe/handler/ItemStackResponseBuilder.php

@@ -53,11 +53,7 @@ final class ItemStackResponseBuilder{
 	 * @phpstan-return array{Inventory, int}
 	 */
 	private function getInventoryAndSlot(int $containerInterfaceId, int $slotId) : ?array{
-		if($containerInterfaceId === ContainerUIIds::OFFHAND && $slotId === 1){
+		[$windowId, $slotId] = ItemStackContainerIdTranslator::translate($containerInterfaceId, $this->inventoryManager->getCurrentWindowId(), $slotId);
-			//TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70
-			$slotId = 0;
-		}
-		$windowId = ItemStackContainerIdTranslator::translate($containerInterfaceId, $this->inventoryManager->getCurrentWindowId());
 		$windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId);
 		if($windowAndSlot === null){
 			return null;