Skip to content

Commit

Permalink
String.prototype.matchAll should throw on non-global regex
Browse files Browse the repository at this point in the history
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 tc39/ecma262#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
  • Loading branch information
ross.kirsling@sony.com committed Oct 23, 2019
1 parent 43e0e14 commit 5ea437b
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 6 deletions.
12 changes: 12 additions & 0 deletions 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)
Expand Down
30 changes: 30 additions & 0 deletions 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');
6 changes: 0 additions & 6 deletions JSTests/test262/expectations.yaml
Expand Up @@ -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'
Expand Down
19 changes: 19 additions & 0 deletions 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
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/builtins/BuiltinNames.h
Expand Up @@ -126,6 +126,7 @@ namespace JSC {
macro(concatMemcpy) \
macro(appendMemcpy) \
macro(regExpCreate) \
macro(isRegExp) \
macro(replaceUsingRegExp) \
macro(replaceUsingStringSearch) \
macro(makeTypeError) \
Expand Down
3 changes: 3 additions & 0 deletions Source/JavaScriptCore/builtins/StringPrototype.js
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Expand Up @@ -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),
Expand Down
6 changes: 6 additions & 0 deletions Source/JavaScriptCore/runtime/RegExpConstructor.cpp
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/runtime/RegExpConstructor.h
Expand Up @@ -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

0 comments on commit 5ea437b

Please sign in to comment.