188874 2018-08-22 17:49:37 -0700 Function object should convert params to string before throw a parsing error 2018-09-01 01:04:17 -0700 1 1 1 Unclassified WebKit JavaScriptCore Safari 11 Unspecified macOS 10.13 RESOLVED FIXED InRadar P2 Normal --- 1 srsj2 ysuzuki darin ews-watchlist fpizlo keith_miller mark.lam msaboff saam webkit-bug-importer ysuzuki oldest_to_newest 1452914 0 srsj2 2018-08-22 17:49:37 -0700 Hi everyone, I found out an inconsistency in JSC when we declare a Function Object with a special characters as params. Version: 235121 O.S: MacOS High Sierra Version 10.13.6 step to reproduce: assertThrowsValue(() => Function("@", { toString() { throw 42; } }), 42); atual result: SyntaxError: Invalid character: '@' expected result: pass without failures V8, Chakra and SpiderMonkey works as expected cinfuzz 1453670 1 348075 ysuzuki 2018-08-24 23:32:42 -0700 Created attachment 348075 Patch 1453699 2 348075 darin 2018-08-25 17:23:50 -0700 Comment on attachment 348075 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348075&action=review > Source/JavaScriptCore/ChangeLog:9 > + ToString operation onto the `body` of the Function constructor should be performed > + before checking syntax correctness of the parameters. I have no doubt that this is better, but I wonder where these semantics are defined and tested unambiguously. There must be lots of other cases where the order of evaluation side effect is visible, and the new test covers just this one. 1454733 3 saam 2018-08-28 23:14:41 -0700 (In reply to Darin Adler from comment #2) > Comment on attachment 348075 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=348075&action=review > > > Source/JavaScriptCore/ChangeLog:9 > > + ToString operation onto the `body` of the Function constructor should be performed > > + before checking syntax correctness of the parameters. > > I have no doubt that this is better, but I wonder where these semantics are > defined and tested unambiguously. There must be lots of other cases where > the order of evaluation side effect is visible, and the new test covers just > this one. It's defined here: https://tc39.github.io/ecma262/#sec-createdynamicfunction I agree that it would be nice to add some more tests that stress the order of evaluation, including the order of toString on the parameters. 1455954 4 348075 ysuzuki 2018-09-01 01:02:58 -0700 Comment on attachment 348075 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348075&action=review Thanks! >>> Source/JavaScriptCore/ChangeLog:9 >>> + before checking syntax correctness of the parameters. >> >> I have no doubt that this is better, but I wonder where these semantics are defined and tested unambiguously. There must be lots of other cases where the order of evaluation side effect is visible, and the new test covers just this one. > > It's defined here: > https://tc39.github.io/ecma262/#sec-createdynamicfunction > > I agree that it would be nice to add some more tests that stress the order of evaluation, including the order of toString on the parameters. I've added the test checking the order of toString onto the arguments of Function constructor. 1455955 5 ysuzuki 2018-09-01 01:03:48 -0700 Committed r235582: <https://trac.webkit.org/changeset/235582> 1455956 6 webkit-bug-importer 2018-09-01 01:04:17 -0700 <rdar://problem/43989360> 348075 2018-08-24 23:32:42 -0700 2018-08-25 17:23:50 -0700 Patch bug-188874-20180825153241.patch text/plain 3545 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjM1MzM4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA0 ZGNmMTZhNDQ3MGZiOWE0NmY5OThlNWEzZDhjYmNlYjNkZDQxNzFlLi5kNmJmNzE0NDk0ZjYyNmJk NTI2NjYyYjBkNzM0MjMzZDY5ZjE4YjNhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNiBAQAorMjAxOC0wOC0yNCAgWXVzdWtlIFN1enVraSAgPHl1c3VrZXN1enVraUBzbG93 c3RhcnQub3JnPgorCisgICAgICAgIEZ1bmN0aW9uIG9iamVjdCBzaG91bGQgY29udmVydCBwYXJh bXMgdG8gc3RyaW5nIGJlZm9yZSB0aHJvdyBhIHBhcnNpbmcgZXJyb3IKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4ODg3NAorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRvU3RyaW5nIG9wZXJhdGlvbiBvbnRv IHRoZSBgYm9keWAgb2YgdGhlIEZ1bmN0aW9uIGNvbnN0cnVjdG9yIHNob3VsZCBiZSBwZXJmb3Jt ZWQKKyAgICAgICAgYmVmb3JlIGNoZWNraW5nIHN5bnRheCBjb3JyZWN0bmVzcyBvZiB0aGUgcGFy YW1ldGVycy4KKworICAgICAgICAqIHJ1bnRpbWUvRnVuY3Rpb25Db25zdHJ1Y3Rvci5jcHA6Cisg ICAgICAgIChKU0M6OmNvbnN0cnVjdEZ1bmN0aW9uU2tpcHBpbmdFdmFsRW5hYmxlZENoZWNrKToK KwogMjAxOC0wOC0yNCAgTWljaGFlbCBTYWJvZmYgIDxtc2Fib2ZmQGFwcGxlLmNvbT4KIAogICAg ICAgICBZQVJSOiBVcGRhdGUgVUNTIGNhbm9uaWNhbGl6YXRpb24gdGFibGVzIGZvciBVbmljb2Rl IDExCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9GdW5jdGlvbkNv bnN0cnVjdG9yLmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0Z1bmN0aW9uQ29u c3RydWN0b3IuY3BwCmluZGV4IDYwMzVkY2ExYTA0N2ViMDhiMzdmNzAzYmM5N2VjNjRmYTZhYzEy YjAuLmZjMzBmY2YzMTBkZjgwOTAxY2MwNmM5YWI5MzI0YTg1YTIxNjRkMzIgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0Z1bmN0aW9uQ29uc3RydWN0b3IuY3BwCisr KyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0Z1bmN0aW9uQ29uc3RydWN0b3IuY3Bw CkBAIC0xMzksNiArMTM5LDggQEAgSlNPYmplY3QqIGNvbnN0cnVjdEZ1bmN0aW9uU2tpcHBpbmdF dmFsRW5hYmxlZENoZWNrKAogICAgICAgICAgICAgUkVUVVJOX0lGX0VYQ0VQVElPTihzY29wZSwg bnVsbHB0cik7CiAgICAgICAgICAgICBwYXJhbWV0ZXJCdWlsZGVyLmFwcGVuZCh2aWV3V2l0aFN0 cmluZy52aWV3KTsKICAgICAgICAgfQorICAgICAgICBhdXRvIGJvZHkgPSBhcmdzLmF0KGFyZ3Mu c2l6ZSgpIC0gMSkudG9XVEZTdHJpbmcoZXhlYyk7CisgICAgICAgIFJFVFVSTl9JRl9FWENFUFRJ T04oc2NvcGUsIG51bGxwdHIpOwogCiAgICAgICAgIHsKICAgICAgICAgICAgIC8vIFRoZSBzcGVj IG1hbmRhdGVzIHRoYXQgdGhlIHBhcmFtZXRlcnMgcGFyc2UgYXMgYSB2YWxpZCBwYXJhbWV0ZXIg bGlzdApAQCAtMTU1LDggKzE1Nyw2IEBAIEpTT2JqZWN0KiBjb25zdHJ1Y3RGdW5jdGlvblNraXBw aW5nRXZhbEVuYWJsZWRDaGVjaygKIAogICAgICAgICBidWlsZGVyLmFwcGVuZChwYXJhbWV0ZXJC dWlsZGVyKTsKICAgICAgICAgYnVpbGRlci5hcHBlbmRMaXRlcmFsKCIpIHtcbiIpOwotICAgICAg ICBhdXRvIGJvZHkgPSBhcmdzLmF0KGFyZ3Muc2l6ZSgpIC0gMSkudG9XVEZTdHJpbmcoZXhlYyk7 Ci0gICAgICAgIFJFVFVSTl9JRl9FWENFUFRJT04oc2NvcGUsIG51bGxwdHIpOwogICAgICAgICBj aGVja0JvZHkoYm9keSk7CiAgICAgICAgIFJFVFVSTl9JRl9FWENFUFRJT04oc2NvcGUsIG51bGxw dHIpOwogICAgICAgICBidWlsZGVyLmFwcGVuZChib2R5KTsKZGlmZiAtLWdpdCBhL0pTVGVzdHMv Q2hhbmdlTG9nIGIvSlNUZXN0cy9DaGFuZ2VMb2cKaW5kZXggYTE5YWI1N2MxOWZlZTU1OTQ1NTU4 MzM5NjBmNmEyNjBmOGEzMTc1NC4uZjU3MjM2ODEzMDMzNDhjMTE0Njg1OGM5MDFkY2M4OWI2MTQ3 YTdlMSAxMDA2NDQKLS0tIGEvSlNUZXN0cy9DaGFuZ2VMb2cKKysrIGIvSlNUZXN0cy9DaGFuZ2VM b2cKQEAgLTEsMyArMSwxMyBAQAorMjAxOC0wOC0yNCAgWXVzdWtlIFN1enVraSAgPHl1c3VrZXN1 enVraUBzbG93c3RhcnQub3JnPgorCisgICAgICAgIEZ1bmN0aW9uIG9iamVjdCBzaG91bGQgY29u dmVydCBwYXJhbXMgdG8gc3RyaW5nIGJlZm9yZSB0aHJvdyBhIHBhcnNpbmcgZXJyb3IKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4ODg3NAorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogc3RyZXNzL2Z1bmN0 aW9uLWJvZHktdG8tc3RyaW5nLWJlZm9yZS1wYXJhbWV0ZXItc3ludGF4LWNoZWNrLmpzOiBBZGRl ZC4KKyAgICAgICAgKHNob3VsZFRocm93KToKKwogMjAxOC0wOC0yMiAgU2FhbSBiYXJhdGkgIDxz YmFyYXRpQGFwcGxlLmNvbT4KIAogICAgICAgICBNYWtlIGRhdGEtdmlldy1hY2Nlc3MuanMgcnVu IGxlc3MgdGltZSB0byBwcmV2ZW50IHRpbWVvdXRzIG9uIDMyLWJpdApkaWZmIC0tZ2l0IGEvSlNU ZXN0cy9zdHJlc3MvZnVuY3Rpb24tYm9keS10by1zdHJpbmctYmVmb3JlLXBhcmFtZXRlci1zeW50 YXgtY2hlY2suanMgYi9KU1Rlc3RzL3N0cmVzcy9mdW5jdGlvbi1ib2R5LXRvLXN0cmluZy1iZWZv cmUtcGFyYW1ldGVyLXN5bnRheC1jaGVjay5qcwpuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi4wOTZjNGQ0ZjIwZTEzYWFl MjFlODNiYWY4Y2M5YmUwODA3Njk4NjM2Ci0tLSAvZGV2L251bGwKKysrIGIvSlNUZXN0cy9zdHJl c3MvZnVuY3Rpb24tYm9keS10by1zdHJpbmctYmVmb3JlLXBhcmFtZXRlci1zeW50YXgtY2hlY2su anMKQEAgLTAsMCArMSwxOCBAQAorZnVuY3Rpb24gc2hvdWxkVGhyb3coZnVuYywgZXJyb3JNZXNz YWdlKSB7CisgICAgdmFyIGVycm9yVGhyb3duID0gZmFsc2U7CisgICAgdmFyIGVycm9yID0gbnVs bDsKKyAgICB0cnkgeworICAgICAgICBmdW5jKCk7CisgICAgfSBjYXRjaCAoZSkgeworICAgICAg ICBlcnJvclRocm93biA9IHRydWU7CisgICAgICAgIGVycm9yID0gZTsKKyAgICB9CisgICAgaWYg KCFlcnJvclRocm93bikKKyAgICAgICAgdGhyb3cgbmV3IEVycm9yKCdub3QgdGhyb3duJyk7Cisg ICAgaWYgKFN0cmluZyhlcnJvcikgIT09IGVycm9yTWVzc2FnZSkKKyAgICAgICAgdGhyb3cgbmV3 IEVycm9yKGBiYWQgZXJyb3I6ICR7U3RyaW5nKGVycm9yKX1gKTsKK30KKworc2hvdWxkVGhyb3co KCkgPT4geworICAgIEZ1bmN0aW9uKCJAIiwgeyB0b1N0cmluZygpIHsgdGhyb3cgNDI7IH0gfSkK K30sIGA0MmApOwo=