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.

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 interface FilterFailedHandler{
-	public static class FilterFailedException extends RuntimeException {
+		/**
-		private Filter filter;
+		 * Invoked when a given filter has failed.
-
+		 * @param event - the packet event.
-		public FilterFailedException() {
+		 * @param filter - the filter that failed.
-			super();
+		 * @param ex - the failure.
+		 * @returns TRUE to keep processing this filter, FALSE to remove it.
+		 */
+		public boolean handle(PacketEvent event, Filter filter, Exception ex);
 	}
 	
-		public FilterFailedException(String message, Filter filter, Throwable cause) {
-			super(message, cause);
-			this.filter = filter;
-		}
-
-		public Filter getFilter() {
-			return filter;
-		}
-	}
 	/**
 	 * 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 class CompilationSuccessCanceller implements MultipleConversationCanceller {
-		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;
-		}
-		
 		@Override
 		public boolean cancelBasedOnInput(ConversationContext context, String in) {
-			Object stored = context.getSessionData(KEY_BRACKET_COUNT);
+			throw new UnsupportedOperationException("Cannot cancel on the last line alone.");
-			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;
 		}
 
 		@Override
 		public void setConversation(Conversation conversation) {
-			// Whatever
+			// Ignore
 		}
 
 		@Override
-		public ConversationCanceller clone() {
+		public boolean cancelBasedOnInput(ConversationContext context, String currentLine, StringBuilder lines, int lineCount) {
-			return new BracketBalance(initialBalance);
+			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 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.
+	 * <p>
+	 * 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 {
+	public boolean filterEvent(PacketEvent event, FilterFailedHandler handler) {
-		for (Filter filter : filters) {
+		for (Iterator<Filter> it = filters.iterator(); it.hasNext(); ) {
+			Filter filter = it.next();
+			
 			try {
 				if (!filter.evaluate(engine, event)) {
 					return false;
 				}
-			} catch (ScriptException e) {
+			} catch (Exception ex) {
-				throw new FilterFailedException("Filter failed.", filter, e);
+				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).

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,16 +13,74 @@ 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 static final String KEY_LINES = KEY + ".linecount";
 
-	private final ConversationCanceller endMarker;
+	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) {
@@ -29,6 +88,7 @@ class MultipleLinesPrompt extends StringPrompt {
 
 		if (result instanceof StringBuilder) {
 			context.setSessionData(KEY, null);
+			context.setSessionData(KEY_LINES, null);
 			return ((StringBuilder) result).toString();
 		} else {
 			return null;
@@ -39,13 +99,31 @@ class MultipleLinesPrompt extends StringPrompt {
 	 * Construct a multiple lines input prompt with a specific end marker.
 	 * <p>
 	 * 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.
+	 * <p>
+	 * 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;