Bug 244196
Summary: | String.prototype.includes incorrectly returns false when string is empty and position is past end of string | ||
---|---|---|---|
Product: | WebKit | Reporter: | EntryHi <entryhii> |
Component: | JavaScriptCore | Assignee: | Darin Adler <darin> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | darin, mark.lam, saam, webkit-bug-importer, ysuzuki |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Local Build | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=144314 |
EntryHi
function foo(n) {
return ("aaa").includes("", n)
}
noInline(foo);
for (var m = 0; m < 200; ++m) {
print(foo(m));
}
With the above script as input to JSC, run JSC with the following parameters:
./jsc test.js --useConcurrentJIT=0 --jitPolicyScale=1
According to ECMAScript Language Specification, when the first parameter of String.prototype.includes is empty, the result should be true regardless of whether the second parameter is greater than length. However, JSC prints false when n exceeds length.
The problem is in StringPrototype.cpp stringIncluesImpl. When positionArg is Int32, it should be clamped. Otherwise, the function findCommon will return notFound and the final result will be false.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Alexey Proskuryakov
On macOS, this prints 4x "true" and 196x "false".
Darin Adler
The suggested change sounds OK. There are other ways to fix it as well. Makes sense that JavaScript calls for this behavior, since it's consistent with indexOf.
I am surprised this is not covered by Test262. Maybe this function is too new?
It looks like the startsWith and endsWith implementations may have a similar bug.
Radar WebKit Bug Importer
<rdar://problem/99271518>
Darin Adler
Pull request: https://github.com/WebKit/WebKit/pull/4180
EWS
Committed 254319@main (ebf196ed5724): <https://commits.webkit.org/254319@main>
Reviewed commits have been landed. Closing PR #4180 and removing active labels.