Merge pull request #400 from sk89q/bugfix/timed-calc

Attach a configurable timeout to expression evaluation

worldedit-bukkit/src/main/resources/defaults/config.yml

@@ -137,6 +137,9 @@ history:
     size: 15
     expiration: 10
 
+calculation:
+    timeout: 100
+
 wand-item: minecraft:wooden_axe
 shell-save-type:
 no-double-slash: false

worldedit-core/src/main/java/com/sk89q/worldedit/LocalConfiguration.java

@@ -127,6 +127,7 @@ public abstract class LocalConfiguration {
     public String navigationWand = ItemTypes.COMPASS.getId();
     public int navigationWandMaxDistance = 50;
     public int scriptTimeout = 3000;
+    public int calculationTimeout = 100;
     public Set<String> allowedDataCycleBlocks = new HashSet<>();
     public String saveDir = "schematics";
     public String scriptsDir = "craftscripts";

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java

@@ -19,6 +19,8 @@
 
 package com.sk89q.worldedit.internal.expression;
 
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import com.sk89q.worldedit.WorldEdit;
 import com.sk89q.worldedit.internal.expression.lexer.Lexer;
 import com.sk89q.worldedit.internal.expression.lexer.tokens.Token;
 import com.sk89q.worldedit.internal.expression.parser.Parser;
@@ -34,6 +36,13 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Stack;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 /**
  * Compiles and evaluates expressions.
@@ -69,6 +78,11 @@ import java.util.Stack;
 public class Expression {
 
     private static final ThreadLocal<Stack<Expression>> instance = new ThreadLocal<>();
+    private static final ExecutorService evalThread = Executors.newCachedThreadPool(
+            new ThreadFactoryBuilder()
+                    .setDaemon(true)
+                    .setNameFormat("worldedit-expression-eval-%d")
+                    .build());
 
     private final Map<String, RValue> variables = new HashMap<>();
     private final String[] variableNames;
@@ -113,13 +127,34 @@ public class Expression {
             ((Variable) invokable).value = values[i];
         }
 
-        pushInstance();
+        Future<Double> result = evalThread.submit(new Callable<Double>() {
+            @Override
+            public Double call() throws Exception {
+                pushInstance();
+                try {
+                    return root.getValue();
+                } finally {
+                    popInstance();
+                }
+            }
+        });
         try {
-            return root.getValue();
+            return result.get(WorldEdit.getInstance().getConfiguration().calculationTimeout, TimeUnit.MILLISECONDS);
-        } catch (ReturnException e) {
+        } catch (InterruptedException e) {
-            return e.getValue();
+            Thread.currentThread().interrupt();
-        } finally {
+            throw new RuntimeException(e);
-            popInstance();
+        } catch (ExecutionException e) {
+            Throwable cause = e.getCause();
+            if (cause instanceof ReturnException) {
+                return ((ReturnException) cause).getValue();
+            }
+            if (cause instanceof RuntimeException) {
+                throw (RuntimeException) cause;
+            }
+            throw new RuntimeException(cause);
+        } catch (TimeoutException e) {
+            result.cancel(true);
+            throw new EvaluationException(-1, "Calculations exceeded time limit.");
         }
     }
 
@@ -146,20 +181,20 @@ public class Expression {
     }
 
     private void pushInstance() {
-        Stack<Expression> foo = instance.get();
+        Stack<Expression> threadLocalExprStack = instance.get();
-        if (foo == null) {
+        if (threadLocalExprStack == null) {
-            instance.set(foo = new Stack<>());
+            instance.set(threadLocalExprStack = new Stack<>());
         }
 
-        foo.push(this);
+        threadLocalExprStack.push(this);
     }
 
     private void popInstance() {
-        Stack<Expression> foo = instance.get();
+        Stack<Expression> threadLocalExprStack = instance.get();
 
-        foo.pop();
+        threadLocalExprStack.pop();
 
-        if (foo.isEmpty()) {
+        if (threadLocalExprStack.isEmpty()) {
             instance.set(null);
         }
     }

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/For.java

@@ -50,6 +50,9 @@ public class For extends Node {
             if (iterations > 256) {
                 throw new EvaluationException(getPosition(), "Loop exceeded 256 iterations.");
             }
+            if (Thread.interrupted()) {
+                throw new EvaluationException(getPosition(), "Calculations exceeded time limit.");
+            }
             ++iterations;
 
             try {

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/SimpleFor.java

@@ -53,6 +53,9 @@ public class SimpleFor extends Node {
             if (iterations > 256) {
                 throw new EvaluationException(getPosition(), "Loop exceeded 256 iterations.");
             }
+            if (Thread.interrupted()) {
+                throw new EvaluationException(getPosition(), "Calculations exceeded time limit.");
+            }
             ++iterations;
 
             try {

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/While.java

@@ -49,6 +49,9 @@ public class While extends Node {
                 if (iterations > 256) {
                     throw new EvaluationException(getPosition(), "Loop exceeded 256 iterations.");
                 }
+                if (Thread.interrupted()) {
+                    throw new EvaluationException(getPosition(), "Calculations exceeded time limit.");
+                }
                 ++iterations;
 
                 try {
@@ -66,6 +69,9 @@ public class While extends Node {
                 if (iterations > 256) {
                     throw new EvaluationException(getPosition(), "Loop exceeded 256 iterations.");
                 }
+                if (Thread.interrupted()) {
+                    throw new EvaluationException(getPosition(), "Calculations exceeded time limit.");
+                }
                 ++iterations;
 
                 try {

worldedit-core/src/main/java/com/sk89q/worldedit/util/PropertiesConfiguration.java

@@ -109,6 +109,7 @@ public class PropertiesConfiguration extends LocalConfiguration {
         navigationWandMaxDistance = getInt("nav-wand-distance", navigationWandMaxDistance);
         navigationUseGlass = getBool("nav-use-glass", navigationUseGlass);
         scriptTimeout = getInt("scripting-timeout", scriptTimeout);
+        calculationTimeout = getInt("calculation-timeout", calculationTimeout);
         saveDir = getString("schematic-save-dir", saveDir);
         scriptsDir = getString("craftscript-dir", scriptsDir);
         butcherDefaultRadius = getInt("butcher-default-radius", butcherDefaultRadius);

worldedit-core/src/main/java/com/sk89q/worldedit/util/YAMLConfiguration.java

@@ -105,6 +105,8 @@ public class YAMLConfiguration extends LocalConfiguration {
         scriptTimeout = config.getInt("scripting.timeout", scriptTimeout);
         scriptsDir = config.getString("scripting.dir", scriptsDir);
 
+        calculationTimeout = config.getInt("calculation.timeout", calculationTimeout);
+
         saveDir = config.getString("saving.dir", saveDir);
 
         allowSymlinks = config.getBoolean("files.allow-symbolic-links", false);

worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java

@@ -22,15 +22,33 @@ package com.sk89q.worldedit.internal.expression;
 import static java.lang.Math.atan2;
 import static java.lang.Math.sin;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import com.sk89q.worldedit.LocalConfiguration;
+import com.sk89q.worldedit.WorldEdit;
+import com.sk89q.worldedit.extension.platform.Platform;
 import com.sk89q.worldedit.internal.expression.lexer.LexerException;
 import com.sk89q.worldedit.internal.expression.parser.ParserException;
 import com.sk89q.worldedit.internal.expression.runtime.EvaluationException;
 import com.sk89q.worldedit.internal.expression.runtime.ExpressionEnvironment;
-import org.junit.Test;
 
 public class ExpressionTest {
+    @Before
+    public void setup() {
+        Platform mockPlat = Mockito.mock(Platform.class);
+        Mockito.when(mockPlat.getConfiguration()).thenReturn(new LocalConfiguration() {
+            @Override
+            public void load() {
+            }
+        });
+        WorldEdit.getInstance().getPlatformManager().register(mockPlat);
+    }
+
     @Test
     public void testEvaluate() throws ExpressionException {
         // check 
@@ -162,6 +180,16 @@ public class ExpressionTest {
         assertEquals(1, simpleEval("!queryRel(3,4,5,100,200)"), 0);
     }
 
+    @Test
+    public void testTimeout() throws Exception {
+        try {
+            simpleEval("for(i=0;i<256;i++){for(j=0;j<256;j++){for(k=0;k<256;k++){for(l=0;l<256;l++){ln(pi)}}}}");
+            fail("Loop was not stopped.");
+        } catch (EvaluationException e) {
+            assertTrue(e.getMessage().contains("Calculations exceeded time limit"));
+        }
+    }
+
     private double simpleEval(String expressionString) throws ExpressionException {
         final Expression expression = compile(expressionString);