From 72172805bae959d4bdbc9e158a2b586fe169dd13 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 9 Apr 2013 16:48:26 +0200 Subject: [PATCH] Exploit the internal JavaScript parser to determine if the exp is done. The original code attempted to parse the JavaScript as it went along, counting open and close brackets. Unfortunately, this doesn't take comments and string literals into consideration, so it would very likely have failed with more complicated filters. Instead, we'll let the JavaScript compiler handle all the complexity and simply see if the code compiles. If it doesn't, but the error occured in the last line, we assume it can be recovered by adding a new line. --- .../com/comphenix/protocol/CommandFilter.java | 132 +++++++++--------- .../protocol/MultipleLinesPrompt.java | 110 +++++++++++++-- 2 files changed, 165 insertions(+), 77 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/CommandFilter.java b/ProtocolLib/src/main/java/com/comphenix/protocol/CommandFilter.java index 0ff5a162..0afe058b 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/CommandFilter.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/CommandFilter.java @@ -2,6 +2,7 @@ package com.comphenix.protocol; import java.util.ArrayList; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Set; @@ -16,11 +17,11 @@ import org.bukkit.conversations.Conversable; import org.bukkit.conversations.Conversation; import org.bukkit.conversations.ConversationAbandonedEvent; import org.bukkit.conversations.ConversationAbandonedListener; -import org.bukkit.conversations.ConversationCanceller; import org.bukkit.conversations.ConversationContext; import org.bukkit.conversations.ConversationFactory; import org.bukkit.plugin.Plugin; +import com.comphenix.protocol.MultipleLinesPrompt.MultipleConversationCanceller; import com.comphenix.protocol.concurrency.IntegerSet; import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.events.PacketEvent; @@ -34,23 +35,17 @@ import com.google.common.collect.Ranges; * @author Kristian */ public class CommandFilter extends CommandBase { - @SuppressWarnings("serial") - public static class FilterFailedException extends RuntimeException { - private Filter filter; - - public FilterFailedException() { - super(); - } - - public FilterFailedException(String message, Filter filter, Throwable cause) { - super(message, cause); - this.filter = filter; - } - - public Filter getFilter() { - return filter; - } + public interface FilterFailedHandler{ + /** + * Invoked when a given filter has failed. + * @param event - the packet event. + * @param filter - the filter that failed. + * @param ex - the failure. + * @returns TRUE to keep processing this filter, FALSE to remove it. + */ + public boolean handle(PacketEvent event, Filter filter, Exception ex); } + /** * Possible sub commands. * @@ -123,7 +118,7 @@ public class CommandFilter extends CommandBase { * @param context - the current script context. * @param event - the packet event to evaluate. * @return TRUE to pass this packet event on to the debug listeners, FALSE otherwise. - * @throws ScriptException If the compilation failed. + * @throws ScriptException If the compilation failed or the filter is not valid. */ public boolean evaluate(ScriptEngine context, PacketEvent event) throws ScriptException { if (!isApplicable(event)) @@ -132,7 +127,13 @@ public class CommandFilter extends CommandBase { compile(context); try { - return (Boolean) ((Invocable) context).invokeFunction(name, event, event.getPacket().getHandle()); + Object result = ((Invocable) context).invokeFunction(name, event, event.getPacket().getHandle()); + + if (result instanceof Boolean) + return (Boolean) result; + else + throw new ScriptException("Filter result wasn't a boolean: " + result); + } catch (NoSuchMethodException e) { // Must be a fault with the script engine itself throw new IllegalStateException("Unable to compile " + name + " into current script engine.", e); @@ -159,54 +160,36 @@ public class CommandFilter extends CommandBase { } } - private static class BracketBalance implements ConversationCanceller { - private String KEY_BRACKET_COUNT = "bracket_balance.count"; - - // What to set the initial counter - private final int initialBalance; - - public BracketBalance(int initialBalance) { - this.initialBalance = initialBalance; - } - + private class CompilationSuccessCanceller implements MultipleConversationCanceller { @Override public boolean cancelBasedOnInput(ConversationContext context, String in) { - Object stored = context.getSessionData(KEY_BRACKET_COUNT); - int value = 0; - - // Get the stored value - if (stored instanceof Integer) { - value = (Integer)stored; - } else { - value = initialBalance; - } - - value += count(in, '{') - count(in, '}'); - context.setSessionData(KEY_BRACKET_COUNT, value); - - // Cancel if the bracket balance is zero - return value <= 0; - } - - private int count(String text, char character) { - int counter = 0; - - for (int i=0; i < text.length(); i++) { - if (text.charAt(i) == character) { - counter++; - } - } - return counter; + throw new UnsupportedOperationException("Cannot cancel on the last line alone."); } @Override public void setConversation(Conversation conversation) { - // Whatever + // Ignore + } + + @Override + public boolean cancelBasedOnInput(ConversationContext context, String currentLine, StringBuilder lines, int lineCount) { + try { + engine.eval("function(event, packet) {\n" + lines.toString()); + + // It compiles - accept the filter! + return true; + } catch (ScriptException e) { + // We also have the function() line + int realLineCount = lineCount + 1; + + // Only possible to recover from an error on the last line. + return e.getLineNumber() < realLineCount; + } } @Override - public ConversationCanceller clone() { - return new BracketBalance(initialBalance); + public CompilationSuccessCanceller clone() { + return new CompilationSuccessCanceller(); } } @@ -251,18 +234,41 @@ public class CommandFilter extends CommandBase { /** * Determine whether or not to pass the given packet event to the packet listeners. + *

+ * Uses a default filter failure handler that simply prints the error message and removes the filter. * @param event - the event. * @return TRUE if we should, FALSE otherwise. + */ + public boolean filterEvent(PacketEvent event) { + return filterEvent(event, new FilterFailedHandler() { + @Override + public boolean handle(PacketEvent event, Filter filter, Exception ex) { + reporter.reportMinimal(plugin, "filterEvent(PacketEvent)", ex, event); + reporter.reportWarning(this, "Removing filter " + filter.getName() + " for causing an exception."); + return false; + } + }); + } + + /** + * Determine whether or not to pass the given packet event to the packet listeners. + * @param event - the event. + * @param handler - failure handler. + * @return TRUE if we should, FALSE otherwise. * @throws FilterFailedException If one of the filters failed. */ - public boolean filterEvent(PacketEvent event) throws FilterFailedException { - for (Filter filter : filters) { + public boolean filterEvent(PacketEvent event, FilterFailedHandler handler) { + for (Iterator it = filters.iterator(); it.hasNext(); ) { + Filter filter = it.next(); + try { if (!filter.evaluate(engine, event)) { return false; } - } catch (ScriptException e) { - throw new FilterFailedException("Filter failed.", filter, e); + } catch (Exception ex) { + if (!handler.handle(event, filter, ex)) { + it.remove(); + } } } // Pass! @@ -297,7 +303,7 @@ public class CommandFilter extends CommandBase { // Make sure we can use the conversable interface if (sender instanceof Conversable) { final MultipleLinesPrompt prompt = - new MultipleLinesPrompt(new BracketBalance(1), "function(event, packet) {"); + new MultipleLinesPrompt(new CompilationSuccessCanceller(), "function(event, packet) {"); new ConversationFactory(plugin). withFirstPrompt(prompt). diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/MultipleLinesPrompt.java b/ProtocolLib/src/main/java/com/comphenix/protocol/MultipleLinesPrompt.java index 48695a60..53f1229e 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/MultipleLinesPrompt.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/MultipleLinesPrompt.java @@ -1,5 +1,6 @@ package com.comphenix.protocol; +import org.bukkit.conversations.Conversation; import org.bukkit.conversations.ConversationCanceller; import org.bukkit.conversations.ConversationContext; import org.bukkit.conversations.ExactMatchConversationCanceller; @@ -12,40 +13,117 @@ import org.bukkit.conversations.StringPrompt; * @author Kristian */ class MultipleLinesPrompt extends StringPrompt { + /** + * Represents a canceller that determines if the multiple lines prompt is finished. + * @author Kristian + */ + public static interface MultipleConversationCanceller extends ConversationCanceller { + @Override + public boolean cancelBasedOnInput(ConversationContext context, String currentLine); + + /** + * Determine if the current prompt is done based on the context, last + * line and collected lines. + * + * @param context - current context. + * @param currentLine - current (last) line. + * @param lines - collected lines. + * @param lineCount - number of lines. + * @return TRUE if we are done, FALSE otherwise. + */ + public boolean cancelBasedOnInput(ConversationContext context, String currentLine, + StringBuilder lines, int lineCount); + } + + /** + * A wrapper class for turning a ConversationCanceller into a MultipleConversationCanceller. + * @author Kristian + */ + private static class MultipleWrapper implements MultipleConversationCanceller { + private ConversationCanceller canceller; + + public MultipleWrapper(ConversationCanceller canceller) { + this.canceller = canceller; + } + + @Override + public boolean cancelBasedOnInput(ConversationContext context, String currentLine) { + return canceller.cancelBasedOnInput(context, currentLine); + } + + @Override + public boolean cancelBasedOnInput(ConversationContext context, String currentLine, + StringBuilder lines, int lineCount) { + return cancelBasedOnInput(context, currentLine); + } + + @Override + public void setConversation(Conversation conversation) { + canceller.setConversation(conversation); + } + + @Override + public MultipleWrapper clone() { + return new MultipleWrapper(canceller.clone()); + } + } + // Feels a bit like Android private static final String KEY = "multiple_lines_prompt"; private static final String KEY_LAST = KEY + ".last_line"; - - private final ConversationCanceller endMarker; + private static final String KEY_LINES = KEY + ".linecount"; + + private final MultipleConversationCanceller endMarker; private final String initialPrompt; - + /** * Retrieve and remove the current accumulated input. - * @param context - conversation context. + * + * @param context + * - conversation context. * @return The accumulated input, or NULL if not found. */ public String removeAccumulatedInput(ConversationContext context) { Object result = context.getSessionData(KEY); - + if (result instanceof StringBuilder) { context.setSessionData(KEY, null); + context.setSessionData(KEY_LINES, null); return ((StringBuilder) result).toString(); } else { return null; } } - + /** - * Construct a multiple lines input prompt with a specific end marker. + * Construct a multiple lines input prompt with a specific end marker. *

* This is usually an empty string. + * * @param endMarker - the end marker. */ public MultipleLinesPrompt(String endMarker, String initialPrompt) { this(new ExactMatchConversationCanceller(endMarker), initialPrompt); } - + + /** + * Construct a multiple lines input prompt with a specific end marker implementation. + *

+ * Note: Use {@link #MultipleLinesPrompt(MultipleConversationCanceller, String)} if implementing a custom canceller. + * @param endMarker - the end marker. + * @param initialPrompt - the initial prompt text. + */ public MultipleLinesPrompt(ConversationCanceller endMarker, String initialPrompt) { + this.endMarker = new MultipleWrapper(endMarker); + this.initialPrompt = initialPrompt; + } + + /** + * Construct a multiple lines input prompt with a specific end marker implementation. + * @param endMarker - the end marker. + * @param initialPrompt - the initial prompt text. + */ + public MultipleLinesPrompt(MultipleConversationCanceller endMarker, String initialPrompt) { this.endMarker = endMarker; this.initialPrompt = initialPrompt; } @@ -53,17 +131,21 @@ class MultipleLinesPrompt extends StringPrompt { @Override public Prompt acceptInput(ConversationContext context, String in) { StringBuilder result = (StringBuilder) context.getSessionData(KEY); + Integer count = (Integer) context.getSessionData(KEY_LINES); - if (result == null) { + // Handle first run + if (result == null) context.setSessionData(KEY, result = new StringBuilder()); - } + if (count == null) + count = 0; // Save the last line as well context.setSessionData(KEY_LAST, in); - result.append(in); - + context.setSessionData(KEY_LINES, ++count); + result.append(in + "\n"); + // And we're done - if (endMarker.cancelBasedOnInput(context, in)) + if (endMarker.cancelBasedOnInput(context, in, result, count)) return Prompt.END_OF_CONVERSATION; else return this; @@ -72,7 +154,7 @@ class MultipleLinesPrompt extends StringPrompt { @Override public String getPromptText(ConversationContext context) { Object last = context.getSessionData(KEY_LAST); - + if (last instanceof String) return (String) last; else