From 5ea437bb97c04bc7d07290a63a84a26832e8ee2e Mon Sep 17 00:00:00 2001
From: "ross.kirsling@sony.com"
 <ross.kirsling@sony.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed, 23 Oct 2019 18:45:37 +0000
Subject: [PATCH] String.prototype.matchAll should throw on non-global regex
 https://bugs.webkit.org/show_bug.cgi?id=202838

Reviewed by Keith Miller.

JSTests:

* stress/string-matchall.js: Added.

* test262/expectations.yaml:
Mark four test cases as passing.

Source/JavaScriptCore:

* builtins/StringPrototype.js:
(matchAll):
Implement normative change from https://github.com/tc39/ecma262/pull/1716.

* builtins/BuiltinNames.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
* runtime/RegExpConstructor.cpp:
(JSC::esSpecIsRegExp): Added.
* runtime/RegExpConstructor.h:
Expose isRegExp to builtins. (This differs from @isRegExpObject by first checking for Symbol.match.)


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251483 268f45cc-cd09-0410-ab3c-d52691b4dbfc
---
 JSTests/ChangeLog                             | 12 ++++++++
 JSTests/stress/string-matchall.js             | 30 +++++++++++++++++++
 JSTests/test262/expectations.yaml             |  6 ----
 Source/JavaScriptCore/ChangeLog               | 19 ++++++++++++
 Source/JavaScriptCore/builtins/BuiltinNames.h |  1 +
 .../builtins/StringPrototype.js               |  3 ++
 .../JavaScriptCore/runtime/JSGlobalObject.cpp |  1 +
 .../runtime/RegExpConstructor.cpp             |  6 ++++
 .../runtime/RegExpConstructor.h               |  1 +
 9 files changed, 73 insertions(+), 6 deletions(-)
 create mode 100644 JSTests/stress/string-matchall.js

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 52368a60c8390..ce9881ef236de 100644
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,15 @@
+2019-10-23  Ross Kirsling  <ross.kirsling@sony.com>
+
+        String.prototype.matchAll should throw on non-global regex
+        https://bugs.webkit.org/show_bug.cgi?id=202838
+
+        Reviewed by Keith Miller.
+
+        * stress/string-matchall.js: Added.
+
+        * test262/expectations.yaml:
+        Mark four test cases as passing.
+
 2019-10-23  Ross Kirsling  <ross.kirsling@sony.com>
 
         Update test262 (2019.10.11)
diff --git a/JSTests/stress/string-matchall.js b/JSTests/stress/string-matchall.js
new file mode 100644
index 0000000000000..3d55ebadd6fc0
--- /dev/null
+++ b/JSTests/stress/string-matchall.js
@@ -0,0 +1,30 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error(`expected ${expected} but got ${actual}`);
+}
+
+function shouldNotThrow(func) {
+  func();
+}
+
+function shouldThrowTypeError(func) {
+    let error;
+    try {
+        func();
+    } catch (e) {
+        error = e;
+    }
+
+    if (!(error instanceof TypeError))
+        throw new Error('Expected TypeError!');
+}
+
+shouldThrowTypeError(() => { 'abaca'.matchAll(/a/); });
+shouldThrowTypeError(() => { 'abaca'.matchAll(new RegExp('a')); });
+shouldThrowTypeError(() => { 'abaca'.matchAll({ [Symbol.match]() {} }); });
+
+shouldNotThrow(() => { 'abaca'.matchAll({ [Symbol.match]() {}, flags: 'g' }); });
+
+shouldBe([...'abaca'.matchAll(/a/g)].join(), 'a,a,a');
+shouldBe([...'abaca'.matchAll(new RegExp('a', 'g'))].join(), 'a,a,a');
+shouldBe([...'abaca'.matchAll({ [Symbol.matchAll]: RegExp.prototype[Symbol.matchAll].bind(/a/g) })].join(), 'a,a,a');
diff --git a/JSTests/test262/expectations.yaml b/JSTests/test262/expectations.yaml
index 8893589c5d03d..ba86019315022 100644
--- a/JSTests/test262/expectations.yaml
+++ b/JSTests/test262/expectations.yaml
@@ -1804,12 +1804,6 @@ test/built-ins/Set/proto-from-ctor-realm.js:
 test/built-ins/String/proto-from-ctor-realm.js:
   default: 'Test262Error: Expected SameValue(«», «») to be true'
   strict mode: 'Test262Error: Expected SameValue(«», «») to be true'
-test/built-ins/String/prototype/matchAll/flags-nonglobal-throws.js:
-  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
-  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
-test/built-ins/String/prototype/matchAll/flags-undefined-throws.js:
-  default: 'Test262Error: Expected a TypeError but got a SyntaxError'
-  strict mode: 'Test262Error: Expected a TypeError but got a SyntaxError'
 test/built-ins/ThrowTypeError/extensible.js:
   default: 'Test262Error: Expected SameValue(«true», «false») to be true'
   strict mode: 'Test262Error: Expected SameValue(«true», «false») to be true'
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index dfd73de88a61e..3f4e85fa1da8d 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,22 @@
+2019-10-23  Ross Kirsling  <ross.kirsling@sony.com>
+
+        String.prototype.matchAll should throw on non-global regex
+        https://bugs.webkit.org/show_bug.cgi?id=202838
+
+        Reviewed by Keith Miller.
+
+        * builtins/StringPrototype.js:
+        (matchAll):
+        Implement normative change from https://github.com/tc39/ecma262/pull/1716.
+
+        * builtins/BuiltinNames.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init):
+        * runtime/RegExpConstructor.cpp:
+        (JSC::esSpecIsRegExp): Added.
+        * runtime/RegExpConstructor.h:
+        Expose isRegExp to builtins. (This differs from @isRegExpObject by first checking for Symbol.match.)
+
 2019-10-23  Sihui Liu  <sihui_liu@apple.com>
 
         [ Mac WK1 ] REGRESSION (r251261): Layout Test inspector/console/webcore-logging.html is consistently Failing
