Skip to content

Firebombing of commands for the first time in 10 years #6837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Parent: Rename `PlayerExhaustEvent` to `EntityExhaustEvent`
wants to merge 8 commits into
base: major-next
Choose a base branch
from
Draft

Firebombing of commands for the first time in 10 years #6837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
50976c2
Firebombing of commands for the first time in 10 years
dktapps Oct 12, 2025
a314d2d
Fixed trailing literals not being required
dktapps Oct 12, 2025
d0fca5e
Fixed bad assumption about literals and subsequent whitespace
dktapps Oct 12, 2025
1ef5e04
Don't scan the entire command string looking for literals
dktapps Oct 12, 2025
4a3f528
Add basic client-side hint support
dktapps Oct 12, 2025
371b991
Clean up
dktapps Oct 12, 2025
835faa4
Update comment about suffixed parameters
dktapps Oct 13, 2025
37632bc
didn't realise I committed this
dktapps Oct 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion src/command/ClosureCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
/**
* @phpstan-type Execute \Closure(CommandSender $sender, Command $command, string $commandLabel, list<string> $args) : mixed
*/
final class ClosureCommand extends Command{
final class ClosureCommand extends LegacyCommand{
/** @phpstan-var Execute */
private \Closure $execute;

/**
* @param string[] $permissions
* @phpstan-param Execute $execute
* @phpstan-param list<string> $permissions
*/
public function __construct(
string $namespace,
Expand Down
166 changes: 99 additions & 67 deletions src/command/Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,40 @@
*/
namespace pocketmine\command;

use pocketmine\command\utils\CommandException;
use pocketmine\command\overload\CommandOverload;
use pocketmine\command\utils\InvalidCommandSyntaxException;
use pocketmine\lang\KnownTranslationFactory;
use pocketmine\lang\Translatable;
use pocketmine\permission\PermissionManager;
use pocketmine\player\Player;
use pocketmine\Server;
use pocketmine\utils\BroadcastLoggerForwarder;
use pocketmine\utils\TextFormat;
use function explode;
use function array_unique;
use function count;
use function implode;
use function str_replace;
use function strtolower;
use function trim;
use const PHP_INT_MAX;

abstract class Command{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should Command be non-abstract now? There are no abstract functions, and there doesn't seem to be a good reason why we need to force users to create a separate class for each command.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Maybe this also makes ClosureCommand obsolete too.

private readonly string $namespace;
private readonly string $name;

/** @var string[] */
private array $permission = [];
private Translatable|string|null $permissionMessage = null;

/**
* @param CommandOverload[] $overloads
* @phpstan-param list<CommandOverload> $overloads
*/
public function __construct(
string $namespace,
string $name,
private array $overloads,
private Translatable|string $description = "",
private Translatable|string|null $usageMessage = null
){
if(count($this->overloads) === 0){
throw new \InvalidArgumentException("At least one overload must be provided (extend LegacyCommand for classic execute())");
}
if($namespace === ""){
throw new \InvalidArgumentException("Command namespace cannot be empty (set it to, for example, your plugin's name)");
}
Expand All @@ -65,14 +71,63 @@ public function __construct(
$this->name = trim($name);
}

final public function executeOverloaded(CommandSender $sender, string $aliasUsed, string $rawArgs) : bool{
foreach($this->overloads as $k => $overload){
if(!$overload->senderHasAnyPermissions($sender)){
continue;
}
try{
$overload->invoke($sender, $aliasUsed, $rawArgs);
return true;
}catch(InvalidCommandSyntaxException $e){
\GlobalLogger::get()->debug("Overload $k of /$aliasUsed rejected: " . $e->getMessage());
}
}

$usages = $this->getUsages($sender, $aliasUsed);
if(count($usages) === 0){
$message = $this->permissionMessage ?? KnownTranslationFactory::pocketmine_command_error_permission($aliasUsed);
if($message instanceof Translatable){
$sender->sendMessage($message->prefix(TextFormat::RED));
}elseif($message !== ""){
$permissions = [];
foreach($this->overloads as $overload){
foreach($overload->getPermissions() as $permission){
$permissions[] = $permission;
}
}
$permissions = array_unique($permissions);
$sender->sendMessage(str_replace("<permission>", implode(";", $permissions), $message));
}
return false;
}

foreach($usages as $usageMessage){
$sender->sendMessage($sender->getLanguage()->translate(KnownTranslationFactory::commands_generic_usage($usageMessage)));
}
return false;
}

/**
* @return CommandOverload[]
* @phpstan-return list<CommandOverload>
*/
public function getOverloads() : array{ return $this->overloads; }

/**
* @param string[] $args
* @phpstan-param list<string> $args
*
* @return mixed
* @throws CommandException
* @return Translatable[]
* @phpstan-return list<Translatable>
*/
abstract public function execute(CommandSender $sender, string $commandLabel, array $args);
public function getUsages(CommandSender $sender, string $aliasUsed) : array{
$usages = [];
foreach($this->overloads as $overload){
if($overload->senderHasAnyPermissions($sender)){
$usages[] = new Translatable("/$aliasUsed {%0}", [$overload->getUsage()]);
}
}

return $usages;
}

final public function getNamespace() : string{
return $this->namespace;
Expand All @@ -93,59 +148,17 @@ final public function getId() : string{
return "$this->namespace:$this->name";
}

/**
* @return string[]
*/
public function getPermissions() : array{
return $this->permission;
}

/**
* @param string[] $permissions
* @phpstan-param list<string> $permissions
*/
public function setPermissions(array $permissions) : void{
$permissionManager = PermissionManager::getInstance();
foreach($permissions as $perm){
if($permissionManager->getPermission($perm) === null){
throw new \InvalidArgumentException("Cannot use non-existing permission \"$perm\"");
}
}
$this->permission = $permissions;
}

public function setPermission(?string $permission) : void{
$this->setPermissions($permission === null ? [] : explode(";", $permission, limit: PHP_INT_MAX));
}

/**
* @param string $context usually the command name, but may include extra args if useful (e.g. for subcommands)
* @param CommandSender $target the target to check the permission for
* @param string|null $permission the permission to check, if null, will check if the target has any of the command's permissions
*/
public function testPermission(string $context, CommandSender $target, ?string $permission = null) : bool{
if($this->testPermissionSilent($target, $permission)){
return true;
}

protected function sendBadPermissionMessage(string $context, CommandSender $sender, array $permissions) : void{
$message = $this->permissionMessage ?? KnownTranslationFactory::pocketmine_command_error_permission($context);
if($message instanceof Translatable){
$target->sendMessage($message->prefix(TextFormat::RED));
$sender->sendMessage($message->prefix(TextFormat::RED));
}elseif($message !== ""){
$target->sendMessage(str_replace("<permission>", $permission ?? implode(";", $this->permission), $message));
}

return false;
}

public function testPermissionSilent(CommandSender $target, ?string $permission = null) : bool{
$list = $permission !== null ? [$permission] : $this->permission;
foreach($list as $p){
if($target->hasPermission($p)){
return true;
}
$sender->sendMessage(str_replace("<permission>", implode(";", $permissions), $message));
}

return false;
}

public function getPermissionMessage() : Translatable|string|null{
Expand All @@ -156,10 +169,6 @@ public function getDescription() : Translatable|string{
return $this->description;
}

public function getUsage() : Translatable|string|null{
return $this->usageMessage;
}

public function setDescription(Translatable|string $description) : void{
$this->description = $description;
}
Expand All @@ -168,10 +177,6 @@ public function setPermissionMessage(Translatable|string $permissionMessage) : v
$this->permissionMessage = $permissionMessage;
}

public function setUsage(Translatable|string|null $usage) : void{
$this->usageMessage = $usage;
}

public static function broadcastCommandMessage(CommandSender $source, Translatable|string $message, bool $sendToSource = true) : void{
$users = $source->getServer()->getBroadcastChannelSubscribers(Server::BROADCAST_CHANNEL_ADMINISTRATIVE);
$result = KnownTranslationFactory::chat_type_admin($source->getName(), $message);
Expand All @@ -189,4 +194,31 @@ public static function broadcastCommandMessage(CommandSender $source, Translatab
}
}
}

protected static function fetchPermittedPlayerTarget(
CommandSender $sender,
?string $target,
string $selfPermission,
string $otherPermission
) : ?Player{
if($target !== null){
$player = $sender->getServer()->getPlayerByPrefix($target);
}elseif($sender instanceof Player){
$player = $sender;
}else{
throw new InvalidCommandSyntaxException();
}

if($player === null){
$sender->sendMessage(KnownTranslationFactory::commands_generic_player_notFound()->prefix(TextFormat::RED));
return null;
}

$permission = $player === $sender ? $selfPermission : $otherPermission;
if(!$sender->hasPermission($permission)){
$sender->sendMessage(TextFormat::RED . "You don't have permission to use this command on others");
return null;
}
return $player;
}
}
48 changes: 13 additions & 35 deletions src/command/FormattedCommandAlias.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,18 @@
namespace pocketmine\command;

use pocketmine\command\utils\CommandStringHelper;
use pocketmine\command\utils\InvalidCommandSyntaxException;
use pocketmine\lang\KnownTranslationFactory;
use pocketmine\timings\Timings;
use pocketmine\utils\AssumptionFailedError;
use pocketmine\utils\TextFormat;
use function array_shift;
use function addcslashes;
use function count;
use function implode;
use function preg_match;
use function str_contains;
use function strlen;
use function strpos;
use function substr;

class FormattedCommandAlias extends Command{
class FormattedCommandAlias extends LegacyCommand{
/**
* - matches a $
* - captures an optional second $ to indicate required/optional
Expand Down Expand Up @@ -77,46 +75,21 @@ public function execute(CommandSender $sender, string $commandLabel, array $args
$processedArgs[] = $processedArg;
}
}
$commands[] = $processedArgs;
$commands[] = implode(" ", $processedArgs);
}catch(\InvalidArgumentException $e){
$sender->sendMessage(TextFormat::RED . $e->getMessage());
return false;
}
}

$commandMap = $sender->getServer()->getCommandMap();
foreach($commands as $commandArgs){
//this approximately duplicates the logic found in SimpleCommandMap::dispatch()
//this is to allow directly invoking the commands without having to rebuild a command string and parse it
//again for no reason
//TODO: a method on CommandMap to invoke a command with pre-parsed arguments would probably be a good idea
//for a future major version
$commandLabel = array_shift($commandArgs);
if($commandLabel === null){
throw new AssumptionFailedError("This should have been checked before construction");
}

//formatted command aliases don't use user-specific aliases since they are globally defined in pocketmine.yml
//using user-specific aliases might break the behaviour
if(($target = $commandMap->getCommand($commandLabel)) instanceof Command){

$timings = Timings::getCommandDispatchTimings($target->getId());
$timings->startTiming();

try{
$target->execute($sender, $commandLabel, $commandArgs);
}catch(InvalidCommandSyntaxException $e){
$sender->sendMessage($sender->getLanguage()->translate(KnownTranslationFactory::commands_generic_usage($target->getUsage() ?? "/$commandLabel")));
}finally{
$timings->stopTiming();
}
}else{
//TODO: this seems suspicious - why do we continue alias execution if one of the commands is borked?
$sender->sendMessage($sender->getLanguage()->translate(KnownTranslationFactory::pocketmine_command_notFound($commandLabel, "/help")->prefix(TextFormat::RED)));

foreach($commands as $commandLine){
$sender->getServer()->getLogger()->debug("Dispatching formatted command: $commandLine");
if(!$commandMap->dispatch($sender, $commandLine)){
//to match the behaviour of SimpleCommandMap::dispatch()
//this shouldn't normally happen, but might happen if the command was unregistered or modified after
//the alias was installed
//TODO: maybe we should abort command processing if there was an error???
$result = false;
}
}
Expand Down Expand Up @@ -160,6 +133,11 @@ private function buildCommand(string $formatString, array $args) : ?string{
$index = $start + strlen($replacement);
}

//we need to assemble a command string to call the target commands, so this needs to be properly quoted
if(str_contains($formatString, " ")){
return '"' . addcslashes($formatString, '"') . '"';
}

return $formatString;
}

Expand Down
Loading