Bug 154817 – [JSC] Private symbols should not be trapped by proxy handler

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Created attachment 272494 [details]
Patch

[Yusuke Suzuki]

Created attachment 272495 [details]
Patch

[Mark Lam]

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.

[Saam Barati]

Comment on attachment 272495 [details]
Patch

We should probably remove the identifierToPublicSafeJSValur

[Yusuke Suzuki]

(In reply to comment #4)
> Comment on attachment 272495 [details]
> Patch
> 
> We should probably remove the identifierToPublicSafeJSValur

OK, I'll remove this :)

[Yusuke Suzuki]

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.

[Saam Barati]

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

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r197383: <http://trac.webkit.org/changeset/197383>