| Summary: | [JSC] Private symbols should not be trapped by proxy handler | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||
| Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | benjamin, commit-queue, fpizlo, ggaren, keith_miller, mark.lam, msaboff, saam | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
Created attachment 272494 [details]
Patch
Created attachment 272495 [details]
Patch
Comment on attachment 272495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272495&action=review r=me > Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:25 > + // this will throw because we conver private symbols to strings. typo: conver ==> convert > Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:51 > + // this will throw because we conver private symbols to strings. Ditto. Typo: conver ==> convert. > Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:65 > + set: function(theTarget, propName, value, reciever) { typo: reciever => receiver. > Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:78 > + // this will throw because we conver private symbols to strings. Ditto. Comment on attachment 272495 [details]
Patch
We should probably remove the identifierToPublicSafeJSValur
(In reply to comment #4) > Comment on attachment 272495 [details] > Patch > > We should probably remove the identifierToPublicSafeJSValur OK, I'll remove this :) Comment on attachment 272495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272495&action=review Thanks! >> Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:25 >> + // this will throw because we conver private symbols to strings. > > typo: conver ==> convert Fixed. >> Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:51 >> + // this will throw because we conver private symbols to strings. > > Ditto. Typo: conver ==> convert. Fixed. >> Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:65 >> + set: function(theTarget, propName, value, reciever) { > > typo: reciever => receiver. Fixed, thanks! >> Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:78 >> + // this will throw because we conver private symbols to strings. > > Ditto. Fixed. Comment on attachment 272495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272495&action=review >> Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:25 >> + // this will throw because we conver private symbols to strings. > > typo: conver ==> convert It's probably worth asserting on the error message here Comment on attachment 272495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272495&action=review Thanks! >>>> Source/JavaScriptCore/tests/stress/proxy-with-private-symbols.js:25 >>>> + // this will throw because we conver private symbols to strings. >>> >>> typo: conver ==> convert >> >> Fixed. > > It's probably worth asserting on the error message here Nice! Added. Committed r197383: <http://trac.webkit.org/changeset/197383> |
Since the runtime has some assumptions on the properties bound with private symbols, ES6 Proxy should not trap these property operations. For example, in ArrayIteratorPrototype.js var itemKind = this.@arrayIterationKind; if (itemKind === @undefined) throw new @TypeError("%ArrayIteratorPrototype%.next requires that |this| be an Array Iterator instance"); Here, we assume that only the array iterator has @arrayIterationKind property that value is non-undefined. But If we implement Proxy with the get handler, that returns non-undefined value for every operations, we accidentally assumes that the given value is an array iterator. To avoid these situation, we perform the default operations onto property ops with private symbols.