From 0fdbdfb5a8601f21740e89f0294d31f845cb6ebb Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Sun, 15 Oct 2023 10:16:30 -0700 Subject: [PATCH] re-add permission handling during PPCE to get around spigot behavior. --- .../java/ch/njol/skript/command/Commands.java | 68 ++++++++++++------- .../ch/njol/skript/command/ScriptCommand.java | 40 +++++++---- 2 files changed, 68 insertions(+), 40 deletions(-) diff --git a/src/main/java/ch/njol/skript/command/Commands.java b/src/main/java/ch/njol/skript/command/Commands.java index 35e12057bb1..e6cb7456be5 100644 --- a/src/main/java/ch/njol/skript/command/Commands.java +++ b/src/main/java/ch/njol/skript/command/Commands.java @@ -41,6 +41,7 @@ import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; import org.bukkit.event.player.AsyncPlayerChatEvent; +import org.bukkit.event.player.PlayerCommandPreprocessEvent; import org.bukkit.event.server.ServerCommandEvent; import org.bukkit.help.HelpMap; import org.bukkit.help.HelpTopic; @@ -68,7 +69,7 @@ */ @SuppressWarnings("deprecation") public abstract class Commands { - + public final static ArgsMessage m_too_many_arguments = new ArgsMessage("commands.too many arguments"); public final static Message m_internal_error = new Message("commands.internal error"); public final static Message m_correct_usage = new Message("commands.correct usage"); @@ -79,26 +80,26 @@ public abstract class Commands { public static final int CONVERTER_NO_COMMAND_ARGUMENTS = 4; private final static Map commands = new HashMap<>(); - + @Nullable private static SimpleCommandMap commandMap = null; @Nullable private static Map cmKnownCommands; @Nullable private static Set cmAliases; - + static { init(); // separate method for the annotation } public static Set getScriptCommands(){ return commands.keySet(); } - + @Nullable public static SimpleCommandMap getCommandMap(){ return commandMap; } - + @SuppressWarnings("unchecked") private static void init() { try { @@ -106,11 +107,11 @@ private static void init() { Field commandMapField = SimplePluginManager.class.getDeclaredField("commandMap"); commandMapField.setAccessible(true); commandMap = (SimpleCommandMap) commandMapField.get(Bukkit.getPluginManager()); - + Field knownCommandsField = SimpleCommandMap.class.getDeclaredField("knownCommands"); knownCommandsField.setAccessible(true); cmKnownCommands = (Map) knownCommandsField.get(commandMap); - + try { Field aliasesField = SimpleCommandMap.class.getDeclaredField("aliases"); aliasesField.setAccessible(true); @@ -125,25 +126,42 @@ private static void init() { commandMap = null; } } - + @Nullable public static List> currentArguments = null; - + @SuppressWarnings("null") private final static Pattern escape = Pattern.compile("[" + Pattern.quote("(|)<>%\\") + "]"); @SuppressWarnings("null") private final static Pattern unescape = Pattern.compile("\\\\[" + Pattern.quote("(|)<>%\\") + "]"); - + public static String escape(String string) { return "" + escape.matcher(string).replaceAll("\\\\$0"); } - + public static String unescape(String string) { return "" + unescape.matcher(string).replaceAll("$0"); } - + private final static Listener commandListener = new Listener() { - @SuppressWarnings("null") + + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) + public void onPlayerCommand(PlayerCommandPreprocessEvent event) { + // Spigot will simply report that the command doesn't exist if a player does not have permission to use it. + // This is good security but, well, it's a breaking change for Skript. So we need to check for permissions + // ourselves and handle those messages, for every command. + + // parse command, see if it's a skript command + String[] cmd = event.getMessage().substring(1).split("\\s+", 2); + String label = cmd[0].toLowerCase(Locale.ENGLISH); + String arguments = cmd.length == 1 ? "" : "" + cmd[1]; + ScriptCommand command = commands.get(label); + + // if so, check permissions + if (command != null && !command.checkPermissions(event.getPlayer(), label, arguments)) + event.setCancelled(true); + } + @EventHandler(priority = EventPriority.HIGHEST) public void onServerCommand(ServerCommandEvent event) { if (event.getCommand().isEmpty() || event.isCancelled()) @@ -154,7 +172,7 @@ public void onServerCommand(ServerCommandEvent event) { } } }; - + static boolean handleEffectCommand(CommandSender sender, String command) { if (!(sender instanceof ConsoleCommandSender || sender.hasPermission("skript.effectcommands") || SkriptConfig.allowOpsToUseEffectCommands.value() && sender.isOp())) return false; @@ -170,7 +188,7 @@ static boolean handleEffectCommand(CommandSender sender, String command) { parserInstance.setCurrentEvent("effect command", EffectCommandEvent.class); Effect effect = Effect.parse(command, null); parserInstance.deleteCurrentEvent(); - + if (effect != null) { log.clear(); // ignore warnings and stuff log.printLog(); @@ -219,7 +237,7 @@ public static boolean scriptCommandExists(String command) { ScriptCommand scriptCommand = commands.get(command); return scriptCommand != null && scriptCommand.getName().equals(command); } - + public static void registerCommand(ScriptCommand command) { // Validate that there are no duplicates ScriptCommand existingCommand = commands.get(command.getLabel()); @@ -230,7 +248,7 @@ public static void registerCommand(ScriptCommand command) { ); return; } - + if (commandMap != null) { assert cmKnownCommands != null;// && cmAliases != null; command.register(commandMap, cmKnownCommands, cmAliases); @@ -262,9 +280,9 @@ public static void unregisterCommand(ScriptCommand scriptCommand) { } commands.values().removeIf(command -> command == scriptCommand); } - + private static boolean registeredListeners = false; - + public static void registerListeners() { if (!registeredListeners) { Bukkit.getPluginManager().registerEvents(commandListener, Skript.getInstance()); @@ -298,15 +316,15 @@ public void onPlayerChat(AsyncPlayerChatEvent event) { registeredListeners = true; } } - + /** * copied from CraftBukkit (org.bukkit.craftbukkit.help.CommandAliasHelpTopic) */ public static final class CommandAliasHelpTopic extends HelpTopic { - + private final String aliasFor; private final HelpMap helpMap; - + public CommandAliasHelpTopic(String alias, String aliasFor, HelpMap helpMap) { this.aliasFor = aliasFor.startsWith("/") ? aliasFor : "/" + aliasFor; this.helpMap = helpMap; @@ -314,7 +332,7 @@ public CommandAliasHelpTopic(String alias, String aliasFor, HelpMap helpMap) { Validate.isTrue(!name.equals(this.aliasFor), "Command " + name + " cannot be alias for itself"); shortText = ChatColor.YELLOW + "Alias for " + ChatColor.WHITE + this.aliasFor; } - + @Override @NotNull public String getFullText(CommandSender forWho) { @@ -326,7 +344,7 @@ public String getFullText(CommandSender forWho) { } return "" + fullText; } - + @Override public boolean canSee(CommandSender commandSender) { if (amendedPermission != null) @@ -335,5 +353,5 @@ public boolean canSee(CommandSender commandSender) { return aliasForTopic != null && aliasForTopic.canSee(commandSender); } } - + } diff --git a/src/main/java/ch/njol/skript/command/ScriptCommand.java b/src/main/java/ch/njol/skript/command/ScriptCommand.java index 2ce33ac7152..05d01680cc3 100644 --- a/src/main/java/ch/njol/skript/command/ScriptCommand.java +++ b/src/main/java/ch/njol/skript/command/ScriptCommand.java @@ -117,7 +117,7 @@ public class ScriptCommand implements TabExecutor { /** * Creates a new SkriptCommand. - * + * * @param name /name * @param pattern * @param arguments the list of Arguments this command takes @@ -237,16 +237,8 @@ public boolean execute(final CommandSender sender, final String commandLabel, fi final ScriptCommandEvent event = new ScriptCommandEvent(ScriptCommand.this, sender, commandLabel, rest); - if (!permission.isEmpty() && !sender.hasPermission(permission)) { - if (sender instanceof Player) { - List components = - permissionMessage.getMessageComponents(event); - ((Player) sender).spigot().sendMessage(BungeeConverter.convert(components)); - } else { - sender.sendMessage(permissionMessage.getSingle(event)); - } + if (!checkPermissions(sender, event)) return false; - } cooldownCheck : { if (sender instanceof Player && cooldown != null) { @@ -316,19 +308,37 @@ boolean execute2(final ScriptCommandEvent event, final CommandSender sender, fin } finally { log.stop(); } - + if (Skript.log(Verbosity.VERY_HIGH)) Skript.info("# /" + name + " " + rest); final long startTrigger = System.nanoTime(); - + if (!trigger.execute(event)) sender.sendMessage(Commands.m_internal_error.toString()); - + if (Skript.log(Verbosity.VERY_HIGH)) Skript.info("# " + name + " took " + 1. * (System.nanoTime() - startTrigger) / 1000000. + " milliseconds"); return true; } + public boolean checkPermissions(CommandSender sender, String commandLabel, String arguments) { + return checkPermissions(sender, new ScriptCommandEvent(this, sender, commandLabel, arguments)); + } + + public boolean checkPermissions(CommandSender sender, Event event) { + if (!permission.isEmpty() && !sender.hasPermission(permission)) { + if (sender instanceof Player) { + List components = + permissionMessage.getMessageComponents(event); + ((Player) sender).spigot().sendMessage(BungeeConverter.convert(components)); + } else { + sender.sendMessage(permissionMessage.getSingle(event)); + } + return false; + } + return true; + } + public void sendHelp(final CommandSender sender) { if (!description.isEmpty()) sender.sendMessage(description); @@ -337,7 +347,7 @@ public void sendHelp(final CommandSender sender) { /** * Gets the arguments this command takes. - * + * * @return The internal list of arguments. Do not modify it! */ public List> getArguments() { @@ -575,7 +585,7 @@ public List onTabComplete(@Nullable CommandSender sender, @Nullable Comm Class argType = arg.getType(); if (argType.equals(Player.class) || argType.equals(OfflinePlayer.class)) return null; // Default completion - + return Collections.emptyList(); // No tab completion here! }