Attachment 340670 Details for Bug 185755 – Updated patch with new results for stack-overflow-regexp.js

[patch] Updated patch with new results for stack-overflow-regexp.js

185755-2.patch (text/plain), 17.35 KB, created by Michael Saboff on 2018-05-17 18:32:36 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Saboff

Created: 2018-05-17 18:32:36 PDT

Size: 17.35 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 231936)
>+++ JSTests/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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
>Index: JSTests/stress/regexp-with-runtime-syntax-errors.js
>===================================================================
>--- JSTests/stress/regexp-with-runtime-syntax-errors.js	(nonexistent)
>+++ JSTests/stress/regexp-with-runtime-syntax-errors.js	(working copy)
>@@ -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: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231934)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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>
> 
>         defaultConstructorSourceCode needs to makeSource every time it's called
>Index: Source/JavaScriptCore/runtime/Error.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/Error.cpp	(revision 231934)
>+++ Source/JavaScriptCore/runtime/Error.cpp	(working copy)
>@@ -353,6 +353,14 @@ JSObject* createOutOfMemoryError(ExecSta
>     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: Source/JavaScriptCore/runtime/Error.h
>===================================================================
>--- Source/JavaScriptCore/runtime/Error.h	(revision 231934)
>+++ Source/JavaScriptCore/runtime/Error.h	(working copy)
>@@ -71,6 +71,7 @@ JS_EXPORT_PRIVATE JSObject* createTypeEr
> 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: Source/JavaScriptCore/runtime/RegExp.h
>===================================================================
>--- Source/JavaScriptCore/runtime/RegExp.h	(revision 231934)
>+++ Source/JavaScriptCore/runtime/RegExp.h	(working copy)
>@@ -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: Source/JavaScriptCore/runtime/RegExpConstructor.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/RegExpConstructor.cpp	(revision 231934)
>+++ Source/JavaScriptCore/runtime/RegExpConstructor.cpp	(working copy)
>@@ -243,7 +243,7 @@ static JSObject* regExpCreate(ExecState*
> 
>     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* exe
>             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: Source/JavaScriptCore/runtime/RegExpPrototype.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/RegExpPrototype.cpp	(revision 231934)
>+++ Source/JavaScriptCore/runtime/RegExpPrototype.cpp	(working copy)
>@@ -174,7 +174,7 @@ EncodedJSValue JSC_HOST_CALL regExpProto
>     }
> 
>     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: Source/JavaScriptCore/yarr/YarrErrorCode.cpp
>===================================================================
>--- Source/JavaScriptCore/yarr/YarrErrorCode.cpp	(revision 231934)
>+++ Source/JavaScriptCore/yarr/YarrErrorCode.cpp	(working copy)
>@@ -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: Source/JavaScriptCore/yarr/YarrErrorCode.h
>===================================================================
>--- Source/JavaScriptCore/yarr/YarrErrorCode.h	(revision 231934)
>+++ Source/JavaScriptCore/yarr/YarrErrorCode.h	(working copy)
>@@ -25,7 +25,12 @@
> 
> #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
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 231934)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -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 NOBODY (OOPS!).
>+
>+        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  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r231899.
>Index: LayoutTests/js/stack-overflow-regexp-expected.txt
>===================================================================
>--- LayoutTests/js/stack-overflow-regexp-expected.txt	(revision 231934)
>+++ LayoutTests/js/stack-overflow-regexp-expected.txt	(working copy)
>@@ -3,30 +3,30 @@ Test that we do not overflow the stack w
> 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: LayoutTests/js/script-tests/stack-overflow-regexp.js
>===================================================================
>--- LayoutTests/js/script-tests/stack-overflow-regexp.js	(revision 231934)
>+++ LayoutTests/js/script-tests/stack-overflow-regexp.js	(working copy)
>@@ -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).joi
>     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"');
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 185755: 340668 | 340669 | 340670