mirrors/PocketMine-MP
1
0
Fork 0
mirror of https://github.com/pmmp/PocketMine-MP.git synced 2025-04-19 15:35:52 +00:00
Code Issues Projects Releases Wiki Activity

Properly handle transaction building errors instead of kicking the player

Browse Source
This commit is contained in:
Dylan K. Taylor 2023-03-20 00:52:26 +00:00
parent 4864444440
commit 7b0816e42f
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
5 changed files with 104 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/inventory/transaction/TransactionBuilderInventory.php
View File

@ -50,6 +50,10 @@ final class TransactionBuilderInventory extends BaseInventory{
$this->changedSlots = new \SplFixedArray($this->actualInventory->getSize());
}
public function getActualInventory() : Inventory{
return $this->actualInventory;
}
protected function internalSetContents(array $items) : void{
for($i = 0, $size = $this->getSize(); $i < $size; ++$i){
if(!isset($items[$i])){

23
src/network/mcpe/InventoryManager.php
View File

@ -555,27 +555,4 @@ class InventoryManager{
$info = new ItemStackInfo($itemStackRequestId, $itemStack->getId() === 0 ? 0 : $this->newItemStackId(), $itemStack);
return $this->itemStackInfos[spl_object_id($inventory)][$slotId] = $info;
}
public function matchItemStack(Inventory $inventory, int $slotId, int $clientItemStackId) : bool{
$inventoryObjectId = spl_object_id($inventory);
if(!isset($this->itemStackInfos[$inventoryObjectId])){
$this->session->getLogger()->debug("Attempted to match item preimage unsynced inventory " . get_class($inventory) . "#" . $inventoryObjectId);
return false;
}
$info = $this->itemStackInfos[$inventoryObjectId][$slotId] ?? null;
if($info === null){
$this->session->getLogger()->debug("Attempted to match item preimage for unsynced slot $slotId in " . get_class($inventory) . "#$inventoryObjectId that isn't synced");
return false;
}
if(!($clientItemStackId < 0 ? $info->getRequestId() === $clientItemStackId : $info->getStackId() === $clientItemStackId)){
$this->session->getLogger()->debug(
"Mismatched expected itemstack: " . get_class($inventory) . "#" . $inventoryObjectId . ", " .
"slot: $slotId, client expected: $clientItemStackId, server actual: " . $info->getStackId() . ", last modified by request: " . ($info->getRequestId() ?? "none")
);
return false;
}
return true;
}
}

13
src/network/mcpe/handler/InGamePacketHandler.php
View File

@ -110,11 +110,13 @@ use pocketmine\player\Player;
use pocketmine\utils\AssumptionFailedError;
use pocketmine\utils\Limits;
use pocketmine\utils\TextFormat;
use pocketmine\utils\Utils;
use pocketmine\world\format\Chunk;
use function array_push;
use function base64_encode;
use function count;
use function fmod;
use function implode;
use function in_array;
use function is_bool;
use function is_infinite;
@ -531,9 +533,14 @@ class InGamePacketHandler extends PacketHandler{
private function handleSingleItemStackRequest(ItemStackRequest $request) : ItemStackResponse{
$executor = new ItemStackRequestExecutor($this->player, $this->inventoryManager, $request);
$transaction = $executor->generateInventoryTransaction();
$result = $this->executeInventoryTransaction($transaction, $request->getRequestId());
$this->session->getLogger()->debug("Item stack request " . $request->getRequestId() . " result: " . ($result ? "success" : "failure"));
try{
$transaction = $executor->generateInventoryTransaction();
$result = $this->executeInventoryTransaction($transaction, $request->getRequestId());
}catch(ItemStackRequestProcessException $e){
$result = false;
$this->session->getLogger()->debug("ItemStackRequest #" . $request->getRequestId() . " failed: " . $e->getMessage());
$this->session->getLogger()->debug(implode("\n", Utils::printableExceptionInfo($e)));
}
return $executor->buildItemStackResponse($result);
}

108
src/network/mcpe/handler/ItemStackRequestExecutor.php
View File

@ -25,6 +25,7 @@ namespace pocketmine\network\mcpe\handler;
use pocketmine\crafting\CraftingGrid;
use pocketmine\inventory\CreativeInventory;
use pocketmine\inventory\Inventory;
use pocketmine\inventory\transaction\action\CreateItemAction;
use pocketmine\inventory\transaction\action\DestroyItemAction;
use pocketmine\inventory\transaction\action\DropItemAction;
@ -81,25 +82,49 @@ final class ItemStackRequestExecutor{
$this->builder = new TransactionBuilder();
}
private function prettyInventoryAndSlot(Inventory $inventory, int $slot) : string{
if($inventory instanceof TransactionBuilderInventory){
$inventory = $inventory->getActualInventory();
}
return (new \ReflectionClass($inventory))->getShortName() . "#" . spl_object_id($inventory) . ", slot: $slot";
}
/**
* @throws ItemStackRequestProcessException
*/
private function matchItemStack(Inventory $inventory, int $slotId, int $clientItemStackId) : void{
$info = $this->inventoryManager->getItemStackInfo($inventory, $slotId);
if($info === null){
throw new AssumptionFailedError("The inventory is tracked and the slot is valid, so this should not be null");
}
if(!($clientItemStackId < 0 ? $info->getRequestId() === $clientItemStackId : $info->getStackId() === $clientItemStackId)){
throw new ItemStackRequestProcessException(
$this->prettyInventoryAndSlot($inventory, $slotId) . ": " .
"Mismatched expected itemstack, " .
"client expected: $clientItemStackId, server actual: " . $info->getStackId() . ", last modified by request: " . ($info->getRequestId() ?? "none")
);
}
}
/**
* @phpstan-return array{TransactionBuilderInventory, int}
*
* @throws ItemStackRequestProcessException
*/
private function getBuilderInventoryAndSlot(ItemStackRequestSlotInfo $info) : array{
$windowId = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId());
$windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $info->getSlotId());
if($windowAndSlot === null){
throw new PacketHandlingException("Stack request action cannot target an inventory that is not open");
throw new ItemStackRequestProcessException("No open inventory matches container UI ID: " . $info->getContainerId() . ", slot ID: " . $info->getSlotId());
}
[$inventory, $slot] = $windowAndSlot;
if(!$inventory->slotExists($slot)){
throw new PacketHandlingException("Stack request action cannot target an inventory slot that does not exist");
throw new ItemStackRequestProcessException("No such inventory slot :" . $this->prettyInventoryAndSlot($inventory, $slot));
}
if(
$info->getStackId() !== $this->request->getRequestId() && //using TransactionBuilderInventory enables this to work
!$this->inventoryManager->matchItemStack($inventory, $slot, $info->getStackId())
){
throw new PacketHandlingException("Inventory " . $info->getContainerId() . ", slot " . $slot . ": server-side item does not match expected");
if($info->getStackId() !== $this->request->getRequestId()){ //the itemstack may have been modified by the current request
$this->matchItemStack($inventory, $slot, $info->getStackId());
}
return [$this->builder->getInventory($inventory), $slot];
@ -112,6 +137,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{
$this->requestSlotInfos[] = $slotInfo;
@ -119,7 +145,7 @@ final class ItemStackRequestExecutor{
$existingItem = $inventory->getItem($slot);
if($existingItem->getCount() < $count){
throw new PacketHandlingException("Cannot take $count items from a stack of " . $existingItem->getCount());
throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Cannot take $count items from a stack of " . $existingItem->getCount());
}
$removed = $existingItem->pop($count);
@ -137,7 +163,7 @@ final class ItemStackRequestExecutor{
$existingItem = $inventory->getItem($slot);
if(!$existingItem->isNull() && !$existingItem->canStackWith($item)){
throw new PacketHandlingException("Can only add items to an empty slot, or a slot containing the same item");
throw new ItemStackRequestProcessException($this->prettyInventoryAndSlot($inventory, $slot) . ": Can only add items to an empty slot, or a slot containing the same item");
}
//we can't use the existing item here; it may be an empty stack
@ -146,6 +172,9 @@ final class ItemStackRequestExecutor{
$inventory->setItem($slot, $newItem);
}
/**
* @throws ItemStackRequestProcessException
*/
private function setNextCreatedItem(?Item $item, bool $creative = false) : void{
if($item !== null && $item->isNull()){
$item = null;
@ -157,7 +186,7 @@ final class ItemStackRequestExecutor{
$this->nextCreatedItem->setCount($this->createdItemsTakenCount);
$this->builder->addAction(new CreateItemAction($this->nextCreatedItem));
}elseif($this->createdItemsTakenCount < $this->nextCreatedItem->getCount()){
throw new PacketHandlingException("Not all of the previous created item was taken");
throw new ItemStackRequestProcessException("Not all of the previous created item was taken");
}
}
$this->nextCreatedItem = $item;
@ -165,24 +194,27 @@ final class ItemStackRequestExecutor{
$this->createdItemsTakenCount = 0;
}
/**
* @throws ItemStackRequestProcessException
*/
private function beginCrafting(int $recipeId, int $repetitions) : void{
if($this->specialTransaction !== null){
throw new PacketHandlingException("Cannot perform more than 1 special action per request");
throw new ItemStackRequestProcessException("Another special transaction is already in progress");
}
if($repetitions < 1){ //TODO: upper bound?
throw new PacketHandlingException("Cannot craft a recipe less than 1 time");
throw new ItemStackRequestProcessException("Cannot craft a recipe less than 1 time");
}
$craftingManager = $this->player->getServer()->getCraftingManager();
$recipe = $craftingManager->getCraftingRecipeFromIndex($recipeId);
if($recipe === null){
throw new PacketHandlingException("Unknown crafting recipe ID $recipeId");
throw new ItemStackRequestProcessException("No such crafting recipe index: $recipeId");
}
$this->specialTransaction = new CraftingTransaction($this->player, $craftingManager, [], $recipe, $repetitions);
$currentWindow = $this->player->getCurrentWindow();
if($currentWindow !== null && !($currentWindow instanceof CraftingGrid)){
throw new PacketHandlingException("Cannot complete crafting when the player's current window is not a crafting grid");
throw new ItemStackRequestProcessException("Player's current window is not a crafting grid");
}
$craftingGrid = $currentWindow ?? $this->player->getCraftingGrid();
@ -200,13 +232,13 @@ final class ItemStackRequestExecutor{
private function takeCreatedItem(ItemStackRequestSlotInfo $destination, int $count) : void{
$createdItem = $this->nextCreatedItem;
if($createdItem === null){
throw new PacketHandlingException("No created item is waiting to be taken");
throw new ItemStackRequestProcessException("No created item is waiting to be taken");
}
if(!$this->createdItemFromCreativeInventory){
$availableCount = $createdItem->getCount() - $this->createdItemsTakenCount;
if($count > $availableCount){
throw new PacketHandlingException("Not enough created items available to be taken (have $availableCount, tried to take $count)");
throw new ItemStackRequestProcessException("Not enough created items available to be taken (have $availableCount, tried to take $count)");
}
}
@ -217,6 +249,22 @@ final class ItemStackRequestExecutor{
}
}
/**
* @throws ItemStackRequestProcessException
*/
private function assertDoingCrafting() : void{
if(!$this->specialTransaction instanceof CraftingTransaction){
if($this->specialTransaction === null){
throw new ItemStackRequestProcessException("Expected CraftRecipe or CraftRecipeAuto action to precede this action");
}else{
throw new ItemStackRequestProcessException("A different special transaction is already in progress");
}
}
}
/**
* @throws ItemStackRequestProcessException
*/
private function processItemStackRequestAction(ItemStackRequestAction $action) : void{
if(
$action instanceof TakeStackRequestAction ||
@ -253,10 +301,7 @@ final class ItemStackRequestExecutor{
}elseif($action instanceof CreativeCreateStackRequestAction){
$item = CreativeInventory::getInstance()->getItem($action->getCreativeItemId());
if($item === null){
//TODO: the item may have been unregistered after the client was sent the creative contents, leaving a
//gap in the creative item list. This probably shouldn't be a violation, but I'm not sure how else to
//handle it right now.
throw new PacketHandlingException("Tried to create nonexisting creative item " . $action->getCreativeItemId());
throw new ItemStackRequestProcessException("No such creative item index: " . $action->getCreativeItemId());
}
$this->setNextCreatedItem($item, true);
@ -265,31 +310,34 @@ final class ItemStackRequestExecutor{
}elseif($action instanceof CraftRecipeAutoStackRequestAction){
$this->beginCrafting($action->getRecipeId(), $action->getRepetitions());
}elseif($action instanceof CraftingConsumeInputStackRequestAction){
if(!$this->specialTransaction instanceof CraftingTransaction){
throw new PacketHandlingException("Cannot consume crafting input when no crafting transaction is in progress");
}
$this->assertDoingCrafting();
$this->removeItemFromSlot($action->getSource(), $action->getCount()); //output discarded - we allow CraftingTransaction to verify the balance
}elseif($action instanceof CraftingCreateSpecificResultStackRequestAction){
if(!$this->specialTransaction instanceof CraftingTransaction){
throw new AssumptionFailedError("Cannot mark crafting result index when no crafting transaction is in progress");
}
$this->assertDoingCrafting();
$nextResultItem = $this->craftingResults[$action->getResultIndex()] ?? null;
if($nextResultItem === null){
throw new PacketHandlingException("No such crafting result index " . $action->getResultIndex());
throw new ItemStackRequestProcessException("No such crafting result index: " . $action->getResultIndex());
}
$this->setNextCreatedItem($nextResultItem);
}elseif($action instanceof DeprecatedCraftingResultsStackRequestAction){
//no obvious use
}else{
throw new PacketHandlingException("Unhandled item stack request action: " . get_class($action));
throw new ItemStackRequestProcessException("Unhandled item stack request action");
}
}
/**
* @throws ItemStackRequestProcessException
*/
public function generateInventoryTransaction() : InventoryTransaction{
foreach($this->request->getActions() as $action){
$this->processItemStackRequestAction($action);
foreach($this->request->getActions() as $k => $action){
try{
$this->processItemStackRequestAction($action);
}catch(ItemStackRequestProcessException $e){
throw new ItemStackRequestProcessException("Error processing action $k (" . (new \ReflectionClass($action))->getShortName() . "): " . $e->getMessage(), 0, $e);
}
}
$this->setNextCreatedItem(null);
$inventoryActions = $this->builder->generateActions();

19
src/network/mcpe/handler/ItemStackResponseBuilder.php
View File

@ -29,7 +29,7 @@ use pocketmine\network\mcpe\protocol\types\inventory\ContainerUIIds;
use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponse;
use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponseContainerInfo;
use pocketmine\network\mcpe\protocol\types\inventory\stackresponse\ItemStackResponseSlotInfo;
use pocketmine\network\PacketHandlingException;
use pocketmine\utils\AssumptionFailedError;
final class ItemStackResponseBuilder{
@ -51,15 +51,15 @@ final class ItemStackResponseBuilder{
/**
* @phpstan-return array{Inventory, int}
*/
private function getInventoryAndSlot(int $containerInterfaceId, int $slotId) : array{
private function getInventoryAndSlot(int $containerInterfaceId, int $slotId) : ?array{
$windowId = ItemStackContainerIdTranslator::translate($containerInterfaceId, $this->inventoryManager->getCurrentWindowId());
$windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId);
if($windowAndSlot === null){
throw new PacketHandlingException("Stack request action cannot target an inventory that is not open");
return null;
}
[$inventory, $slot] = $windowAndSlot;
if(!$inventory->slotExists($slot)){
throw new PacketHandlingException("Stack request action cannot target an inventory slot that does not exist");
return null;
}
return [$inventory, $slot];
@ -72,16 +72,21 @@ final class ItemStackResponseBuilder{
continue;
}
foreach($slotIds as $slotId){
[$inventory, $slot] = $this->getInventoryAndSlot($containerInterfaceId, $slotId);
$inventoryAndSlot = $this->getInventoryAndSlot($containerInterfaceId, $slotId);
if($inventoryAndSlot === null){
//a plugin may have closed the inventory during an event, or the slot may have been invalid
continue;
}
[$inventory, $slot] = $inventoryAndSlot;
$itemStackInfo = $this->inventoryManager->getItemStackInfo($inventory, $slot);
if($itemStackInfo === null){
//TODO: what if a plugin closes the inventory while the transaction is ongoing?
throw new \LogicException("ItemStackInfo should never be null for an open inventory");
throw new AssumptionFailedError("ItemStackInfo should never be null for an open inventory");
}
if($itemStackInfo->getRequestId() !== $this->requestId){
//the itemstack may have been synced due to transaction producing results that the client did not
//predict correctly, which will wipe out the tracked request ID (intentionally)
//TODO: is this the correct behaviour?
continue;
}
$item = $inventory->getItem($slot);