Bug 154385 - Proxy's don't properly handle Symbols as PropertyKeys.
Summary: Proxy's don't properly handle Symbols as PropertyKeys.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-17 23:02 PST by Saam Barati
Modified: 2016-02-18 16:28 PST (History)
11 users (show)

See Also:


Attachments
patch (4.94 KB, patch)
2016-02-17 23:14 PST, Saam Barati
no flags Details | Formatted Diff | Diff
patch (5.77 KB, patch)
2016-02-17 23:42 PST, Saam Barati
mark.lam: review+
Details | Formatted Diff | Diff
patch (6.23 KB, patch)
2016-02-18 11:55 PST, Saam Barati
ysuzuki: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-02-17 23:02:03 PST
...
Comment 1 Saam Barati 2016-02-17 23:14:39 PST
Created attachment 271638 [details]
patch
Comment 2 Saam Barati 2016-02-17 23:42:12 PST
Created attachment 271639 [details]
patch
Comment 3 Mark Lam 2016-02-18 08:50:08 PST
Comment on attachment 271639 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271639&action=review

r=me

> Source/JavaScriptCore/runtime/ProxyObject.cpp:30
> +#include "Identifier.h"

Because you #include "IdentifierInlines.h", you don't need this.  #include "IdentifierInlines.h" implies that you that #include'd "Identifier.h".
Comment 4 Yusuke Suzuki 2016-02-18 08:53:11 PST
Comment on attachment 271639 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271639&action=review

> Source/JavaScriptCore/runtime/ProxyObject.cpp:113
> +    arguments.append(identifierToJSValue(vm, Identifier::fromUid(&vm, propertyName.uid())));

We should avoid leaking private symbols (used for JSC runtime) to users (e.g. @arrayIteratorNextIndex).
It offers an accessability to the private (unsafe) data structures in JSObject.
So if the given properyName is a private symbol, it should not be trapped by Proxies.
Comment 5 Saam Barati 2016-02-18 11:55:32 PST
Created attachment 271678 [details]
patch

updated with Yusuke's comments.
Comment 6 Yusuke Suzuki 2016-02-18 12:24:28 PST
Comment on attachment 271678 [details]
patch

Adding test for this would be nice :D (I think ArrayIterator.prototype.next can be used).
Comment 7 Saam Barati 2016-02-18 12:30:26 PST
(In reply to comment #6)
> Comment on attachment 271678 [details]
> patch
> 
> Adding test for this would be nice :D (I think ArrayIterator.prototype.next
> can be used).

Will add. Thanks
Comment 8 Saam Barati 2016-02-18 16:28:02 PST
landed in:
http://trac.webkit.org/changeset/196785