Summary: | Proxy's don't properly handle Symbols as PropertyKeys. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||
Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | benjamin, commit-queue, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, oliver, sukolsak, ysuzuki | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Saam Barati
2016-02-17 23:02:03 PST
Created attachment 271638 [details]
patch
Created attachment 271639 [details]
patch
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 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. Created attachment 271678 [details]
patch
updated with Yusuke's comments.
Comment on attachment 271678 [details]
patch
Adding test for this would be nice :D (I think ArrayIterator.prototype.next can be used).
(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 landed in: http://trac.webkit.org/changeset/196785 |