We don't throw SyntaxErrors for runtime generated regular expressions with errors
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 May 2018 02:59:31 +0000 (02:59 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 May 2018 02:59:31 +0000 (02:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185755

Reviewed by Keith Miller.

JSTests:

New regression test.

* stress/regexp-with-runtime-syntax-errors.js: Added.
(testThrowsSyntaxtError):
(fromExecWithBadUnicodeEscape):
(fromTestWithBadUnicodeProperty):
(fromSplitWithBadUnicodeIdentity):
(fromMatchWithBadUnicodeBackReference):
(fromReplaceWithBadUnicodeEscape):
(fromSearchWithBadUnicodeEscape):

Source/JavaScriptCore:

Added a new helper that creates the correct exception to throw for each type of error when
compiling a RegExp.  Using that new helper, added missing checks for RegExp for the cases
where we create a new RegExp from an existing one.  Also refactored other places that we
throw SyntaxErrors after a failed RegExp compile to use the new helper.

* runtime/RegExp.h:
* runtime/RegExpConstructor.cpp:
(JSC::regExpCreate):
(JSC::constructRegExp):
* runtime/RegExpPrototype.cpp:
(JSC::regExpProtoFuncCompile):
* yarr/YarrErrorCode.cpp:
(JSC::Yarr::errorToThrow):
* yarr/YarrErrorCode.h:

LayoutTests:

Updated test and results from reporting a SyntaxError to an Out of memory error.

* js/script-tests/stack-overflow-regexp.js:
(shouldThrow.recursiveCall):
(shouldThrow):
(recursiveCall):
* js/stack-overflow-regexp-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231939 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
JSTests/ChangeLog
JSTests/stress/regexp-with-runtime-syntax-errors.js [new file with mode: 0644]
LayoutTests/ChangeLog
LayoutTests/js/script-tests/stack-overflow-regexp.js
LayoutTests/js/stack-overflow-regexp-expected.txt
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/Error.cpp
Source/JavaScriptCore/runtime/Error.h
Source/JavaScriptCore/runtime/RegExp.h
Source/JavaScriptCore/runtime/RegExpConstructor.cpp
Source/JavaScriptCore/runtime/RegExpPrototype.cpp
Source/JavaScriptCore/yarr/YarrErrorCode.cpp
Source/JavaScriptCore/yarr/YarrErrorCode.h

index adb0425..eadc1b2 100644 (file)
@@ -1,3 +1,21 @@
+2018-05-17  Michael Saboff  <msaboff@apple.com>
+
+        We don't throw SyntaxErrors for runtime generated regular expressions with errors
+        https://bugs.webkit.org/show_bug.cgi?id=185755
+
+        Reviewed by Keith Miller.
+
+        New regression test.
+
+        * stress/regexp-with-runtime-syntax-errors.js: Added.
+        (testThrowsSyntaxtError):
+        (fromExecWithBadUnicodeEscape):
+        (fromTestWithBadUnicodeProperty):
+        (fromSplitWithBadUnicodeIdentity):
+        (fromMatchWithBadUnicodeBackReference):
+        (fromReplaceWithBadUnicodeEscape):
+        (fromSearchWithBadUnicodeEscape):
+
 2018-05-16  Caio Lima  <ticaiolima@gmail.com>
 
         [ESNext][BigInt] Implement support for "/" operation
diff --git a/JSTests/stress/regexp-with-runtime-syntax-errors.js b/JSTests/stress/regexp-with-runtime-syntax-errors.js
new file mode 100644 (file)
index 0000000..3321a31
--- /dev/null
@@ -0,0 +1,66 @@
+// This test checks that malformed regular expressions compiled at runtime throw SyntaxErrors
+
+function testThrowsSyntaxtError(f)
+{
+    try {
+        f();
+    } catch (e) {
+        if (!e instanceof SyntaxError)
+            throw "Expected SynteaxError, but got: " + e;
+    }
+}
+
+function fromExecWithBadUnicodeEscape()
+{
+    let baseRE = /\u{123x}/;
+    let line = "abc";
+
+    (new RegExp(baseRE, "u")).exec(line);
+}
+
+function fromTestWithBadUnicodeProperty()
+{
+    let baseRE = /a|\p{Blah}/;
+    let line = "abc";
+
+    (new RegExp(baseRE, "u")).test(line);
+}
+
+function fromSplitWithBadUnicodeIdentity()
+{
+    let baseRE = /,|:|\-/;
+    let line = "abc:def-ghi";
+
+    let fields = line.split(new RegExp(baseRE, "u"));
+}
+
+function fromMatchWithBadUnicodeBackReference()
+{
+    let baseRE = /\123/;
+    let line = "xyz";
+
+    let fields = line.match(new RegExp(baseRE, "u"));
+}
+
+function fromReplaceWithBadUnicodeEscape()
+{
+    let baseRE = /\%/;
+    let line = "xyz";
+
+    let fields = line.replace(new RegExp(baseRE, "u"), "x");
+}
+
+function fromSearchWithBadUnicodeEscape()
+{
+    let baseRE = /\=/;
+    let line = "xyz";
+
+    let fields = line.search(new RegExp(baseRE, "u"));
+}
+
+testThrowsSyntaxtError(fromExecWithBadUnicodeEscape);
+testThrowsSyntaxtError(fromTestWithBadUnicodeProperty);
+testThrowsSyntaxtError(fromSplitWithBadUnicodeIdentity);
+testThrowsSyntaxtError(fromMatchWithBadUnicodeBackReference);
+testThrowsSyntaxtError(fromReplaceWithBadUnicodeEscape);
+testThrowsSyntaxtError(fromSearchWithBadUnicodeEscape);
index 62b3983..29cac6a 100644 (file)
@@ -1,3 +1,18 @@
+2018-05-17  Michael Saboff  <msaboff@apple.com>
+
+        We don't throw SyntaxErrors for runtime generated regular expressions with errors
+        https://bugs.webkit.org/show_bug.cgi?id=185755
+
+        Reviewed by Keith Miller.
+
+        Updated test and results from reporting a SyntaxError to an Out of memory error.
+
+        * js/script-tests/stack-overflow-regexp.js:
+        (shouldThrow.recursiveCall):
+        (shouldThrow):
+        (recursiveCall):
+        * js/stack-overflow-regexp-expected.txt:
+
 2018-05-17  Nan Wang  <n_wang@apple.com>
 
         AX: [macOS] Expose the primary screen height through AX API
index 914a934..ffd1103 100644 (file)
@@ -1,13 +1,13 @@
 description('Test that we do not overflow the stack while handling regular expressions');
 
 // Base case.
-shouldThrow('new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")"))', '"SyntaxError: Invalid regular expression: too many nested disjunctions"');
+shouldThrow('new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")"))', '"Error: Out of memory: Invalid regular expression: too many nested disjunctions"');
 
 { // Verify that a deep JS stack does not help avoiding the error.
     function recursiveCall(depth) {
         if (!(depth % 10)) {
             debug("Creating RegExp at depth " + depth);
-            shouldThrow('new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")"))', '"SyntaxError: Invalid regular expression: too many nested disjunctions"');
+            shouldThrow('new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")"))', '"Error: Out of memory: Invalid regular expression: too many nested disjunctions"');
         }
         if (depth < 100) {
             recursiveCall(depth + 1);
@@ -25,5 +25,5 @@ shouldThrow('new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")"))',
     for (let i = 0; i < 50000; ++i) {
         expression += ")(c))";
     }
-    shouldThrow('new RegExp(expression)', '"SyntaxError: Invalid regular expression: too many nested disjunctions"');
+    shouldThrow('new RegExp(expression)', '"Error: Out of memory: Invalid regular expression: too many nested disjunctions"');
 }
index 1f9cc0f..c7b5b7f 100644 (file)
@@ -3,30 +3,30 @@ Test that we do not overflow the stack while handling regular expressions
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 0
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 10
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 20
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 30
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 40
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 50
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 60
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 70
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 80
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 90
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 Creating RegExp at depth 100
-PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
-PASS new RegExp(expression) threw exception SyntaxError: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(Array(50000).join("(") + "a" + Array(50000).join(")")) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
+PASS new RegExp(expression) threw exception Error: Out of memory: Invalid regular expression: too many nested disjunctions.
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 4fe30fe..e26319e 100644 (file)
@@ -1,3 +1,25 @@
+2018-05-17  Michael Saboff  <msaboff@apple.com>
+
+        We don't throw SyntaxErrors for runtime generated regular expressions with errors
+        https://bugs.webkit.org/show_bug.cgi?id=185755
+
+        Reviewed by Keith Miller.
+
+        Added a new helper that creates the correct exception to throw for each type of error when
+        compiling a RegExp.  Using that new helper, added missing checks for RegExp for the cases
+        where we create a new RegExp from an existing one.  Also refactored other places that we
+        throw SyntaxErrors after a failed RegExp compile to use the new helper.
+
+        * runtime/RegExp.h:
+        * runtime/RegExpConstructor.cpp:
+        (JSC::regExpCreate):
+        (JSC::constructRegExp):
+        * runtime/RegExpPrototype.cpp:
+        (JSC::regExpProtoFuncCompile):
+        * yarr/YarrErrorCode.cpp:
+        (JSC::Yarr::errorToThrow):
+        * yarr/YarrErrorCode.h:
+
 2018-05-17  Saam Barati  <sbarati@apple.com>
 
         Remove shrinkFootprint test from apitests since it's flaky
index 49abb1a..601f823 100644 (file)
@@ -353,6 +353,14 @@ JSObject* createOutOfMemoryError(ExecState* exec)
     return error;
 }
 
+JSObject* createOutOfMemoryError(ExecState* exec, const String& message)
+{
+    
+    auto* error = createError(exec, makeString("Out of memory: ", message), nullptr);
+    jsCast<ErrorInstance*>(error)->setOutOfMemoryError();
+    return error;
+}
+
 
 const ClassInfo StrictModeTypeErrorFunction::s_info = { "Function", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(StrictModeTypeErrorFunction) };
 
index b7e2692..773cd27 100644 (file)
@@ -71,6 +71,7 @@ JS_EXPORT_PRIVATE JSObject* createTypeError(ExecState*, const String&);
 JS_EXPORT_PRIVATE JSObject* createNotEnoughArgumentsError(ExecState*);
 JS_EXPORT_PRIVATE JSObject* createURIError(ExecState*, const String&);
 JS_EXPORT_PRIVATE JSObject* createOutOfMemoryError(ExecState*);
+JS_EXPORT_PRIVATE JSObject* createOutOfMemoryError(ExecState*, const String&);
 
 JS_EXPORT_PRIVATE JSObject* createError(ExecState*, ErrorType, const String&);
 
index 3191f49..b6c25ab 100644 (file)
@@ -62,6 +62,7 @@ public:
 
     bool isValid() const { return !Yarr::hasError(m_constructionErrorCode) && m_flags != InvalidFlags; }
     const char* errorMessage() const { return Yarr::errorMessage(m_constructionErrorCode); }
+    JSObject* errorToThrow(ExecState* exec) { return Yarr::errorToThrow(exec, m_constructionErrorCode); }
 
     JS_EXPORT_PRIVATE int match(VM&, const String&, unsigned startOffset, Vector<int>& ovector);
 
index 7b93df1..e8d485b 100644 (file)
@@ -243,7 +243,7 @@ static JSObject* regExpCreate(ExecState* exec, JSGlobalObject* globalObject, JSV
 
     RegExp* regExp = RegExp::create(vm, pattern, flags);
     if (!regExp->isValid())
-        return throwException(exec, scope, createSyntaxError(exec, regExp->errorMessage()));
+        return throwException(exec, scope, regExp->errorToThrow(exec));
 
     Structure* structure = getRegExpStructure(exec, globalObject, newTarget);
     RETURN_IF_EXCEPTION(scope, nullptr);
@@ -281,6 +281,9 @@ JSObject* constructRegExp(ExecState* exec, JSGlobalObject* globalObject, const A
             if (flags == InvalidFlags)
                 return nullptr;
             regExp = RegExp::create(vm, regExp->pattern(), flags);
+
+            if (!regExp->isValid())
+                return throwException(exec, scope, regExp->errorToThrow(exec));
         }
 
         return RegExpObject::create(vm, structure, regExp);
index 7631047..a7d9539 100644 (file)
@@ -174,7 +174,7 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncCompile(ExecState* exec)
     }
 
     if (!regExp->isValid())
-        return throwVMError(exec, scope, createSyntaxError(exec, regExp->errorMessage()));
+        return throwVMError(exec, scope, regExp->errorToThrow(exec));
 
     thisRegExp->setRegExp(vm, regExp);
     scope.release();
index b90a081..aaebd46 100644 (file)
@@ -26,6 +26,8 @@
 #include "config.h"
 #include "YarrErrorCode.h"
 
+#include "Error.h"
+
 namespace JSC { namespace Yarr {
 
 const char* errorMessage(ErrorCode error)
@@ -58,4 +60,37 @@ const char* errorMessage(ErrorCode error)
     return errorMessages[static_cast<unsigned>(error)];
 }
 
+JSObject* errorToThrow(ExecState* exec, ErrorCode error)
+{
+    switch (error) {
+    case ErrorCode::NoError:
+        ASSERT_NOT_REACHED();
+        return nullptr;
+    case ErrorCode::PatternTooLarge:
+    case ErrorCode::QuantifierOutOfOrder:
+    case ErrorCode::QuantifierWithoutAtom:
+    case ErrorCode::QuantifierTooLarge:
+    case ErrorCode::MissingParentheses:
+    case ErrorCode::ParenthesesUnmatched:
+    case ErrorCode::ParenthesesTypeInvalid:
+    case ErrorCode::InvalidGroupName:
+    case ErrorCode::DuplicateGroupName:
+    case ErrorCode::CharacterClassUnmatched:
+    case ErrorCode::CharacterClassOutOfOrder:
+    case ErrorCode::EscapeUnterminated:
+    case ErrorCode::InvalidUnicodeEscape:
+    case ErrorCode::InvalidBackreference:
+    case ErrorCode::InvalidIdentityEscape:
+    case ErrorCode::InvalidUnicodePropertyExpression:
+    case ErrorCode::OffsetTooLarge:
+    case ErrorCode::InvalidRegularExpressionFlags:
+        return createSyntaxError(exec, errorMessage(error));
+    case ErrorCode::TooManyDisjunctions:
+        return createOutOfMemoryError(exec, errorMessage(error));
+    }
+
+    ASSERT_NOT_REACHED();
+    return nullptr;
+}
+
 } } // namespace JSC::Yarr
index 0b947b9..344b4be 100644 (file)
 
 #pragma once
 
-namespace JSC { namespace Yarr {
+namespace JSC {
+
+class ExecState;
+class JSObject;
+
+namespace Yarr {
 
 enum class ErrorCode : unsigned {
     NoError = 0,
@@ -55,5 +60,6 @@ inline bool hasError(ErrorCode errorCode)
 {
     return errorCode != ErrorCode::NoError;
 }
+JS_EXPORT_PRIVATE JSObject* errorToThrow(ExecState*, ErrorCode);
 
 } } // namespace JSC::Yarr