diff --git a/Source/JavaScriptCore/builtins/BuiltinNames.h b/Source/JavaScriptCore/builtins/BuiltinNames.h
index 48806592660df..dd33ca8036a30 100644
--- a/Source/JavaScriptCore/builtins/BuiltinNames.h
+++ b/Source/JavaScriptCore/builtins/BuiltinNames.h
@@ -126,6 +126,7 @@ namespace JSC {
     macro(concatMemcpy) \
     macro(appendMemcpy) \
     macro(regExpCreate) \
+    macro(isRegExp) \
     macro(replaceUsingRegExp) \
     macro(replaceUsingStringSearch) \
     macro(makeTypeError) \
diff --git a/Source/JavaScriptCore/builtins/StringPrototype.js b/Source/JavaScriptCore/builtins/StringPrototype.js
index 274812ff72c41..da8a37e2bc8a8 100644
--- a/Source/JavaScriptCore/builtins/StringPrototype.js
+++ b/Source/JavaScriptCore/builtins/StringPrototype.js
@@ -51,6 +51,9 @@ function matchAll(arg)
         @throwTypeError("String.prototype.matchAll requires |this| not to be null nor undefined");
 
     if (!@isUndefinedOrNull(arg)) {
+        if (@isRegExp(arg) && !@stringIncludesInternal.@call(@toString(arg.flags), "g"))
+            @throwTypeError("String.prototype.matchAll argument must not be a non-global regular expression");
+
         let matcher = arg.@matchAllSymbol;
         if (!@isUndefinedOrNull(matcher))
             return matcher.@call(arg, this);
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
index 63e3c86c1e8dd..776c02fafc1b5 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -1036,6 +1036,7 @@ capitalName ## Constructor* lowerName ## Constructor = featureFlag ? capitalName
         // RegExp.prototype helpers.
         GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpBuiltinExecPrivateName(), builtinRegExpExec, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpCreatePrivateName(), JSFunction::create(vm, this, 2, String(), esSpecRegExpCreate, NoIntrinsic), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
+        GlobalPropertyInfo(vm.propertyNames->builtinNames().isRegExpPrivateName(), JSFunction::create(vm, this, 1, String(), esSpecIsRegExp, NoIntrinsic), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpMatchFastPrivateName(), JSFunction::create(vm, this, 1, String(), regExpProtoFuncMatchFast, RegExpMatchFastIntrinsic), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpSearchFastPrivateName(), JSFunction::create(vm, this, 1, String(), regExpProtoFuncSearchFast), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpSplitFastPrivateName(), JSFunction::create(vm, this, 2, String(), regExpProtoFuncSplitFast), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
diff --git a/Source/JavaScriptCore/runtime/RegExpConstructor.cpp b/Source/JavaScriptCore/runtime/RegExpConstructor.cpp
index 463beca42bdc9..2a0cf710b1161 100644
--- a/Source/JavaScriptCore/runtime/RegExpConstructor.cpp
+++ b/Source/JavaScriptCore/runtime/RegExpConstructor.cpp
@@ -277,6 +277,12 @@ EncodedJSValue JSC_HOST_CALL esSpecRegExpCreate(JSGlobalObject* globalObject, Ca
     return JSValue::encode(regExpCreate(globalObject, jsUndefined(), patternArg, flagsArg));
 }
 
+EncodedJSValue JSC_HOST_CALL esSpecIsRegExp(JSGlobalObject* globalObject, CallFrame* callFrame)
+{
+    VM& vm = globalObject->vm();
+    return JSValue::encode(jsBoolean(isRegExp(vm, globalObject, callFrame->argument(0))));
+}
+
 static EncodedJSValue JSC_HOST_CALL constructWithRegExpConstructor(JSGlobalObject* globalObject, CallFrame* callFrame)
 {
     ArgList args(callFrame);
diff --git a/Source/JavaScriptCore/runtime/RegExpConstructor.h b/Source/JavaScriptCore/runtime/RegExpConstructor.h
index 5f2ee4183712e..0fe61015a13e6 100644
--- a/Source/JavaScriptCore/runtime/RegExpConstructor.h
+++ b/Source/JavaScriptCore/runtime/RegExpConstructor.h
@@ -76,5 +76,6 @@ ALWAYS_INLINE bool isRegExp(VM& vm, JSGlobalObject* globalObject, JSValue value)
 }
 
 EncodedJSValue JSC_HOST_CALL esSpecRegExpCreate(JSGlobalObject*, CallFrame*);
+EncodedJSValue JSC_HOST_CALL esSpecIsRegExp(JSGlobalObject*, CallFrame*);
 
 } // namespace JSC