From bb923aeb594571b64b8737432e6dc564d26b10f0 Mon Sep 17 00:00:00 2001 From: Kenzie Togami Date: Thu, 2 Nov 2017 14:39:37 -0700 Subject: [PATCH 1/4] Attach a configurable timeout to expression evaluation --- .../sk89q/worldedit/LocalConfiguration.java | 1 + .../internal/expression/Expression.java | 41 +++++++- .../internal/expression/runtime/For.java | 3 + .../expression/runtime/SimpleFor.java | 3 + .../internal/expression/runtime/While.java | 6 ++ .../util/PropertiesConfiguration.java | 1 + .../worldedit/util/YAMLConfiguration.java | 2 + .../expression/ExpressionPlatform.java | 96 +++++++++++++++++++ 8 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/LocalConfiguration.java b/worldedit-core/src/main/java/com/sk89q/worldedit/LocalConfiguration.java index 81b4d892a..3ef39d94b 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/LocalConfiguration.java +++ b/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 allowedDataCycleBlocks = new HashSet<>(); public String saveDir = "schematics"; public String scriptsDir = "craftscripts"; diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java index 463792213..27266e642 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java +++ b/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> instance = new ThreadLocal<>(); + private static final ExecutorService evalThread = Executors.newCachedThreadPool( + new ThreadFactoryBuilder() + .setDaemon(true) + .setNameFormat("worldedit-expression-eval-%d") + .build()); private final Map variables = new HashMap<>(); private final String[] variableNames; @@ -115,9 +129,30 @@ public class Expression { pushInstance(); try { - return root.getValue(); - } catch (ReturnException e) { - return e.getValue(); + Future result = evalThread.submit(new Callable() { + @Override + public Double call() throws Exception { + return root.getValue(); + } + }); + try { + return result.get(WorldEdit.getInstance().getConfiguration().calculationTimeout, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } 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."); + } } finally { popInstance(); } diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/For.java b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/For.java index 868c83f96..6334a7350 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/For.java +++ b/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 { diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/SimpleFor.java b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/SimpleFor.java index 36cf5da81..1576c3484 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/SimpleFor.java +++ b/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 { diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/While.java b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/While.java index 4c277058a..5da3dae01 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/runtime/While.java +++ b/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 { diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/util/PropertiesConfiguration.java b/worldedit-core/src/main/java/com/sk89q/worldedit/util/PropertiesConfiguration.java index 65f44c691..fee1917aa 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/util/PropertiesConfiguration.java +++ b/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); diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/util/YAMLConfiguration.java b/worldedit-core/src/main/java/com/sk89q/worldedit/util/YAMLConfiguration.java index 41745a0aa..9fa16df5f 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/util/YAMLConfiguration.java +++ b/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); diff --git a/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java b/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java new file mode 100644 index 000000000..6d806d8ab --- /dev/null +++ b/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java @@ -0,0 +1,96 @@ +/* + * WorldEdit, a Minecraft world manipulation toolkit + * Copyright (C) sk89q + * Copyright (C) WorldEdit team and contributors + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package com.sk89q.worldedit.internal.expression; + +import com.google.common.collect.ImmutableMap; +import com.sk89q.worldedit.LocalConfiguration; +import com.sk89q.worldedit.entity.Player; +import com.sk89q.worldedit.extension.platform.AbstractPlatform; +import com.sk89q.worldedit.extension.platform.Capability; +import com.sk89q.worldedit.extension.platform.Preference; +import com.sk89q.worldedit.util.command.Dispatcher; +import com.sk89q.worldedit.world.World; + +import java.util.Map; + +final class ExpressionPlatform extends AbstractPlatform { + + @Override + public int resolveItem(String name) { + return 0; + } + + @Override + public boolean isValidMobType(String type) { + return false; + } + + @Override + public void reload() { + } + + @Override + public Player matchPlayer(Player player) { + return null; + } + + @Override + public World matchWorld(World world) { + return null; + } + + @Override + public void registerCommands(Dispatcher dispatcher) { + } + + @Override + public void registerGameHooks() { + } + + @Override + public LocalConfiguration getConfiguration() { + return new LocalConfiguration() { + + @Override + public void load() { + } + }; + } + + @Override + public String getVersion() { + return "INVALID"; + } + + @Override + public String getPlatformName() { + return "Expression Test"; + } + + @Override + public String getPlatformVersion() { + return "INVALID"; + } + + @Override + public Map getCapabilities() { + return ImmutableMap.of(Capability.CONFIGURATION, Preference.PREFER_OTHERS); + } +} From 21db86f26b3515e1b40035318f26b51546f427cc Mon Sep 17 00:00:00 2001 From: Kenzie Togami Date: Mon, 6 Nov 2017 10:28:32 -0800 Subject: [PATCH 2/4] Register a platform for expression tests --- .../internal/expression/Expression.java | 50 +++++++++---------- .../internal/expression/ExpressionTest.java | 12 ++++- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java index 27266e642..b6bd21bb7 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java @@ -127,34 +127,34 @@ public class Expression { ((Variable) invokable).value = values[i]; } - pushInstance(); - try { - Future result = evalThread.submit(new Callable() { - @Override - public Double call() throws Exception { + Future result = evalThread.submit(new Callable() { + @Override + public Double call() throws Exception { + pushInstance(); + try { return root.getValue(); + } finally { + popInstance(); } - }); - try { - return result.get(WorldEdit.getInstance().getConfiguration().calculationTimeout, TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new RuntimeException(e); - } 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."); } - } finally { - popInstance(); + }); + try { + return result.get(WorldEdit.getInstance().getConfiguration().calculationTimeout, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } 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."); } } diff --git a/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java b/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java index 1d0456d01..207bb4e56 100644 --- a/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java +++ b/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java @@ -24,13 +24,23 @@ import static java.lang.Math.sin; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import org.junit.Before; +import org.junit.Test; + +import com.sk89q.worldedit.WorldEdit; +import com.sk89q.worldedit.event.platform.PlatformReadyEvent; 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() { + WorldEdit.getInstance().getPlatformManager().register(new ExpressionPlatform()); + WorldEdit.getInstance().getPlatformManager().handlePlatformReady(new PlatformReadyEvent()); + } + @Test public void testEvaluate() throws ExpressionException { // check From 776eb24c0e7621b761f399b97fb90568ae92ba26 Mon Sep 17 00:00:00 2001 From: Legoman99573 Date: Wed, 22 Nov 2017 18:04:12 -0600 Subject: [PATCH 3/4] Calculation Config missing and typo --- worldedit-bukkit/src/main/resources/defaults/config.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/worldedit-bukkit/src/main/resources/defaults/config.yml b/worldedit-bukkit/src/main/resources/defaults/config.yml index a01430cfd..58bff2d20 100644 --- a/worldedit-bukkit/src/main/resources/defaults/config.yml +++ b/worldedit-bukkit/src/main/resources/defaults/config.yml @@ -136,6 +136,9 @@ files: history: size: 15 expiration: 10 + +calculation: + timeout: 100 wand-item: minecraft:wooden_axe shell-save-type: From e16dacc11ee0f6df7854989d4e1b0d6068adfd75 Mon Sep 17 00:00:00 2001 From: Kenzie Togami Date: Mon, 1 Oct 2018 16:04:48 -0700 Subject: [PATCH 4/4] Small patches for timed-calc post-1.12-merge --- .../src/main/resources/defaults/config.yml | 4 +- .../internal/expression/Expression.java | 14 +-- .../expression/ExpressionPlatform.java | 96 ------------------- .../internal/expression/ExpressionTest.java | 24 ++++- 4 files changed, 30 insertions(+), 108 deletions(-) delete mode 100644 worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java diff --git a/worldedit-bukkit/src/main/resources/defaults/config.yml b/worldedit-bukkit/src/main/resources/defaults/config.yml index 58bff2d20..108994b61 100644 --- a/worldedit-bukkit/src/main/resources/defaults/config.yml +++ b/worldedit-bukkit/src/main/resources/defaults/config.yml @@ -136,7 +136,7 @@ files: history: size: 15 expiration: 10 - + calculation: timeout: 100 @@ -146,4 +146,4 @@ no-double-slash: false no-op-permissions: false debug: false show-help-on-first-use: true -server-side-cui: true \ No newline at end of file +server-side-cui: true diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java index b6bd21bb7..ec774f088 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/Expression.java @@ -181,20 +181,20 @@ public class Expression { } private void pushInstance() { - Stack foo = instance.get(); - if (foo == null) { - instance.set(foo = new Stack<>()); + Stack threadLocalExprStack = instance.get(); + if (threadLocalExprStack == null) { + instance.set(threadLocalExprStack = new Stack<>()); } - foo.push(this); + threadLocalExprStack.push(this); } private void popInstance() { - Stack foo = instance.get(); + Stack threadLocalExprStack = instance.get(); - foo.pop(); + threadLocalExprStack.pop(); - if (foo.isEmpty()) { + if (threadLocalExprStack.isEmpty()) { instance.set(null); } } diff --git a/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java b/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java deleted file mode 100644 index 6d806d8ab..000000000 --- a/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionPlatform.java +++ /dev/null @@ -1,96 +0,0 @@ -/* - * WorldEdit, a Minecraft world manipulation toolkit - * Copyright (C) sk89q - * Copyright (C) WorldEdit team and contributors - * - * This program is free software: you can redistribute it and/or modify it - * under the terms of the GNU Lesser General Public License as published by the - * Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License - * for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program. If not, see . - */ - -package com.sk89q.worldedit.internal.expression; - -import com.google.common.collect.ImmutableMap; -import com.sk89q.worldedit.LocalConfiguration; -import com.sk89q.worldedit.entity.Player; -import com.sk89q.worldedit.extension.platform.AbstractPlatform; -import com.sk89q.worldedit.extension.platform.Capability; -import com.sk89q.worldedit.extension.platform.Preference; -import com.sk89q.worldedit.util.command.Dispatcher; -import com.sk89q.worldedit.world.World; - -import java.util.Map; - -final class ExpressionPlatform extends AbstractPlatform { - - @Override - public int resolveItem(String name) { - return 0; - } - - @Override - public boolean isValidMobType(String type) { - return false; - } - - @Override - public void reload() { - } - - @Override - public Player matchPlayer(Player player) { - return null; - } - - @Override - public World matchWorld(World world) { - return null; - } - - @Override - public void registerCommands(Dispatcher dispatcher) { - } - - @Override - public void registerGameHooks() { - } - - @Override - public LocalConfiguration getConfiguration() { - return new LocalConfiguration() { - - @Override - public void load() { - } - }; - } - - @Override - public String getVersion() { - return "INVALID"; - } - - @Override - public String getPlatformName() { - return "Expression Test"; - } - - @Override - public String getPlatformVersion() { - return "INVALID"; - } - - @Override - public Map getCapabilities() { - return ImmutableMap.of(Capability.CONFIGURATION, Preference.PREFER_OTHERS); - } -} diff --git a/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java b/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java index 207bb4e56..bdb91abc2 100644 --- a/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java +++ b/worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java @@ -22,13 +22,16 @@ 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.event.platform.PlatformReadyEvent; +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; @@ -37,8 +40,13 @@ import com.sk89q.worldedit.internal.expression.runtime.ExpressionEnvironment; public class ExpressionTest { @Before public void setup() { - WorldEdit.getInstance().getPlatformManager().register(new ExpressionPlatform()); - WorldEdit.getInstance().getPlatformManager().handlePlatformReady(new PlatformReadyEvent()); + Platform mockPlat = Mockito.mock(Platform.class); + Mockito.when(mockPlat.getConfiguration()).thenReturn(new LocalConfiguration() { + @Override + public void load() { + } + }); + WorldEdit.getInstance().getPlatformManager().register(mockPlat); } @Test @@ -172,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);