From ea5b249197efcba63bfbcb0e01abee635b24ad52 Mon Sep 17 00:00:00 2001 From: asofold Date: Wed, 17 Dec 2014 18:33:16 +0100 Subject: [PATCH] Alter how command lists are interpreted. Might fix some issues. Details: * Only trim commands from the left side (both on feed and check). * Ensure versions with and without leading '/' are fed into trees. * Move methods to feed CharPrefixTree from configs to CommandUtil. Potentially fixes (untested): * Only deny the sub-commands for a prefix, example: feed 'version ' to consoleonly, in order to allow 'version' but not 'version NoCheatplus'. * Might fix some plugin/server specific prefixes not being detected, example: feed "/version" and expect "/bukkit:version" to be blocked. --- .../nocheatplus/utilities/StringUtil.java | 39 +++- .../nocheatplus/test/TestStringUtil.java | 38 +++- .../nocheatplus/checks/chat/ChatListener.java | 213 ++++++++---------- .../nocheatplus/command/CommandUtil.java | 54 +++++ 4 files changed, 214 insertions(+), 130 deletions(-) diff --git a/NCPCommons/src/main/java/fr/neatmonster/nocheatplus/utilities/StringUtil.java b/NCPCommons/src/main/java/fr/neatmonster/nocheatplus/utilities/StringUtil.java index a4e42642..822c9e90 100644 --- a/NCPCommons/src/main/java/fr/neatmonster/nocheatplus/utilities/StringUtil.java +++ b/NCPCommons/src/main/java/fr/neatmonster/nocheatplus/utilities/StringUtil.java @@ -164,7 +164,7 @@ public class StringUtil { return p[n]; } - + /** * Just return the stack trace as new-line-separated string. * @param t @@ -214,7 +214,7 @@ public class StringUtil { } return b.toString(); } - + /** * Convenience method for stackTraceToString(t). * @param t @@ -223,7 +223,7 @@ public class StringUtil { public static final String throwableToString(final Throwable t) { return stackTraceToString(t, true, true); } - + /** * Count number of needles left in a dart board. * @param dartBoard @@ -243,4 +243,37 @@ public class StringUtil { return n; } + /** + * Get a version of a String with all leading whitespace removed. + * + * @param input + * @return String with leading whitespace removed. Returns the original + * reference, if there is no leading whitespace. + */ + public static final String leftTrim(final String input) { + if (input == null) { + return null; + } + final int len = input.length(); + int beginIndex = 0; + for (int i = 0; i < len; i++) { + if (Character.isWhitespace(input.charAt(i))) { + ++beginIndex; + } + else { + break; + } + } + if (beginIndex > 0) { + if (beginIndex >= len) { + return ""; + } else { + return input.substring(beginIndex); + } + } + else { + return input; + } + } + } diff --git a/NCPCommons/src/test/java/fr/neatmonster/nocheatplus/test/TestStringUtil.java b/NCPCommons/src/test/java/fr/neatmonster/nocheatplus/test/TestStringUtil.java index b3ec2498..43f0cbf5 100644 --- a/NCPCommons/src/test/java/fr/neatmonster/nocheatplus/test/TestStringUtil.java +++ b/NCPCommons/src/test/java/fr/neatmonster/nocheatplus/test/TestStringUtil.java @@ -13,14 +13,14 @@ import fr.neatmonster.nocheatplus.utilities.StringUtil; * */ public class TestStringUtil { - + private void assertCount(String data, char searchFor, int num) { int res = StringUtil.count(data, searchFor); if (res != num) { fail("Expect to find '" + searchFor + "' " + num + " times in '" + data + "', got instead: " + res); } } - + @Test public void testCount() { assertCount("" , 'x', 0); @@ -34,7 +34,7 @@ public class TestStringUtil { assertCount("oxx" , 'x', 2); assertCount("230489tuvn1374z1hxk,34htmc1", '3', 3); } - + private void recursiveFail(int now, int max) { now ++; if (now >= max) { @@ -44,7 +44,7 @@ public class TestStringUtil { recursiveFail(now, max); } } - + /** * Indirectly by counting line breaks with StringUtil. * @param recursionDepth @@ -62,7 +62,7 @@ public class TestStringUtil { } } } - + /** * Indirectly by counting line breaks with StringUtil. * @param recursionDepth @@ -80,15 +80,37 @@ public class TestStringUtil { } } } - + @Test public void testStackTraceLinear() { assertMinimumStackTraceLength(1000, 1000, false); } - + @Test public void testStackTraceTrimmed() { assertMaximumStackTraceLength(1000, 50, true); } - + + private void testLeftTrim(String input, String expectedResult) { + String result = StringUtil.leftTrim(input); + if (!expectedResult.equals(result)) { + fail("Expect leftTrim for '" + input + "' to return '" + expectedResult + "', got instead: '" + result + "'."); + } + } + + @Test + public void testLeftTrim() { + if (StringUtil.leftTrim(null) != null) { + fail("Expect leftTrim to return null for null input, got instead: '" + StringUtil.leftTrim(null) + "'."); + } + for (String[] spec : new String[][]{ + {"", ""}, + {" \t", ""}, + {" TEST", "TEST"}, + {"\t\n TEST", "TEST"}, + {" TEST ", "TEST "} + }) { + testLeftTrim(spec[0], spec[1]); + } + } } diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/chat/ChatListener.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/chat/ChatListener.java index 62f1bad8..dabde546 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/chat/ChatListener.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/chat/ChatListener.java @@ -1,7 +1,6 @@ package fr.neatmonster.nocheatplus.checks.chat; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import org.bukkit.command.Command; @@ -22,6 +21,7 @@ import fr.neatmonster.nocheatplus.components.JoinLeaveListener; import fr.neatmonster.nocheatplus.config.ConfPaths; import fr.neatmonster.nocheatplus.config.ConfigFile; import fr.neatmonster.nocheatplus.config.ConfigManager; +import fr.neatmonster.nocheatplus.utilities.StringUtil; import fr.neatmonster.nocheatplus.utilities.TickTask; import fr.neatmonster.nocheatplus.utilities.ds.prefixtree.SimpleCharPrefixTree; @@ -31,81 +31,56 @@ import fr.neatmonster.nocheatplus.utilities.ds.prefixtree.SimpleCharPrefixTree; * @see ChatEvent */ public class ChatListener extends CheckListener implements INotifyReload, JoinLeaveListener { - + // Checks. - + /** Captcha handler. */ private final Captcha captcha = addCheck(new Captcha()); /** The color check. */ private final Color color = addCheck(new Color()); - + /** Commands repetition check. */ private final Commands commands = addCheck(new Commands()); - + /** Logins check (global) */ private final Logins logins = addCheck(new Logins()); - + /** Chat message check. */ private final Text text = addCheck(new Text()); - + /** Relogging check. */ private final Relog relog = addCheck(new Relog()); - + // Auxiliary stuff. - + /** Commands to be ignored completely. */ private final SimpleCharPrefixTree commandExclusions = new SimpleCharPrefixTree(); - + /** Commands to be handled as chat. */ private final SimpleCharPrefixTree chatCommands = new SimpleCharPrefixTree(); - + /** Commands not to be executed in-game. */ private final SimpleCharPrefixTree consoleOnlyCommands = new SimpleCharPrefixTree(); - - public ChatListener(){ - super(CheckType.CHAT); - ConfigFile config = ConfigManager.getConfigFile(); - initFilters(config); - // (text inits in constructor.) + + public ChatListener() { + super(CheckType.CHAT); + ConfigFile config = ConfigManager.getConfigFile(); + initFilters(config); + // (text inits in constructor.) } - - /** - * Clear tree and feed lower case. Add versions with "/" if missing. - * @param tree - * @param inputs - */ - private void feedCommands(SimpleCharPrefixTree tree, Collection inputs){ - tree.clear(); - tree.feedAll(inputs, false, true); - for (String input : inputs){ - if (!input.trim().startsWith("/")){ - tree.feed("/" + input.trim().toLowerCase()); - } - } + + private void initFilters(ConfigFile config) { + CommandUtil.feedCommands(consoleOnlyCommands, config, ConfPaths.PROTECT_COMMANDS_CONSOLEONLY_CMDS, true); + CommandUtil.feedCommands(chatCommands, config, ConfPaths.CHAT_COMMANDS_HANDLEASCHAT, true); + CommandUtil.feedCommands(commandExclusions, config, ConfPaths.CHAT_COMMANDS_EXCLUSIONS, true); } - - /** - * Read string list from config and call feedCommands(tree, list). - * @param tree - * @param config - * @param configPath - */ - private void feedCommands(SimpleCharPrefixTree tree, ConfigFile config, String configPath){ - feedCommands(tree, config.getStringList(configPath)); - } - - private void initFilters(ConfigFile config) { - feedCommands(consoleOnlyCommands, config, ConfPaths.PROTECT_COMMANDS_CONSOLEONLY_CMDS); - feedCommands(chatCommands, config, ConfPaths.CHAT_COMMANDS_HANDLEASCHAT); - feedCommands(commandExclusions, config, ConfPaths.CHAT_COMMANDS_EXCLUSIONS); - } - - @EventHandler(priority=EventPriority.MONITOR) - public void onPlayerChangedWorld(final PlayerChangedWorldEvent event){ - // Tell TickTask to update cached permissions. + + @EventHandler(priority=EventPriority.MONITOR) + public void onPlayerChangedWorld(final PlayerChangedWorldEvent event) { + // Tell TickTask to update cached permissions. TickTask.requestPermissionUpdate(event.getPlayer().getName(), CheckType.CHAT); - } + } /** * We listen to PlayerChat events for obvious reasons. @@ -115,10 +90,10 @@ public class ChatListener extends CheckListener implements INotifyReload, JoinLe */ @EventHandler(ignoreCancelled = false, priority = EventPriority.LOWEST) public void onPlayerChat(final AsyncPlayerChatEvent event) { - + final Player player = event.getPlayer(); final boolean alreadyCancelled = event.isCancelled(); - + // Tell TickTask to update cached permissions. // (Might omit this if already cancelled.) // TODO: Implement to only update on "timeout" or checks being activated at all. @@ -126,15 +101,15 @@ public class ChatListener extends CheckListener implements INotifyReload, JoinLe // First the color check. if (!alreadyCancelled && color.isEnabled(player)) { - event.setMessage(color.check(player, event.getMessage(), false)); + event.setMessage(color.check(player, event.getMessage(), false)); } // Then the no chat check. // TODO: isMainThread: Could consider event.isAsync ? if (textChecks(player, event.getMessage(), false, alreadyCancelled)) { - event.setCancelled(true); + event.setCancelled(true); } - + } /** @@ -146,71 +121,71 @@ public class ChatListener extends CheckListener implements INotifyReload, JoinLe @EventHandler(priority = EventPriority.LOWEST) public void onPlayerCommandPreprocess(final PlayerCommandPreprocessEvent event) { final Player player = event.getPlayer(); - + // Tell TickTask to update cached permissions. TickTask.requestPermissionUpdate(player.getName(), CheckType.CHAT); - final ChatConfig cc = ChatConfig.getConfig(player); - + final ChatConfig cc = ChatConfig.getConfig(player); + // Checks that replace parts of the message (color). - if (color.isEnabled(player)){ - event.setMessage(color.check(player, event.getMessage(), true)); + if (color.isEnabled(player)) { + event.setMessage(color.check(player, event.getMessage(), true)); } - - // Trim is necessary because the server accepts leading spaces with commands. + + // Left-trim is necessary because the server accepts leading spaces with commands. final String message = event.getMessage(); - final String lcMessage = message.trim().toLowerCase(); + final String lcMessage = StringUtil.leftTrim(message).toLowerCase(); // TODO: Remove bukkit: etc. - + final String[] split = lcMessage.split(" ", 2); final String alias = split[0].substring(1); - final Command command = CommandUtil.getCommand(alias); - - final List messageVars = new ArrayList(); // Could as well use an array and allow null on input of SimpleCharPrefixTree. - messageVars.add(lcMessage); - String checkMessage = message; // Message to run chat checks on. - if (command != null){ - messageVars.add("/" + command.getLabel().toLowerCase() + (split.length > 1 ? (" " + split[1]) : "")); - } + final Command command = CommandUtil.getCommand(alias); + + final List messageVars = new ArrayList(); // Could as well use an array and allow null on input of SimpleCharPrefixTree. + messageVars.add(lcMessage); + String checkMessage = message; // Message to run chat checks on. + if (command != null) { + messageVars.add("/" + command.getLabel().toLowerCase() + (split.length > 1 ? (" " + split[1]) : "")); + } if (alias.indexOf(":") != -1) { - final int index = message.indexOf(":") + 1; - if (index < message.length()) { - checkMessage = message.substring(index); - messageVars.add(checkMessage.toLowerCase()); - } + final int index = message.indexOf(":") + 1; + if (index < message.length()) { + checkMessage = message.substring(index); + messageVars.add(checkMessage.toLowerCase()); + } } // Prevent commands from being used by players (e.g. /op /deop /reload). - if (cc.consoleOnlyCheck && consoleOnlyCommands.hasAnyPrefixWords(messageVars)) { - if (command == null || command.testPermission(player)){ - player.sendMessage(cc.consoleOnlyMessage); - } - event.setCancelled(true); - return; - } + if (cc.consoleOnlyCheck && consoleOnlyCommands.hasAnyPrefixWords(messageVars)) { + if (command == null || command.testPermission(player)) { + player.sendMessage(cc.consoleOnlyMessage); + } + event.setCancelled(true); + return; + } // Handle as chat or command. - if (chatCommands.hasAnyPrefixWords(messageVars)){ + if (chatCommands.hasAnyPrefixWords(messageVars)) { // Treat as chat. - // TODO: Consider requesting permission updates on these, for consistency. - // TODO: Cut off the command ?. + // TODO: Consider requesting permission updates on these, for consistency. + // TODO: Cut off the command ?. if (textChecks(player, checkMessage, true, false)) { - event.setCancelled(true); + event.setCancelled(true); } } - else if (!commandExclusions.hasAnyPrefixWords(messageVars)){ + else if (!commandExclusions.hasAnyPrefixWords(messageVars)) { // Treat as command. if (commands.isEnabled(player) && commands.check(player, checkMessage, captcha)) { - event.setCancelled(true); + event.setCancelled(true); } } } - + private boolean textChecks(final Player player, final String message, final boolean isMainThread, final boolean alreadyCancelled) { - return text.isEnabled(player) && text.check(player, message, captcha, isMainThread, alreadyCancelled); + return text.isEnabled(player) && text.check(player, message, captcha, isMainThread, alreadyCancelled); } - /** + /** * We listen to this type of events to prevent spambots from login to the server. * * @param event @@ -219,54 +194,54 @@ public class ChatListener extends CheckListener implements INotifyReload, JoinLe @EventHandler( priority = EventPriority.NORMAL) public void onPlayerLogin(final PlayerLoginEvent event) { - if (event.getResult() != Result.ALLOWED) return; + if (event.getResult() != Result.ALLOWED) return; final Player player = event.getPlayer(); final ChatConfig cc = ChatConfig.getConfig(player); final ChatData data = ChatData.getData(player); - + // Tell TickTask to update cached permissions. TickTask.requestPermissionUpdate(player.getName(), CheckType.CHAT); // Force permission update. TickTask.updatePermissions(); // TODO: This updates ALL... something more efficient ? - + // Reset captcha of player if needed. - synchronized(data){ + synchronized(data) { captcha.resetCaptcha(cc, data); } // Fast relog check. - if (relog.isEnabled(player) && relog.unsafeLoginCheck(player, cc, data)){ - event.disallow(Result.KICK_OTHER, cc.relogKickMessage); + if (relog.isEnabled(player) && relog.unsafeLoginCheck(player, cc, data)) { + event.disallow(Result.KICK_OTHER, cc.relogKickMessage); } - else if (logins.isEnabled(player) && logins.check(player, cc, data)){ - event.disallow(Result.KICK_OTHER, cc.loginsKickMessage); + else if (logins.isEnabled(player) && logins.check(player, cc, data)) { + event.disallow(Result.KICK_OTHER, cc.loginsKickMessage); } } - @Override - public void onReload() { - // Read some things from the global config file. - ConfigFile config = ConfigManager.getConfigFile(); - initFilters(config); - text.onReload(); - logins.onReload(); - } + @Override + public void onReload() { + // Read some things from the global config file. + ConfigFile config = ConfigManager.getConfigFile(); + initFilters(config); + text.onReload(); + logins.onReload(); + } - @Override - public void playerJoins(final Player player) { - final ChatConfig cc = ChatConfig.getConfig(player); + @Override + public void playerJoins(final Player player) { + final ChatConfig cc = ChatConfig.getConfig(player); final ChatData data = ChatData.getData(player); synchronized (data) { - if (captcha.isEnabled(player) && captcha.shouldCheckCaptcha(cc, data)){ + if (captcha.isEnabled(player) && captcha.shouldCheckCaptcha(cc, data)) { // shouldCheckCaptcha: only if really enabled. // TODO: Later: add check for cc.captchaOnLogin or so (before shouldCheckCaptcha). // TODO: maybe schedule this to come after other plugins messages. captcha.sendNewCaptcha(player, cc, data); } } - } + } - @Override - public void playerLeaves(final Player player) { - } + @Override + public void playerLeaves(final Player player) { + } } diff --git a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/command/CommandUtil.java b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/command/CommandUtil.java index a53c282c..cf6137bd 100644 --- a/NCPCore/src/main/java/fr/neatmonster/nocheatplus/command/CommandUtil.java +++ b/NCPCore/src/main/java/fr/neatmonster/nocheatplus/command/CommandUtil.java @@ -17,7 +17,10 @@ import org.bukkit.plugin.java.JavaPlugin; import fr.neatmonster.nocheatplus.NCPAPIProvider; import fr.neatmonster.nocheatplus.checks.CheckType; +import fr.neatmonster.nocheatplus.config.ConfigFile; import fr.neatmonster.nocheatplus.logging.StaticLog; +import fr.neatmonster.nocheatplus.utilities.StringUtil; +import fr.neatmonster.nocheatplus.utilities.ds.prefixtree.SimpleCharPrefixTree; public class CommandUtil { @@ -154,4 +157,55 @@ public class CommandUtil { return res; } + /** + * Clear tree and feed lower case. Adds versions with "/" if missing, also adds input without the first "/". No trimming is used, except left-trim. + * @param tree + * @param inputs + */ + public static void feedCommands(SimpleCharPrefixTree tree, Collection inputs, boolean clear) { + if (clear) { + tree.clear(); + } + for (String input : inputs) { + if (input == null) { + continue; + } + if (!input.isEmpty()) { + // Do feed the original input. + tree.feed(input); + } + input = StringUtil.leftTrim(input).toLowerCase(); + if (input.isEmpty()) { + continue; + } + tree.feed(input); + if (!input.startsWith("/")) { + // Add version with '/'. + tree.feed("/" + input); + } else { + // Add version without the first '/'. + // TODO: Consider removing all leading '/' here. + input = input.substring(1); + if (!input.isEmpty()) { + tree.feed(input); + } + } + } + } + + /** + * Read string list from config and call feedCommands(tree, list). + * + * @param tree + * @param config + * @param configPath + * @param clear If to clear the map before inserting anything. + */ + public static void feedCommands(SimpleCharPrefixTree tree, ConfigFile config, String configPath, boolean clear) { + final List prefixes = config.getStringList(configPath); + if (prefixes != null) { + feedCommands(tree, prefixes, clear); + } + } + }