Closed Bug 1389752 Opened 3 years ago Closed 3 years ago

Throw TypeError if proxy [[OwnPropertyKeys]] returns duplicate entries

Categories

(Core :: JavaScript: Standard Library, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: evilpie, Assigned: anba)

References

()

Details

Attachments

(2 files)

This semantically reverts bug 1293995, which wasn't a very good idea. We have to throw an error after CreateFilteredListFromArrayLike, so maybe we should modify that function to use an additional HashSet.
Attached patch bug1389752.patchSplinter Review
This merges step 9 and step 18, so we can directly detect duplicates when creating the |uncheckedResultKeys| set.
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Attachment #8900488 - Flags: review?(till)
Comment on attachment 8900488 [details] [diff] [review]
bug1389752.patch

Review of attachment 8900488 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, this shouldn't even make Proxy.[[OwnPropertyKeys]]() all that much slower.
Attachment #8900488 - Flags: review?(till) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/57ec52606739
Throw TypeError if [[OwnPropertyKeys]] of scripted proxies contains duplicates. r=till
Keywords: checkin-needed
Backed out for failing web-platform-tests' /fetch/api/headers/headers-record.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/030e166c812c60064543baa102b8011f31684a5f

Push which ran failing tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c216b1ee565e736235b57e60c0754331af3f20e0&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=125901705&repo=mozilla-inbound

[task 2017-08-25T13:08:14.698872Z] 13:08:14     INFO - TEST-PASS | /fetch/api/headers/headers-record.html | Correct operation ordering with non-enumerable properties 
[task 2017-08-25T13:08:14.699104Z] 13:08:14     INFO - TEST-PASS | /fetch/api/headers/headers-record.html | Correct operation ordering with undefined descriptors 
[task 2017-08-25T13:08:14.699413Z] 13:08:14     INFO - TEST-UNEXPECTED-FAIL | /fetch/api/headers/headers-record.html | Correct operation ordering with repeated keys - proxy [[OwnPropertyKeys]] can't report property '"a"' more than once
[task 2017-08-25T13:08:14.699684Z] 13:08:14     INFO - loggingHandler[prop]@http://web-platform.test:8000/fetch/api/headers/headers-record.html:22:28
[task 2017-08-25T13:08:14.699911Z] 13:08:14     INFO - @http://web-platform.test:8000/fetch/api/headers/headers-record.html:275:11
[task 2017-08-25T13:08:14.700207Z] 13:08:14     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1441:20
[task 2017-08-25T13:08:14.700393Z] 13:08:14     INFO - test@http://web-platform.test:8000/resources/testharness.js:501:9
[task 2017-08-25T13:08:14.700714Z] 13:08:14     INFO - @http://web-platform.test:8000/fetch/api/headers/headers-record.html:247:1
[task 2017-08-25T13:08:14.701252Z] 13:08:14     INFO - ..
Flags: needinfo?(andrebargull)
With the new Proxy [[OwnPropertyKeys]] restriction to disallow duplicate property keys, large parts of the test are no longer valid. There are only three things we can test now:
- The [[Get]] trap for @@iterator is called
- The [[OwnPropertyKeys]] is called
- And a TypeError is thrown
Flags: needinfo?(andrebargull)
Attachment #8901759 - Flags: review?(kyle)
Attachment #8901759 - Flags: review?(kyle) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9affea4e745f
Throw TypeError if [[OwnPropertyKeys]] of scripted proxies contains duplicates. r=till, r=qdot
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9affea4e745f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Duplicate of this bug: 1458628
You need to log in before you can comment on or make changes to this bug.