Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Array proxies as keys? #309

Closed
inexorabletash opened this issue Nov 20, 2019 · 2 comments · Fixed by #311
Closed

Array proxies as keys? #309

inexorabletash opened this issue Nov 20, 2019 · 2 comments · Fixed by #311
Assignees
@inexorabletash
Labels
needs-tests
Milestone
V3

Comments

@inexorabletash
Copy link
Member

I noticed that this throws in both Chrome and Firefox:

const k = [1,2,3];
const p = new Proxy(k, {});
indexedDB.cmp(k, p);

In Chrome, at least, the proxy fails an internal "is this an array?" check.

In the spec convert a value to a key calls ECMAScript's IsArray which would seem to return true for array proxies.

At the very least, we don't have tests.

Should we update the spec to exclude proxies, e.g. replacing "if IsArray(input)" with "if input is an Array exotic object"?

Alternately, should array proxies be supported? What are the consequences of allowing these? Would the value→key conversion algorithm potentially blow up in more places?
@inexorabletash inexorabletash added this to the V3 milestone Nov 20, 2019
@asutherland
Copy link
Collaborator

The algorithms where we clone a value already forbid proxies thanks to step 23 of StructuredSerializeInternal (https://html.spec.whatwg.org/#structuredserializeinternal) which prevents use of proxies.

It makes sense to me to also forbid proxies where we directly invoke algorithms without first performing the clone step.

(Also, Firefox/Gecko is a hard no on supporting proxies in IndexedDB were that not already the case!)

@inexorabletash
Copy link
Member Author

SGTM. I'll put up a spec PR and wpt PR, time permitting, changing the text to the exotic Array object check.

@inexorabletash inexorabletash self-assigned this Nov 20, 2019
This was referenced Nov 20, 2019
Merged
Merged
inexorabletash added a commit to web-platform-tests/wpt that referenced this issue Nov 21, 2019
@inexorabletash
Indexed DB: Verify array proxies are not valid keys (#20352)
c655190 
Arrays of keys are valid keys, but implementations don't allow proxies of arrays to be keys. Add a test, corresponding to pending spec PR:

Bug: w3c/IndexedDB#309
PR: w3c/IndexedDB#311
inexorabletash added a commit that referenced this issue Nov 22, 2019
@inexorabletash
Disallow proxies during key conversion
7d425a9 
All engines disallow using proxies when specifying keys. Update spec text to reflect this.

Closes #309
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 29, 2019
@inexorabletash @moz-wptsync-bot
Bug 1598170 [wpt PR 20352] - Indexed DB: Verify array proxies are not…
832e08b 
… valid keys, a=testonly

Automatic update from web-platform-tests
Indexed DB: Verify array proxies are not valid keys (#20352)

Arrays of keys are valid keys, but implementations don't allow proxies of arrays to be keys. Add a test, corresponding to pending spec PR:

Bug: w3c/IndexedDB#309
PR: w3c/IndexedDB#311

--

wpt-commits: c65519066cf95e902607cce03a9057b859a50242
wpt-pr: 20352
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Nov 29, 2019
@inexorabletash @moz-wptsync-bot
Bug 1598170 [wpt PR 20352] - Indexed DB: Verify array proxies are not…
d3f67e9 
… valid keys, a=testonly

Automatic update from web-platform-tests
Indexed DB: Verify array proxies are not valid keys (#20352)

Arrays of keys are valid keys, but implementations don't allow proxies of arrays to be keys. Add a test, corresponding to pending spec PR:

Bug: w3c/IndexedDB#309
PR: w3c/IndexedDB#311

--

wpt-commits: c65519066cf95e902607cce03a9057b859a50242
wpt-pr: 20352
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Nov 30, 2019
@marco-c
Bug 1598170 [wpt PR 20352] - Indexed DB: Verify array proxies are not…
027b3d9 
… valid keys, a=testonly

Automatic update from web-platform-tests
Indexed DB: Verify array proxies are not valid keys (#20352)

Arrays of keys are valid keys, but implementations don't allow proxies of arrays to be keys. Add a test, corresponding to pending spec PR:

Bug: w3c/IndexedDB#309
PR: w3c/IndexedDB#311

--

wpt-commits: c65519066cf95e902607cce03a9057b859a50242
wpt-pr: 20352

UltraBlame original commit: a1415e648ad42f37c59425a107c33728af78cfb1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Nov 30, 2019
@marco-c
Bug 1598170 [wpt PR 20352] - Indexed DB: Verify array proxies are not…
daf983e 
… valid keys, a=testonly

Automatic update from web-platform-tests
Indexed DB: Verify array proxies are not valid keys (#20352)

Arrays of keys are valid keys, but implementations don't allow proxies of arrays to be keys. Add a test, corresponding to pending spec PR:

Bug: w3c/IndexedDB#309
PR: w3c/IndexedDB#311

--

wpt-commits: c65519066cf95e902607cce03a9057b859a50242
wpt-pr: 20352

UltraBlame original commit: a1415e648ad42f37c59425a107c33728af78cfb1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Nov 30, 2019
@marco-c
Bug 1598170 [wpt PR 20352] - Indexed DB: Verify array proxies are not…
37280ba 
… valid keys, a=testonly

Automatic update from web-platform-tests
Indexed DB: Verify array proxies are not valid keys (#20352)

Arrays of keys are valid keys, but implementations don't allow proxies of arrays to be keys. Add a test, corresponding to pending spec PR:

Bug: w3c/IndexedDB#309
PR: w3c/IndexedDB#311

--

wpt-commits: c65519066cf95e902607cce03a9057b859a50242
wpt-pr: 20352

UltraBlame original commit: a1415e648ad42f37c59425a107c33728af78cfb1
@inexorabletash inexorabletash mentioned this issue Sep 28, 2020
Closed
inexorabletash added a commit that referenced this issue Feb 1, 2021
@inexorabletash
Disallow proxies during key conversion
e71f613 
All engines disallow using proxies when specifying keys. Update spec text to reflect this.

Closes #309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@inexorabletash inexorabletash

Labels
needs-tests
Projects
None yet
Milestone
V3
Development

Successfully merging a pull request may close this issue.

Disallow proxies during key conversion w3c/IndexedDB
2 participants
@asutherland @inexorabletash