From 6375139d0b9ae8186c8f83afb5f675fd8b4fde4f Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 24 Dec 2022 17:47:17 +0000 Subject: [PATCH 1/5] DefaultPermissions: improve readability slightly --- src/permission/DefaultPermissions.php | 142 +++++++++++++------------- 1 file changed, 72 insertions(+), 70 deletions(-) diff --git a/src/permission/DefaultPermissions.php b/src/permission/DefaultPermissions.php index 6f9c79b80..be68ebd3f 100644 --- a/src/permission/DefaultPermissions.php +++ b/src/permission/DefaultPermissions.php @@ -23,10 +23,12 @@ declare(strict_types=1); namespace pocketmine\permission; +use pocketmine\permission\DefaultPermissionNames as Names; + abstract class DefaultPermissions{ - public const ROOT_CONSOLE = DefaultPermissionNames::GROUP_CONSOLE; - public const ROOT_OPERATOR = DefaultPermissionNames::GROUP_OPERATOR; - public const ROOT_USER = DefaultPermissionNames::GROUP_USER; + public const ROOT_CONSOLE = Names::GROUP_CONSOLE; + public const ROOT_OPERATOR = Names::GROUP_OPERATOR; + public const ROOT_USER = Names::GROUP_USER; /** * @param Permission[] $grantedBy @@ -55,84 +57,84 @@ abstract class DefaultPermissions{ $operatorRoot = self::registerPermission(new Permission(self::ROOT_OPERATOR, "Grants all operator permissions"), [$consoleRoot]); $everyoneRoot = self::registerPermission(new Permission(self::ROOT_USER, "Grants all non-sensitive permissions that everyone gets by default"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::BROADCAST_ADMIN, "Allows the user to receive administrative broadcasts"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::BROADCAST_USER, "Allows the user to receive user broadcasts"), [$everyoneRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_BAN_IP, "Allows the user to ban IP addresses"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_BAN_LIST, "Allows the user to list banned players"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_BAN_PLAYER, "Allows the user to ban players"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_CLEAR_OTHER, "Allows the user to clear inventory of other players"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_CLEAR_SELF, "Allows the user to clear their own inventory"), [$everyoneRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_DEFAULTGAMEMODE, "Allows the user to change the default gamemode"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_DIFFICULTY, "Allows the user to change the game difficulty"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_DUMPMEMORY, "Allows the user to dump memory contents"), [$consoleRoot]); + self::registerPermission(new Permission(Names::BROADCAST_ADMIN, "Allows the user to receive administrative broadcasts"), [$operatorRoot]); + self::registerPermission(new Permission(Names::BROADCAST_USER, "Allows the user to receive user broadcasts"), [$everyoneRoot]); + self::registerPermission(new Permission(Names::COMMAND_BAN_IP, "Allows the user to ban IP addresses"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_BAN_LIST, "Allows the user to list banned players"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_BAN_PLAYER, "Allows the user to ban players"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_CLEAR_OTHER, "Allows the user to clear inventory of other players"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_CLEAR_SELF, "Allows the user to clear their own inventory"), [$everyoneRoot]); + self::registerPermission(new Permission(Names::COMMAND_DEFAULTGAMEMODE, "Allows the user to change the default gamemode"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_DIFFICULTY, "Allows the user to change the game difficulty"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_DUMPMEMORY, "Allows the user to dump memory contents"), [$consoleRoot]); - $effectRoot = self::registerDeprecatedPermission(DefaultPermissionNames::COMMAND_EFFECT); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_EFFECT_OTHER, "Allows the user to modify effects of other players"), [$operatorRoot, $effectRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_EFFECT_SELF, "Allows the user to modify their own effects"), [$operatorRoot, $effectRoot]); + $effectRoot = self::registerDeprecatedPermission(Names::COMMAND_EFFECT); + self::registerPermission(new Permission(Names::COMMAND_EFFECT_OTHER, "Allows the user to modify effects of other players"), [$operatorRoot, $effectRoot]); + self::registerPermission(new Permission(Names::COMMAND_EFFECT_SELF, "Allows the user to modify their own effects"), [$operatorRoot, $effectRoot]); - $enchantRoot = self::registerDeprecatedPermission(DefaultPermissionNames::COMMAND_ENCHANT); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_ENCHANT_OTHER, "Allows the user to enchant the held items of other players"), [$operatorRoot, $enchantRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_ENCHANT_SELF, "Allows the user to enchant their own held item"), [$operatorRoot, $enchantRoot]); + $enchantRoot = self::registerDeprecatedPermission(Names::COMMAND_ENCHANT); + self::registerPermission(new Permission(Names::COMMAND_ENCHANT_OTHER, "Allows the user to enchant the held items of other players"), [$operatorRoot, $enchantRoot]); + self::registerPermission(new Permission(Names::COMMAND_ENCHANT_SELF, "Allows the user to enchant their own held item"), [$operatorRoot, $enchantRoot]); - $gameModeRoot = self::registerDeprecatedPermission(DefaultPermissionNames::COMMAND_GAMEMODE); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_GAMEMODE_OTHER, "Allows the user to change the game mode of other players"), [$operatorRoot, $gameModeRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_GAMEMODE_SELF, "Allows the user to change their own game mode"), [$operatorRoot, $gameModeRoot]); + $gameModeRoot = self::registerDeprecatedPermission(Names::COMMAND_GAMEMODE); + self::registerPermission(new Permission(Names::COMMAND_GAMEMODE_OTHER, "Allows the user to change the game mode of other players"), [$operatorRoot, $gameModeRoot]); + self::registerPermission(new Permission(Names::COMMAND_GAMEMODE_SELF, "Allows the user to change their own game mode"), [$operatorRoot, $gameModeRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_GC, "Allows the user to fire garbage collection tasks"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_GC, "Allows the user to fire garbage collection tasks"), [$operatorRoot]); - $giveRoot = self::registerDeprecatedPermission(DefaultPermissionNames::COMMAND_GIVE); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_GIVE_OTHER, "Allows the user to give items to other players"), [$operatorRoot, $giveRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_GIVE_SELF, "Allows the user to give items to themselves"), [$operatorRoot, $giveRoot]); + $giveRoot = self::registerDeprecatedPermission(Names::COMMAND_GIVE); + self::registerPermission(new Permission(Names::COMMAND_GIVE_OTHER, "Allows the user to give items to other players"), [$operatorRoot, $giveRoot]); + self::registerPermission(new Permission(Names::COMMAND_GIVE_SELF, "Allows the user to give items to themselves"), [$operatorRoot, $giveRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_HELP, "Allows the user to view the help menu"), [$everyoneRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_KICK, "Allows the user to kick players"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_KILL_OTHER, "Allows the user to kill other players"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_KILL_SELF, "Allows the user to commit suicide"), [$everyoneRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_LIST, "Allows the user to list all online players"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_ME, "Allows the user to perform a chat action"), [$everyoneRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_OP_GIVE, "Allows the user to give a player operator status"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_OP_TAKE, "Allows the user to take a player's operator status"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_PARTICLE, "Allows the user to create particle effects"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_PLUGINS, "Allows the user to view the list of plugins"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SAVE_DISABLE, "Allows the user to disable automatic saving"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SAVE_ENABLE, "Allows the user to enable automatic saving"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SAVE_PERFORM, "Allows the user to perform a manual save"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SAY, "Allows the user to talk as the console"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SEED, "Allows the user to view the seed of the world"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SETWORLDSPAWN, "Allows the user to change the world spawn"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_HELP, "Allows the user to view the help menu"), [$everyoneRoot]); + self::registerPermission(new Permission(Names::COMMAND_KICK, "Allows the user to kick players"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_KILL_OTHER, "Allows the user to kill other players"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_KILL_SELF, "Allows the user to commit suicide"), [$everyoneRoot]); + self::registerPermission(new Permission(Names::COMMAND_LIST, "Allows the user to list all online players"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_ME, "Allows the user to perform a chat action"), [$everyoneRoot]); + self::registerPermission(new Permission(Names::COMMAND_OP_GIVE, "Allows the user to give a player operator status"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_OP_TAKE, "Allows the user to take a player's operator status"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_PARTICLE, "Allows the user to create particle effects"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_PLUGINS, "Allows the user to view the list of plugins"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_SAVE_DISABLE, "Allows the user to disable automatic saving"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_SAVE_ENABLE, "Allows the user to enable automatic saving"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_SAVE_PERFORM, "Allows the user to perform a manual save"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_SAY, "Allows the user to talk as the console"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_SEED, "Allows the user to view the seed of the world"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_SETWORLDSPAWN, "Allows the user to change the world spawn"), [$operatorRoot]); - $spawnpointRoot = self::registerDeprecatedPermission(DefaultPermissionNames::COMMAND_SPAWNPOINT); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SPAWNPOINT_OTHER, "Allows the user to change the respawn point of other players"), [$operatorRoot, $spawnpointRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_SPAWNPOINT_SELF, "Allows the user to change their own respawn point"), [$operatorRoot, $spawnpointRoot]); + $spawnpointRoot = self::registerDeprecatedPermission(Names::COMMAND_SPAWNPOINT); + self::registerPermission(new Permission(Names::COMMAND_SPAWNPOINT_OTHER, "Allows the user to change the respawn point of other players"), [$operatorRoot, $spawnpointRoot]); + self::registerPermission(new Permission(Names::COMMAND_SPAWNPOINT_SELF, "Allows the user to change their own respawn point"), [$operatorRoot, $spawnpointRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_STATUS, "Allows the user to view the server performance"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_STOP, "Allows the user to stop the server"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_STATUS, "Allows the user to view the server performance"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_STOP, "Allows the user to stop the server"), [$operatorRoot]); - $teleportRoot = self::registerDeprecatedPermission(DefaultPermissionNames::COMMAND_TELEPORT); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TELEPORT_OTHER, "Allows the user to teleport other players"), [$operatorRoot, $teleportRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TELEPORT_SELF, "Allows the user to teleport themselves"), [$operatorRoot, $teleportRoot]); + $teleportRoot = self::registerDeprecatedPermission(Names::COMMAND_TELEPORT); + self::registerPermission(new Permission(Names::COMMAND_TELEPORT_OTHER, "Allows the user to teleport other players"), [$operatorRoot, $teleportRoot]); + self::registerPermission(new Permission(Names::COMMAND_TELEPORT_SELF, "Allows the user to teleport themselves"), [$operatorRoot, $teleportRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TELL, "Allows the user to privately message another player"), [$everyoneRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TIME_ADD, "Allows the user to fast-forward time"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TIME_QUERY, "Allows the user query the time"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TIME_SET, "Allows the user to change the time"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TIME_START, "Allows the user to restart the time"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TIME_STOP, "Allows the user to stop the time"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TIMINGS, "Allows the user to record timings to analyse server performance"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_TELL, "Allows the user to privately message another player"), [$everyoneRoot]); + self::registerPermission(new Permission(Names::COMMAND_TIME_ADD, "Allows the user to fast-forward time"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_TIME_QUERY, "Allows the user query the time"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_TIME_SET, "Allows the user to change the time"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_TIME_START, "Allows the user to restart the time"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_TIME_STOP, "Allows the user to stop the time"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_TIMINGS, "Allows the user to record timings to analyse server performance"), [$operatorRoot]); - $titleRoot = self::registerDeprecatedPermission(DefaultPermissionNames::COMMAND_TITLE); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TITLE_OTHER, "Allows the user to send a title to the specified player"), [$operatorRoot, $titleRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TITLE_SELF, "Allows the user to send a title to themselves"), [$operatorRoot, $titleRoot]); + $titleRoot = self::registerDeprecatedPermission(Names::COMMAND_TITLE); + self::registerPermission(new Permission(Names::COMMAND_TITLE_OTHER, "Allows the user to send a title to the specified player"), [$operatorRoot, $titleRoot]); + self::registerPermission(new Permission(Names::COMMAND_TITLE_SELF, "Allows the user to send a title to themselves"), [$operatorRoot, $titleRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_TRANSFERSERVER, "Allows the user to transfer self to another server"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_UNBAN_IP, "Allows the user to unban IP addresses"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_UNBAN_PLAYER, "Allows the user to unban players"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_VERSION, "Allows the user to view the version of the server"), [$everyoneRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_WHITELIST_ADD, "Allows the user to add a player to the server whitelist"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_WHITELIST_DISABLE, "Allows the user to disable the server whitelist"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_WHITELIST_ENABLE, "Allows the user to enable the server whitelist"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_WHITELIST_LIST, "Allows the user to list all players on the server whitelist"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_WHITELIST_RELOAD, "Allows the user to reload the server whitelist"), [$operatorRoot]); - self::registerPermission(new Permission(DefaultPermissionNames::COMMAND_WHITELIST_REMOVE, "Allows the user to remove a player from the server whitelist"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_TRANSFERSERVER, "Allows the user to transfer self to another server"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_UNBAN_IP, "Allows the user to unban IP addresses"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_UNBAN_PLAYER, "Allows the user to unban players"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_VERSION, "Allows the user to view the version of the server"), [$everyoneRoot]); + self::registerPermission(new Permission(Names::COMMAND_WHITELIST_ADD, "Allows the user to add a player to the server whitelist"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_WHITELIST_DISABLE, "Allows the user to disable the server whitelist"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_WHITELIST_ENABLE, "Allows the user to enable the server whitelist"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_WHITELIST_LIST, "Allows the user to list all players on the server whitelist"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_WHITELIST_RELOAD, "Allows the user to reload the server whitelist"), [$operatorRoot]); + self::registerPermission(new Permission(Names::COMMAND_WHITELIST_REMOVE, "Allows the user to remove a player from the server whitelist"), [$operatorRoot]); } } From f7d0d16eb3b1f4be73535a5674b2da8a2b799a94 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 24 Dec 2022 20:06:00 +0000 Subject: [PATCH 2/5] NetworkSession: defer destructive cleanup until the next session tick() call this fixes crashes when kicking players during PlayerJoinEvent and various other events. --- src/network/mcpe/NetworkSession.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index ff91aa5b5..a765815f0 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -547,13 +547,18 @@ class NetworkSession{ $this->disposeHooks->clear(); $this->setHandler(null); $this->connected = false; - $this->manager->remove($this); $this->logger->info("Session closed due to $reason"); - - $this->invManager = null; //break cycles - TODO: this really ought to be deferred until it's safe } } + /** + * Performs actions after the session has been disconnected. By this point, nothing should be interacting with the + * session, so it's safe to destroy any cycles and perform destructive cleanup. + */ + private function dispose() : void{ + $this->invManager = null; + } + /** * Disconnects the session, destroying the associated player (if it exists). */ @@ -1114,6 +1119,11 @@ class NetworkSession{ } public function tick() : void{ + if(!$this->isConnected()){ + $this->dispose(); + return; + } + if($this->info === null){ if(time() >= $this->connectTime + 10){ $this->disconnect("Login timeout"); From 80832ff763b415212c63027588e8ec9fae991dd8 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 24 Dec 2022 20:12:50 +0000 Subject: [PATCH 3/5] NetworkSession: do not send packets to disconnected sessions this is mostly harmless, since the packets land in a buffer that gets discarded, but we also need to avoid calling DataPacketSendEvent. --- src/network/mcpe/NetworkSession.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index a765815f0..57eb1f699 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -424,6 +424,9 @@ class NetworkSession{ } public function sendDataPacket(ClientboundPacket $packet, bool $immediate = false) : bool{ + if(!$this->connected){ + return false; + } //Basic safety restriction. TODO: improve this if(!$this->loggedIn && !$packet->canBeSentBeforeLogin()){ throw new \InvalidArgumentException("Attempted to send " . get_class($packet) . " to " . $this->getDisplayName() . " too early"); From b1c0eae1f69bbdc74562a7a68acaaa9254b91562 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 24 Dec 2022 20:36:18 +0000 Subject: [PATCH 4/5] NetworkSession: tidy up common disconnection logic --- src/network/mcpe/NetworkSession.php | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 57eb1f699..88b888fdd 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -544,6 +544,8 @@ class NetworkSession{ $this->disconnectGuard = true; $func(); $this->disconnectGuard = false; + $this->flushSendBuffer(true); + $this->sender->close(""); foreach($this->disposeHooks as $callback){ $callback(); } @@ -567,10 +569,12 @@ class NetworkSession{ */ public function disconnect(string $reason, bool $notify = true) : void{ $this->tryDisconnect(function() use ($reason, $notify) : void{ + if($notify){ + $this->sendDataPacket(DisconnectPacket::create($reason)); + } if($this->player !== null){ $this->player->onPostDisconnect($reason, null); } - $this->doServerDisconnect($reason, $notify); }, $reason); } @@ -583,7 +587,6 @@ class NetworkSession{ if($this->player !== null){ $this->player->onPostDisconnect($reason, null); } - $this->doServerDisconnect($reason, false); }, $reason); } @@ -592,21 +595,10 @@ class NetworkSession{ */ public function onPlayerDestroyed(string $reason) : void{ $this->tryDisconnect(function() use ($reason) : void{ - $this->doServerDisconnect($reason, true); + $this->sendDataPacket(DisconnectPacket::create($reason)); }, $reason); } - /** - * Internal helper function used to handle server disconnections. - */ - private function doServerDisconnect(string $reason, bool $notify = true) : void{ - if($notify){ - $this->sendDataPacket(DisconnectPacket::create($reason !== "" ? $reason : null), true); - } - - $this->sender->close($notify ? $reason : ""); - } - /** * Called by the network interface to close the session when the client disconnects without server input, for * example in a timeout condition or voluntary client disconnect. From 0d169b4e80b6746b9157b65c3bd83dc6eedd3903 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 25 Dec 2022 17:13:51 +0000 Subject: [PATCH 5/5] Filesystem: added fileGetContents to reduce ErrorToExceptionHandler boilerplate code --- build/make-release.php | 4 ++-- src/Server.php | 5 ++--- src/crafting/CraftingManagerFromDataHelper.php | 5 ++--- .../LegacyToStringBidirectionalIdMap.php | 5 ++--- src/inventory/CreativeInventory.php | 4 ++-- src/item/LegacyStringToItemParser.php | 5 ++--- src/network/mcpe/cache/StaticPacketCache.php | 6 ++---- .../mcpe/convert/GlobalItemTypeDictionary.php | 5 ++--- src/network/mcpe/convert/ItemTranslator.php | 4 ++-- .../mcpe/convert/RuntimeBlockMapping.php | 7 +++---- src/player/DatFilePlayerDataProvider.php | 5 ++--- src/resourcepacks/ResourcePackManager.php | 9 +++------ src/utils/Config.php | 6 +----- src/utils/Filesystem.php | 18 ++++++++++++++++++ src/world/format/io/data/BedrockWorldData.php | 8 ++++---- src/world/format/io/data/JavaWorldData.php | 8 ++++---- 16 files changed, 53 insertions(+), 51 deletions(-) diff --git a/build/make-release.php b/build/make-release.php index 2701c7500..610dbbe6d 100644 --- a/build/make-release.php +++ b/build/make-release.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace pocketmine\build\make_release; +use pocketmine\utils\Filesystem; use pocketmine\utils\Utils; use pocketmine\utils\VersionString; use pocketmine\VersionInfo; @@ -30,7 +31,6 @@ use function array_keys; use function array_map; use function dirname; use function fgets; -use function file_get_contents; use function file_put_contents; use function fwrite; use function getopt; @@ -50,7 +50,7 @@ use const STR_PAD_LEFT; require_once dirname(__DIR__) . '/vendor/autoload.php'; function replaceVersion(string $versionInfoPath, string $newVersion, bool $isDev, string $channel) : void{ - $versionInfo = Utils::assumeNotFalse(file_get_contents($versionInfoPath), $versionInfoPath . " should always exist"); + $versionInfo = Filesystem::fileGetContents($versionInfoPath); $versionInfo = preg_replace( $pattern = '/^([\t ]*public )?const BASE_VERSION = "(\d+)\.(\d+)\.(\d+)(?:-(.*))?";$/m', '$1const BASE_VERSION = "' . $newVersion . '";', diff --git a/src/Server.php b/src/Server.php index 4dfa29207..6d762d538 100644 --- a/src/Server.php +++ b/src/Server.php @@ -125,7 +125,6 @@ use function count; use function date; use function fclose; use function file_exists; -use function file_get_contents; use function file_put_contents; use function filemtime; use function fopen; @@ -777,7 +776,7 @@ class Server{ $this->logger->info("Loading server configuration"); $pocketmineYmlPath = Path::join($this->dataPath, "pocketmine.yml"); if(!file_exists($pocketmineYmlPath)){ - $content = Utils::assumeNotFalse(file_get_contents(Path::join(\pocketmine\RESOURCE_PATH, "pocketmine.yml")), "Missing required resource file"); + $content = Filesystem::fileGetContents(Path::join(\pocketmine\RESOURCE_PATH, "pocketmine.yml")); if(VersionInfo::IS_DEVELOPMENT_BUILD){ $content = str_replace("preferred-channel: stable", "preferred-channel: beta", $content); } @@ -950,7 +949,7 @@ class Server{ copy(Path::join(\pocketmine\RESOURCE_PATH, 'plugin_list.yml'), $graylistFile); } try{ - $pluginGraylist = PluginGraylist::fromArray(yaml_parse(file_get_contents($graylistFile))); + $pluginGraylist = PluginGraylist::fromArray(yaml_parse(Filesystem::fileGetContents($graylistFile))); }catch(\InvalidArgumentException $e){ $this->logger->emergency("Failed to load $graylistFile: " . $e->getMessage()); $this->forceShutdownExit(); diff --git a/src/crafting/CraftingManagerFromDataHelper.php b/src/crafting/CraftingManagerFromDataHelper.php index e91cb4ba6..a728ab93d 100644 --- a/src/crafting/CraftingManagerFromDataHelper.php +++ b/src/crafting/CraftingManagerFromDataHelper.php @@ -26,9 +26,8 @@ namespace pocketmine\crafting; use pocketmine\item\Item; use pocketmine\item\ItemFactory; use pocketmine\utils\AssumptionFailedError; -use pocketmine\utils\Utils; +use pocketmine\utils\Filesystem; use function array_map; -use function file_get_contents; use function is_array; use function json_decode; @@ -52,7 +51,7 @@ final class CraftingManagerFromDataHelper{ } public static function make(string $filePath) : CraftingManager{ - $recipes = json_decode(Utils::assumeNotFalse(file_get_contents($filePath), "Missing required resource file"), true); + $recipes = json_decode(Filesystem::fileGetContents($filePath), true); if(!is_array($recipes)){ throw new AssumptionFailedError("recipes.json root should contain a map of recipe types"); } diff --git a/src/data/bedrock/LegacyToStringBidirectionalIdMap.php b/src/data/bedrock/LegacyToStringBidirectionalIdMap.php index 2ffe1b986..c5fa272c2 100644 --- a/src/data/bedrock/LegacyToStringBidirectionalIdMap.php +++ b/src/data/bedrock/LegacyToStringBidirectionalIdMap.php @@ -24,8 +24,7 @@ declare(strict_types=1); namespace pocketmine\data\bedrock; use pocketmine\utils\AssumptionFailedError; -use pocketmine\utils\Utils; -use function file_get_contents; +use pocketmine\utils\Filesystem; use function is_array; use function is_int; use function is_string; @@ -45,7 +44,7 @@ abstract class LegacyToStringBidirectionalIdMap{ private array $stringToLegacy = []; public function __construct(string $file){ - $stringToLegacyId = json_decode(Utils::assumeNotFalse(file_get_contents($file), "Missing required resource file"), true); + $stringToLegacyId = json_decode(Filesystem::fileGetContents($file), true); if(!is_array($stringToLegacyId)){ throw new AssumptionFailedError("Invalid format of ID map"); } diff --git a/src/inventory/CreativeInventory.php b/src/inventory/CreativeInventory.php index 980c2e35f..8037e6091 100644 --- a/src/inventory/CreativeInventory.php +++ b/src/inventory/CreativeInventory.php @@ -25,9 +25,9 @@ namespace pocketmine\inventory; use pocketmine\item\Durable; use pocketmine\item\Item; +use pocketmine\utils\Filesystem; use pocketmine\utils\SingletonTrait; use Symfony\Component\Filesystem\Path; -use function file_get_contents; use function json_decode; final class CreativeInventory{ @@ -37,7 +37,7 @@ final class CreativeInventory{ private array $creative = []; private function __construct(){ - $creativeItems = json_decode(file_get_contents(Path::join(\pocketmine\BEDROCK_DATA_PATH, "creativeitems.json")), true); + $creativeItems = json_decode(Filesystem::fileGetContents(Path::join(\pocketmine\BEDROCK_DATA_PATH, "creativeitems.json")), true); foreach($creativeItems as $data){ $item = Item::jsonDeserialize($data); diff --git a/src/item/LegacyStringToItemParser.php b/src/item/LegacyStringToItemParser.php index 9ec97ede8..1137674a3 100644 --- a/src/item/LegacyStringToItemParser.php +++ b/src/item/LegacyStringToItemParser.php @@ -24,11 +24,10 @@ declare(strict_types=1); namespace pocketmine\item; use pocketmine\utils\AssumptionFailedError; +use pocketmine\utils\Filesystem; use pocketmine\utils\SingletonTrait; -use pocketmine\utils\Utils; use Symfony\Component\Filesystem\Path; use function explode; -use function file_get_contents; use function is_array; use function is_int; use function is_numeric; @@ -53,7 +52,7 @@ final class LegacyStringToItemParser{ private static function make() : self{ $result = new self(ItemFactory::getInstance()); - $mappingsRaw = Utils::assumeNotFalse(@file_get_contents(Path::join(\pocketmine\RESOURCE_PATH, 'item_from_string_bc_map.json')), "Missing required resource file"); + $mappingsRaw = Filesystem::fileGetContents(Path::join(\pocketmine\RESOURCE_PATH, 'item_from_string_bc_map.json')); $mappings = json_decode($mappingsRaw, true); if(!is_array($mappings)) throw new AssumptionFailedError("Invalid mappings format, expected array"); diff --git a/src/network/mcpe/cache/StaticPacketCache.php b/src/network/mcpe/cache/StaticPacketCache.php index c3eca501d..4cf878fc3 100644 --- a/src/network/mcpe/cache/StaticPacketCache.php +++ b/src/network/mcpe/cache/StaticPacketCache.php @@ -27,9 +27,9 @@ use pocketmine\network\mcpe\protocol\AvailableActorIdentifiersPacket; use pocketmine\network\mcpe\protocol\BiomeDefinitionListPacket; use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer; use pocketmine\network\mcpe\protocol\types\CacheableNbt; +use pocketmine\utils\Filesystem; use pocketmine\utils\SingletonTrait; use Symfony\Component\Filesystem\Path; -use function file_get_contents; class StaticPacketCache{ use SingletonTrait; @@ -38,9 +38,7 @@ class StaticPacketCache{ * @phpstan-return CacheableNbt<\pocketmine\nbt\tag\CompoundTag> */ private static function loadCompoundFromFile(string $filePath) : CacheableNbt{ - $rawNbt = @file_get_contents($filePath); - if($rawNbt === false) throw new \RuntimeException("Failed to read file"); - return new CacheableNbt((new NetworkNbtSerializer())->read($rawNbt)->mustGetCompoundTag()); + return new CacheableNbt((new NetworkNbtSerializer())->read(Filesystem::fileGetContents($filePath))->mustGetCompoundTag()); } private static function make() : self{ diff --git a/src/network/mcpe/convert/GlobalItemTypeDictionary.php b/src/network/mcpe/convert/GlobalItemTypeDictionary.php index f33683346..0795d2fde 100644 --- a/src/network/mcpe/convert/GlobalItemTypeDictionary.php +++ b/src/network/mcpe/convert/GlobalItemTypeDictionary.php @@ -26,10 +26,9 @@ namespace pocketmine\network\mcpe\convert; use pocketmine\network\mcpe\protocol\serializer\ItemTypeDictionary; use pocketmine\network\mcpe\protocol\types\ItemTypeEntry; use pocketmine\utils\AssumptionFailedError; +use pocketmine\utils\Filesystem; use pocketmine\utils\SingletonTrait; -use pocketmine\utils\Utils; use Symfony\Component\Filesystem\Path; -use function file_get_contents; use function is_array; use function is_bool; use function is_int; @@ -40,7 +39,7 @@ final class GlobalItemTypeDictionary{ use SingletonTrait; private static function make() : self{ - $data = Utils::assumeNotFalse(file_get_contents(Path::join(\pocketmine\BEDROCK_DATA_PATH, 'required_item_list.json')), "Missing required resource file"); + $data = Filesystem::fileGetContents(Path::join(\pocketmine\BEDROCK_DATA_PATH, 'required_item_list.json')); $table = json_decode($data, true); if(!is_array($table)){ throw new AssumptionFailedError("Invalid item list format"); diff --git a/src/network/mcpe/convert/ItemTranslator.php b/src/network/mcpe/convert/ItemTranslator.php index ba13fa784..375199601 100644 --- a/src/network/mcpe/convert/ItemTranslator.php +++ b/src/network/mcpe/convert/ItemTranslator.php @@ -26,11 +26,11 @@ namespace pocketmine\network\mcpe\convert; use pocketmine\data\bedrock\LegacyItemIdToStringIdMap; use pocketmine\network\mcpe\protocol\serializer\ItemTypeDictionary; use pocketmine\utils\AssumptionFailedError; +use pocketmine\utils\Filesystem; use pocketmine\utils\SingletonTrait; use pocketmine\utils\Utils; use Symfony\Component\Filesystem\Path; use function array_key_exists; -use function file_get_contents; use function is_array; use function is_numeric; use function is_string; @@ -67,7 +67,7 @@ final class ItemTranslator{ private array $complexNetToCoreMapping = []; private static function make() : self{ - $data = Utils::assumeNotFalse(file_get_contents(Path::join(\pocketmine\BEDROCK_DATA_PATH, 'r16_to_current_item_map.json')), "Missing required resource file"); + $data = Filesystem::fileGetContents(Path::join(\pocketmine\BEDROCK_DATA_PATH, 'r16_to_current_item_map.json')); $json = json_decode($data, true); if(!is_array($json) || !isset($json["simple"], $json["complex"]) || !is_array($json["simple"]) || !is_array($json["complex"])){ throw new AssumptionFailedError("Invalid item table format"); diff --git a/src/network/mcpe/convert/RuntimeBlockMapping.php b/src/network/mcpe/convert/RuntimeBlockMapping.php index 547bd0d81..53b146be6 100644 --- a/src/network/mcpe/convert/RuntimeBlockMapping.php +++ b/src/network/mcpe/convert/RuntimeBlockMapping.php @@ -30,10 +30,9 @@ use pocketmine\nbt\tag\CompoundTag; use pocketmine\network\mcpe\protocol\serializer\NetworkNbtSerializer; use pocketmine\network\mcpe\protocol\serializer\PacketSerializer; use pocketmine\network\mcpe\protocol\serializer\PacketSerializerContext; +use pocketmine\utils\Filesystem; use pocketmine\utils\SingletonTrait; -use pocketmine\utils\Utils; use Symfony\Component\Filesystem\Path; -use function file_get_contents; /** * @internal @@ -57,7 +56,7 @@ final class RuntimeBlockMapping{ public function __construct(string $canonicalBlockStatesFile, string $r12ToCurrentBlockMapFile){ $stream = PacketSerializer::decoder( - Utils::assumeNotFalse(file_get_contents($canonicalBlockStatesFile), "Missing required resource file"), + Filesystem::fileGetContents($canonicalBlockStatesFile), 0, new PacketSerializerContext(GlobalItemTypeDictionary::getInstance()->getDictionary()) ); @@ -75,7 +74,7 @@ final class RuntimeBlockMapping{ /** @var R12ToCurrentBlockMapEntry[] $legacyStateMap */ $legacyStateMap = []; $legacyStateMapReader = PacketSerializer::decoder( - Utils::assumeNotFalse(file_get_contents($r12ToCurrentBlockMapFile), "Missing required resource file"), + Filesystem::fileGetContents($r12ToCurrentBlockMapFile), 0, new PacketSerializerContext(GlobalItemTypeDictionary::getInstance()->getDictionary()) ); diff --git a/src/player/DatFilePlayerDataProvider.php b/src/player/DatFilePlayerDataProvider.php index 53388dceb..13b5ea423 100644 --- a/src/player/DatFilePlayerDataProvider.php +++ b/src/player/DatFilePlayerDataProvider.php @@ -32,7 +32,6 @@ use pocketmine\utils\Filesystem; use pocketmine\utils\Utils; use Symfony\Component\Filesystem\Path; use function file_exists; -use function file_get_contents; use function rename; use function strtolower; use function zlib_decode; @@ -70,8 +69,8 @@ final class DatFilePlayerDataProvider implements PlayerDataProvider{ } try{ - $contents = ErrorToExceptionHandler::trapAndRemoveFalse(fn() => file_get_contents($path)); - }catch(\ErrorException $e){ + $contents = Filesystem::fileGetContents($path); + }catch(\RuntimeException $e){ throw new PlayerDataLoadException("Failed to read player data file \"$path\": " . $e->getMessage(), 0, $e); } try{ diff --git a/src/resourcepacks/ResourcePackManager.php b/src/resourcepacks/ResourcePackManager.php index 655f3a3a9..6429ac06b 100644 --- a/src/resourcepacks/ResourcePackManager.php +++ b/src/resourcepacks/ResourcePackManager.php @@ -23,14 +23,13 @@ declare(strict_types=1); namespace pocketmine\resourcepacks; -use pocketmine\errorhandler\ErrorToExceptionHandler; use pocketmine\utils\Config; +use pocketmine\utils\Filesystem; use Symfony\Component\Filesystem\Path; use function array_keys; use function copy; use function count; use function file_exists; -use function file_get_contents; use function gettype; use function is_array; use function is_dir; @@ -104,10 +103,8 @@ class ResourcePackManager{ $keyPath = Path::join($this->path, $pack . ".key"); if(file_exists($keyPath)){ try{ - $key = ErrorToExceptionHandler::trapAndRemoveFalse( - fn() => file_get_contents($keyPath) - ); - }catch(\ErrorException $e){ + $key = Filesystem::fileGetContents($keyPath); + }catch(\RuntimeException $e){ throw new ResourcePackException("Could not read encryption key file: " . $e->getMessage(), 0, $e); } $key = rtrim($key, "\r\n"); diff --git a/src/utils/Config.php b/src/utils/Config.php index 617feeacf..670d6bda3 100644 --- a/src/utils/Config.php +++ b/src/utils/Config.php @@ -33,7 +33,6 @@ use function count; use function date; use function explode; use function file_exists; -use function file_get_contents; use function get_debug_type; use function implode; use function is_array; @@ -162,10 +161,7 @@ class Config{ $this->config = $default; $this->save(); }else{ - $content = file_get_contents($this->file); - if($content === false){ - throw new \RuntimeException("Unable to load config file"); - } + $content = Filesystem::fileGetContents($this->file); switch($this->type){ case Config::PROPERTIES: $config = self::parseProperties($content); diff --git a/src/utils/Filesystem.php b/src/utils/Filesystem.php index 0a183d4cd..71de8bc34 100644 --- a/src/utils/Filesystem.php +++ b/src/utils/Filesystem.php @@ -30,6 +30,7 @@ use function dirname; use function fclose; use function fflush; use function file_exists; +use function file_get_contents; use function file_put_contents; use function flock; use function fopen; @@ -296,4 +297,21 @@ final class Filesystem{ @unlink($temporaryFileName); } } + + /** + * Wrapper around file_get_contents() which throws an exception instead of generating E_* errors. + * + * @phpstan-param resource|null $context + * @phpstan-param 0|positive-int $offset + * @phpstan-param 0|positive-int|null $length + * + * @throws \RuntimeException + */ + public static function fileGetContents(string $fileName, bool $useIncludePath = false, $context = null, int $offset = 0, ?int $length = null) : string{ + try{ + return ErrorToExceptionHandler::trapAndRemoveFalse(fn() => file_get_contents($fileName, $useIncludePath, $context, $offset, $length)); + }catch(\ErrorException $e){ + throw new \RuntimeException("Failed to read file $fileName: " . $e->getMessage(), 0, $e); + } + } } diff --git a/src/world/format/io/data/BedrockWorldData.php b/src/world/format/io/data/BedrockWorldData.php index 995f64898..d6f1b2e10 100644 --- a/src/world/format/io/data/BedrockWorldData.php +++ b/src/world/format/io/data/BedrockWorldData.php @@ -39,7 +39,6 @@ use pocketmine\world\generator\GeneratorManager; use pocketmine\world\World; use pocketmine\world\WorldCreationOptions; use Symfony\Component\Filesystem\Path; -use function file_get_contents; use function file_put_contents; use function strlen; use function substr; @@ -111,9 +110,10 @@ class BedrockWorldData extends BaseNbtWorldData{ } protected function load() : CompoundTag{ - $rawLevelData = @file_get_contents($this->dataPath); - if($rawLevelData === false){ - throw new CorruptedWorldException("Failed to read level.dat (permission denied or doesn't exist)"); + try{ + $rawLevelData = Filesystem::fileGetContents($this->dataPath); + }catch(\RuntimeException $e){ + throw new CorruptedWorldException($e->getMessage(), 0, $e); } if(strlen($rawLevelData) <= 8){ throw new CorruptedWorldException("Truncated level.dat"); diff --git a/src/world/format/io/data/JavaWorldData.php b/src/world/format/io/data/JavaWorldData.php index e53d857ad..377480c9a 100644 --- a/src/world/format/io/data/JavaWorldData.php +++ b/src/world/format/io/data/JavaWorldData.php @@ -37,7 +37,6 @@ use pocketmine\world\World; use pocketmine\world\WorldCreationOptions; use Symfony\Component\Filesystem\Path; use function ceil; -use function file_get_contents; use function file_put_contents; use function microtime; use function zlib_decode; @@ -75,9 +74,10 @@ class JavaWorldData extends BaseNbtWorldData{ } protected function load() : CompoundTag{ - $rawLevelData = @file_get_contents($this->dataPath); - if($rawLevelData === false){ - throw new CorruptedWorldException("Failed to read level.dat (permission denied or doesn't exist)"); + try{ + $rawLevelData = Filesystem::fileGetContents($this->dataPath); + }catch(\RuntimeException $e){ + throw new CorruptedWorldException($e->getMessage(), 0, $e); } $nbt = new BigEndianNbtSerializer(); $decompressed = @zlib_decode($rawLevelData);