- Revision
- 196785
- Author
- sbar...@apple.com
- Date
- 2016-02-18 16:27:15 -0800 (Thu, 18 Feb 2016)
Log Message
Proxy's don't properly handle Symbols as PropertyKeys.
https://bugs.webkit.org/show_bug.cgi?id=154385
Reviewed by Mark Lam and Yusuke Suzuki.
We were converting all PropertyKeys to strings, even when
the PropertyName was a Symbol. In the spec, PropertyKeys are
either a Symbol or a String. We now respect that in Proxy.[[Get]] and
Proxy.[[GetOwnProperty]].
* runtime/Completion.cpp:
(JSC::profiledEvaluate):
(JSC::createSymbolForEntryPointModule):
(JSC::identifierToJSValue): Deleted.
* runtime/Identifier.h:
(JSC::parseIndex):
* runtime/IdentifierInlines.h:
(JSC::Identifier::fromString):
(JSC::identifierToJSValue):
(JSC::identifierToSafePublicJSValue):
* runtime/ProxyObject.cpp:
(JSC::performProxyGet):
(JSC::ProxyObject::performInternalMethodGetOwnProperty):
* tests/es6.yaml:
* tests/stress/proxy-basic.js:
(let.handler.getOwnPropertyDescriptor):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (196784 => 196785)
--- trunk/Source/_javascript_Core/ChangeLog 2016-02-19 00:03:58 UTC (rev 196784)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-02-19 00:27:15 UTC (rev 196785)
@@ -1,5 +1,34 @@
2016-02-18 Saam Barati <sbar...@apple.com>
+ Proxy's don't properly handle Symbols as PropertyKeys.
+ https://bugs.webkit.org/show_bug.cgi?id=154385
+
+ Reviewed by Mark Lam and Yusuke Suzuki.
+
+ We were converting all PropertyKeys to strings, even when
+ the PropertyName was a Symbol. In the spec, PropertyKeys are
+ either a Symbol or a String. We now respect that in Proxy.[[Get]] and
+ Proxy.[[GetOwnProperty]].
+
+ * runtime/Completion.cpp:
+ (JSC::profiledEvaluate):
+ (JSC::createSymbolForEntryPointModule):
+ (JSC::identifierToJSValue): Deleted.
+ * runtime/Identifier.h:
+ (JSC::parseIndex):
+ * runtime/IdentifierInlines.h:
+ (JSC::Identifier::fromString):
+ (JSC::identifierToJSValue):
+ (JSC::identifierToSafePublicJSValue):
+ * runtime/ProxyObject.cpp:
+ (JSC::performProxyGet):
+ (JSC::ProxyObject::performInternalMethodGetOwnProperty):
+ * tests/es6.yaml:
+ * tests/stress/proxy-basic.js:
+ (let.handler.getOwnPropertyDescriptor):
+
+2016-02-18 Saam Barati <sbar...@apple.com>
+
Follow up fix to Implement Proxy.[[GetOwnProperty]]
https://bugs.webkit.org/show_bug.cgi?id=154314
Modified: trunk/Source/_javascript_Core/runtime/Completion.cpp (196784 => 196785)
--- trunk/Source/_javascript_Core/runtime/Completion.cpp 2016-02-19 00:03:58 UTC (rev 196784)
+++ trunk/Source/_javascript_Core/runtime/Completion.cpp 2016-02-19 00:27:15 UTC (rev 196785)
@@ -27,6 +27,7 @@
#include "CodeProfiling.h"
#include "Debugger.h"
#include "Exception.h"
+#include "IdentifierInlines.h"
#include "Interpreter.h"
#include "JSCInlines.h"
#include "JSGlobalObject.h"
@@ -120,13 +121,6 @@
return evaluate(exec, source, thisValue, returnedException);
}
-static JSValue identifierToJSValue(VM& vm, const Identifier& identifier)
-{
- if (identifier.isSymbol())
- return Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()));
- return jsString(&vm, identifier.impl());
-}
-
static Symbol* createSymbolForEntryPointModule(VM& vm)
{
// Generate the unique key for the source-provided module.
Modified: trunk/Source/_javascript_Core/runtime/Identifier.h (196784 => 196785)
--- trunk/Source/_javascript_Core/runtime/Identifier.h 2016-02-19 00:03:58 UTC (rev 196784)
+++ trunk/Source/_javascript_Core/runtime/Identifier.h 2016-02-19 00:27:15 UTC (rev 196785)
@@ -282,6 +282,11 @@
return parseIndex(*uid);
}
+JSValue identifierToJSValue(VM&, const Identifier&);
+// This will stringify private symbols. When leaking JSValues to
+// non-internal code, make sure to use this function and not the above one.
+JSValue identifierToSafePublicJSValue(VM&, const Identifier&);
+
// FIXME: It may be better for this to just be a typedef for PtrHash, since PtrHash may be cheaper to
// compute than loading the StringImpl's hash from memory. That change would also reduce the likelihood of
// crashes in code that somehow dangled a StringImpl.
Modified: trunk/Source/_javascript_Core/runtime/IdentifierInlines.h (196784 => 196785)
--- trunk/Source/_javascript_Core/runtime/IdentifierInlines.h 2016-02-19 00:03:58 UTC (rev 196784)
+++ trunk/Source/_javascript_Core/runtime/IdentifierInlines.h 2016-02-19 00:27:15 UTC (rev 196785)
@@ -28,6 +28,7 @@
#include "CallFrame.h"
#include "Identifier.h"
+#include "Symbol.h"
namespace JSC {
@@ -134,6 +135,20 @@
return Identifier(exec, AtomicString(s));
}
+inline JSValue identifierToJSValue(VM& vm, const Identifier& identifier)
+{
+ if (identifier.isSymbol())
+ return Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()));
+ return jsString(&vm, identifier.impl());
+}
+
+inline JSValue identifierToSafePublicJSValue(VM& vm, const Identifier& identifier)
+{
+ if (identifier.isSymbol() && !vm.propertyNames->isPrivateName(identifier))
+ return Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()));
+ return jsString(&vm, identifier.impl());
+}
+
} // namespace JSC
#endif // IdentifierInlines_h
Modified: trunk/Source/_javascript_Core/runtime/ProxyObject.cpp (196784 => 196785)
--- trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2016-02-19 00:03:58 UTC (rev 196784)
+++ trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2016-02-19 00:27:15 UTC (rev 196785)
@@ -104,7 +104,7 @@
MarkedArgumentBuffer arguments;
arguments.append(target);
- arguments.append(jsString(exec, propertyName.uid()));
+ arguments.append(identifierToSafePublicJSValue(vm, Identifier::fromUid(&vm, propertyName.uid())));
if (exec->hadException())
return JSValue::encode(jsUndefined());
arguments.append(thisObject);
@@ -151,7 +151,7 @@
MarkedArgumentBuffer arguments;
arguments.append(target);
- arguments.append(jsString(exec, propertyName.uid()));
+ arguments.append(identifierToSafePublicJSValue(vm, Identifier::fromUid(&vm, propertyName.uid())));
if (exec->hadException())
return false;
JSValue trapResult = call(exec, getOwnPropertyDescriptorMethod, callType, callData, handler, arguments);
Modified: trunk/Source/_javascript_Core/tests/es6.yaml (196784 => 196785)
--- trunk/Source/_javascript_Core/tests/es6.yaml 2016-02-19 00:03:58 UTC (rev 196784)
+++ trunk/Source/_javascript_Core/tests/es6.yaml 2016-02-19 00:27:15 UTC (rev 196785)
@@ -949,7 +949,7 @@
- path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.unshift.js
cmd: runES6 :fail
- path: es6/Proxy_internal_get_calls_Array.from.js
- cmd: runES6 :fail
+ cmd: runES6 :normal
- path: es6/Proxy_internal_get_calls_Array.prototype.concat.js
cmd: runES6 :fail
- path: es6/Proxy_internal_get_calls_Array.prototype.pop.js
Modified: trunk/Source/_javascript_Core/tests/stress/proxy-basic.js (196784 => 196785)
--- trunk/Source/_javascript_Core/tests/stress/proxy-basic.js 2016-02-19 00:03:58 UTC (rev 196784)
+++ trunk/Source/_javascript_Core/tests/stress/proxy-basic.js 2016-02-19 00:27:15 UTC (rev 196785)
@@ -209,3 +209,76 @@
}
}
}
+
+{
+ let field = Symbol();
+ let theTarget = {
+ [field]: 40
+ };
+ let handler = {
+ get: function(target, propName, proxyArg) {
+ assert(propName === field);
+ return target[field];
+ }
+ };
+
+ let proxy = new Proxy(theTarget, handler);
+ for (let i = 0; i < 500; i++) {
+ assert(proxy[field] === 40);
+ }
+}
+
+{
+ let theTarget = [];
+ let sawPrivateSymbolAsString = false;
+ let handler = {
+ get: function(target, propName, proxyArg) {
+ if (typeof propName === "string")
+ sawPrivateSymbolAsString = propName === "PrivateSymbol.arrayIterationKind";
+ return target[propName];
+ }
+ };
+
+ let proxy = new Proxy(theTarget, handler);
+ for (let i = 0; i < 100; i++) {
+ let threw = false;
+ try {
+ proxy[Symbol.iterator]().next.call(proxy);
+ } catch(e) {
+ // this will throw because we conver private symbols to strings.
+ threw = true;
+ }
+ assert(threw);
+ assert(sawPrivateSymbolAsString);
+ sawPrivateSymbolAsString = false;
+ }
+}
+
+{
+ let prop = Symbol();
+ let theTarget = { };
+ Object.defineProperty(theTarget, prop, {
+ enumerable: true,
+ configurable: true
+ });
+ let called = false;
+ let handler = {
+ getOwnPropertyDescriptor: function(target, propName) {
+ assert(prop === propName);
+ called = true;
+ return {
+ enumerable: true,
+ configurable: true
+ };
+ }
+ };
+
+ let proxy = new Proxy(theTarget, handler);
+ for (let i = 0; i < 100; i++) {
+ let pDesc = Object.getOwnPropertyDescriptor(proxy, prop);
+ assert(pDesc.configurable);
+ assert(pDesc.enumerable);
+ assert(called);
+ called = false;
+ }
+}
