From e392856049bded8dadb0a0fa9340976b5fe069b0 Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Wed, 17 Jan 2024 19:20:25 +0100 Subject: [PATCH 1/3] Make regexp execution loop interruptible #1189 Uses Thread.currentThread.isInterrupted() so that the interruption flag remains set to true, we only terminate the RegExp evaluation loop, but other (potentially third-party calling) code may still have to check for the interrupted flag to stop its execution as well. I also added a test with a long-running regexp that fails without the interrupt check. --- .../javascript/regexp/NativeRegExp.java | 3 +++ .../javascript/tests/NativeRegExpTest.java | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/org/mozilla/javascript/regexp/NativeRegExp.java b/src/org/mozilla/javascript/regexp/NativeRegExp.java index 2b7cffdefe..66dedbdaca 100644 --- a/src/org/mozilla/javascript/regexp/NativeRegExp.java +++ b/src/org/mozilla/javascript/regexp/NativeRegExp.java @@ -1878,6 +1878,9 @@ private static boolean executeREBytecode(REGlobalData gData, String input, int e for (; ; ) { + if (Thread.currentThread().isInterrupted()) { + throw new RuntimeException("Received interrupt while executing regexp"); + } if (reopIsSimple(op)) { int match = simpleMatch(gData, input, op, program, pc, end, true); result = match >= 0; diff --git a/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java b/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java index 944629438d..e1abcce32d 100644 --- a/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java +++ b/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java @@ -5,12 +5,21 @@ package org.mozilla.javascript.tests; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import org.junit.Test; import org.mozilla.javascript.Context; import org.mozilla.javascript.Scriptable; import org.mozilla.javascript.ScriptableObject; +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; + public class NativeRegExpTest { @Test @@ -49,6 +58,21 @@ public void ignoreCase() throws Exception { testEvaluate("i-false-true-false-false", "/foo/i;"); } + /** @throws Exception if an error occurs */ + @Test + public void interruptLongRunningRegExpEvaluation() throws Exception { + ExecutorService executorService = Executors.newSingleThreadExecutor(); + try { + Future future = executorService.submit(() -> test("false", "/(.*){1,32000}[bc]/.test(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\");")); + assertThrows(TimeoutException.class, () -> future.get(1, TimeUnit.SECONDS)); + executorService.shutdownNow(); + assertTrue(executorService.awaitTermination(30, TimeUnit.SECONDS)); + } + finally { + executorService.shutdown(); + } + } + /** @throws Exception if an error occurs */ @Test public void multilineCtor() throws Exception { From 6cdf7370011a339811d316b07d157c95ab43ae01 Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Wed, 24 Jan 2024 09:03:52 +0100 Subject: [PATCH 2/3] run spotless --- .../javascript/tests/NativeRegExpTest.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java b/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java index e1abcce32d..0d5fabb45e 100644 --- a/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java +++ b/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java @@ -8,17 +8,15 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import org.junit.Test; -import org.mozilla.javascript.Context; -import org.mozilla.javascript.Scriptable; -import org.mozilla.javascript.ScriptableObject; - -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; +import org.junit.Test; +import org.mozilla.javascript.Context; +import org.mozilla.javascript.Scriptable; +import org.mozilla.javascript.ScriptableObject; public class NativeRegExpTest { @@ -63,12 +61,16 @@ public void ignoreCase() throws Exception { public void interruptLongRunningRegExpEvaluation() throws Exception { ExecutorService executorService = Executors.newSingleThreadExecutor(); try { - Future future = executorService.submit(() -> test("false", "/(.*){1,32000}[bc]/.test(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\");")); + Future future = + executorService.submit( + () -> + test( + "false", + "/(.*){1,32000}[bc]/.test(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\");")); assertThrows(TimeoutException.class, () -> future.get(1, TimeUnit.SECONDS)); executorService.shutdownNow(); assertTrue(executorService.awaitTermination(30, TimeUnit.SECONDS)); - } - finally { + } finally { executorService.shutdown(); } } From dd62bc02d01926d288c7b03dac7e2556d5ac28fa Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Tue, 30 Jan 2024 20:11:14 +0100 Subject: [PATCH 3/3] Use ScriptRuntime#observeInstructionCount for regexp loop #1440 --- .../javascript/regexp/NativeRegExp.java | 12 ++++---- .../javascript/tests/NativeRegExpTest.java | 28 +------------------ .../tests/ObserveInstructionCountTest.java | 7 +++++ 3 files changed, 14 insertions(+), 33 deletions(-) diff --git a/src/org/mozilla/javascript/regexp/NativeRegExp.java b/src/org/mozilla/javascript/regexp/NativeRegExp.java index 66dedbdaca..c21e71d377 100644 --- a/src/org/mozilla/javascript/regexp/NativeRegExp.java +++ b/src/org/mozilla/javascript/regexp/NativeRegExp.java @@ -1847,7 +1847,8 @@ private static int simpleMatch( return -1; } - private static boolean executeREBytecode(REGlobalData gData, String input, int end) { + private static boolean executeREBytecode( + Context cx, REGlobalData gData, String input, int end) { int pc = 0; byte[] program = gData.regexp.program; int continuationOp = REOP_END; @@ -1877,10 +1878,8 @@ private static boolean executeREBytecode(REGlobalData gData, String input, int e } for (; ; ) { + ScriptRuntime.addInstructionCount(cx, 5); - if (Thread.currentThread().isInterrupted()) { - throw new RuntimeException("Received interrupt while executing regexp"); - } if (reopIsSimple(op)) { int match = simpleMatch(gData, input, op, program, pc, end, true); result = match >= 0; @@ -2309,6 +2308,7 @@ && simpleMatch(gData, input, op, program, pc, end, false) < 0) { } private static boolean matchRegExp( + Context cx, REGlobalData gData, RECompiled re, String input, @@ -2362,7 +2362,7 @@ && upcase(matchCh) == upcase((char) anchorCh))) { for (int j = 0; j < re.parenCount; j++) { gData.parens[j] = -1L; } - boolean result = executeREBytecode(gData, input, end); + boolean result = executeREBytecode(cx, gData, input, end); gData.backTrackStackTop = null; gData.stateStackTop = null; @@ -2396,7 +2396,7 @@ Object executeRegExp( // // Call the recursive matcher to do the real work. // - boolean matches = matchRegExp(gData, re, str, start, end, res.multiline); + boolean matches = matchRegExp(cx, gData, re, str, start, end, res.multiline); if (!matches) { if (matchType != PREFIX) return null; return Undefined.instance; diff --git a/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java b/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java index 0d5fabb45e..944629438d 100644 --- a/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java +++ b/testsrc/org/mozilla/javascript/tests/NativeRegExpTest.java @@ -5,14 +5,7 @@ package org.mozilla.javascript.tests; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; - -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; + import org.junit.Test; import org.mozilla.javascript.Context; import org.mozilla.javascript.Scriptable; @@ -56,25 +49,6 @@ public void ignoreCase() throws Exception { testEvaluate("i-false-true-false-false", "/foo/i;"); } - /** @throws Exception if an error occurs */ - @Test - public void interruptLongRunningRegExpEvaluation() throws Exception { - ExecutorService executorService = Executors.newSingleThreadExecutor(); - try { - Future future = - executorService.submit( - () -> - test( - "false", - "/(.*){1,32000}[bc]/.test(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\");")); - assertThrows(TimeoutException.class, () -> future.get(1, TimeUnit.SECONDS)); - executorService.shutdownNow(); - assertTrue(executorService.awaitTermination(30, TimeUnit.SECONDS)); - } finally { - executorService.shutdown(); - } - } - /** @throws Exception if an error occurs */ @Test public void multilineCtor() throws Exception { diff --git a/testsrc/org/mozilla/javascript/tests/ObserveInstructionCountTest.java b/testsrc/org/mozilla/javascript/tests/ObserveInstructionCountTest.java index a83b351c6b..72d34115ea 100644 --- a/testsrc/org/mozilla/javascript/tests/ObserveInstructionCountTest.java +++ b/testsrc/org/mozilla/javascript/tests/ObserveInstructionCountTest.java @@ -118,4 +118,11 @@ public void forever() { String source = "for(;;);"; baseCase(source); } + + @Test + public void longRunningRegExp() { + String source = + "/(.*){1,32000}[bc]/.test(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\");"; + baseCase(source); + } }