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.
This commit is contained in:
asofold 2014-12-17 18:33:16 +01:00
parent 878848c4d1
commit ea5b249197
4 changed files with 214 additions and 130 deletions

View File

@ -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;
}
}
}

View File

@ -91,4 +91,26 @@ public class TestStringUtil {
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]);
}
}
}

View File

@ -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;
@ -63,49 +63,24 @@ public class ChatListener extends CheckListener implements INotifyReload, JoinLe
/** 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<String> 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.
@ -126,13 +101,13 @@ 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);
}
}
@ -150,67 +125,67 @@ public class ChatListener extends CheckListener implements INotifyReload, JoinLe
// 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 Command command = CommandUtil.getCommand(alias);
final List<String> messageVars = new ArrayList<String>(); // 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 List<String> messageVars = new ArrayList<String>(); // 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]) : ""));
}
// 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;
}
// Handle as chat or command.
if (chatCommands.hasAnyPrefixWords(messageVars)){
// Treat as chat.
// TODO: Consider requesting permission updates on these, for consistency.
// TODO: Cut off the command ?.
if (textChecks(player, checkMessage, true, false)) {
event.setCancelled(true);
if (alias.indexOf(":") != -1) {
final int index = message.indexOf(":") + 1;
if (index < message.length()) {
checkMessage = message.substring(index);
messageVars.add(checkMessage.toLowerCase());
}
}
else if (!commandExclusions.hasAnyPrefixWords(messageVars)){
// 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;
}
// Handle as chat or command.
if (chatCommands.hasAnyPrefixWords(messageVars)) {
// Treat as chat.
// TODO: Consider requesting permission updates on these, for consistency.
// TODO: Cut off the command ?.
if (textChecks(player, checkMessage, true, false)) {
event.setCancelled(true);
}
}
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,7 +194,7 @@ 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);
@ -230,43 +205,43 @@ public class ChatListener extends CheckListener implements INotifyReload, JoinLe
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) {
}
}

View File

@ -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<String> 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<String> prefixes = config.getStringList(configPath);
if (prefixes != null) {
feedCommands(tree, prefixes, clear);
}
}
}