Skip to content

Commit

Permalink
[MERGE #4806 @rhuanjl] Implement JsGetProxyProperties API fixes #950
Browse files Browse the repository at this point in the history
Merge pull request #4806 from rhuanjl:JsGetProxyProperties

Responding to issue #950

This PR:
1. Adds a JavascriptProxy::IsRevoked method to the Javascript proxy class (necessary for the below)
2. Adds a JsGetProxyProperties API to Jsrt which can:
    a) check if an object is a proxy -> set a provided bool to true/false
    b) if it is a proxy check if it's revoked
    c) if it is a revoked proxy set provided target and handler references to nullptr
    d) if it is a proxy that is not revoked provide references to it's target and handler
3. Tracks the same API through to WScriptJsrt in ch
4. Adds a test that uses the ch implementation

(Targeting 1.9 as this will assist with an issue in node-chakracore nodejs/node-chakracore#488 )

**CC:** @jackhorton @kfarnung @dilijev
  • Loading branch information
boingoing committed Mar 13, 2018
2 parents f58e17f + e891f8c commit 6861888
Show file tree
Hide file tree
Showing 11 changed files with 231 additions and 0 deletions.
2 changes: 2 additions & 0 deletions bin/ChakraCore/ChakraCore.def
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,5 @@ JsLessThanOrEqual
JsCreateEnhancedFunction

JsSetHostPromiseRejectionTracker

JsGetProxyProperties
1 change: 1 addition & 0 deletions bin/ch/ChakraRtInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ bool ChakraRTInterface::LoadChakraDll(ArgInfo* argInfo, HINSTANCE *outLibrary)
m_jsApiHooks.pfJsrtCopyString = (JsAPIHooks::JsrtCopyString)GetChakraCoreSymbol(library, "JsCopyString");
m_jsApiHooks.pfJsrtCreatePropertyId = (JsAPIHooks::JsrtCreatePropertyId)GetChakraCoreSymbol(library, "JsCreatePropertyId");
m_jsApiHooks.pfJsrtCreateExternalArrayBuffer = (JsAPIHooks::JsrtCreateExternalArrayBuffer)GetChakraCoreSymbol(library, "JsCreateExternalArrayBuffer");
m_jsApiHooks.pfJsrtGetProxyProperties = (JsAPIHooks::JsrtGetProxyProperties)GetChakraCoreSymbol(library, "JsGetProxyProperties");

m_jsApiHooks.pfJsrtTTDCreateRecordRuntime = (JsAPIHooks::JsrtTTDCreateRecordRuntimePtr)GetChakraCoreSymbol(library, "JsTTDCreateRecordRuntime");
m_jsApiHooks.pfJsrtTTDCreateReplayRuntime = (JsAPIHooks::JsrtTTDCreateReplayRuntimePtr)GetChakraCoreSymbol(library, "JsTTDCreateReplayRuntime");
Expand Down
3 changes: 3 additions & 0 deletions bin/ch/ChakraRtInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ struct JsAPIHooks

typedef JsErrorCode(WINAPI *JsrtCreateExternalArrayBuffer)(void *data, unsigned int byteLength, JsFinalizeCallback finalizeCallback, void *callbackState, JsValueRef *result);
typedef JsErrorCode(WINAPI *JsrtCreatePropertyId)(const char *name, size_t length, JsPropertyIdRef *propertyId);
typedef JsErrorCode(WINAPI *JsrtGetProxyProperties)(JsValueRef object, bool* isProxy, JsValueRef* target, JsValueRef* handler);

typedef JsErrorCode(WINAPI *JsrtTTDCreateRecordRuntimePtr)(JsRuntimeAttributes attributes, bool enableDebugging, size_t snapInterval, size_t snapHistoryLength, TTDOpenResourceStreamCallback openResourceStream, JsTTDWriteBytesToStreamCallback writeBytesToStream, JsTTDFlushAndCloseStreamCallback flushAndCloseStream, JsThreadServiceCallback threadService, JsRuntimeHandle *runtime);
typedef JsErrorCode(WINAPI *JsrtTTDCreateReplayRuntimePtr)(JsRuntimeAttributes attributes, const char* infoUri, size_t infoUriCount, bool enableDebugging, TTDOpenResourceStreamCallback openResourceStream, JsTTDReadBytesFromStreamCallback readBytesFromStream, JsTTDFlushAndCloseStreamCallback flushAndCloseStream, JsThreadServiceCallback threadService, JsRuntimeHandle *runtime);
Expand Down Expand Up @@ -183,6 +184,7 @@ struct JsAPIHooks
JsrtCopyString pfJsrtCopyString;
JsrtCreatePropertyId pfJsrtCreatePropertyId;
JsrtCreateExternalArrayBuffer pfJsrtCreateExternalArrayBuffer;
JsrtGetProxyProperties pfJsrtGetProxyProperties;

JsrtTTDCreateRecordRuntimePtr pfJsrtTTDCreateRecordRuntime;
JsrtTTDCreateReplayRuntimePtr pfJsrtTTDCreateReplayRuntime;
Expand Down Expand Up @@ -412,6 +414,7 @@ class ChakraRTInterface
static JsErrorCode WINAPI JsCreateStringUtf16(const uint16_t *content, size_t length, JsValueRef *value) { return HOOK_JS_API(CreateStringUtf16(content, length, value)); }
static JsErrorCode WINAPI JsCreatePropertyId(const char *name, size_t length, JsPropertyIdRef *propertyId) { return HOOK_JS_API(CreatePropertyId(name, length, propertyId)); }
static JsErrorCode WINAPI JsCreateExternalArrayBuffer(void *data, unsigned int byteLength, JsFinalizeCallback finalizeCallback, void *callbackState, JsValueRef *result) { return HOOK_JS_API(CreateExternalArrayBuffer(data, byteLength, finalizeCallback, callbackState, result)); }
static JsErrorCode WINAPI JsGetProxyProperties(JsValueRef object, bool* isProxy, JsValueRef* target, JsValueRef* handler) { return HOOK_JS_API(GetProxyProperties(object, isProxy, target, handler)); }
};

class AutoRestoreContext
Expand Down
52 changes: 52 additions & 0 deletions bin/ch/WScriptJsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,7 @@ bool WScriptJsrt::Initialize()
IfFalseGo(WScriptJsrt::InstallObjectsOnObject(wscript, "LoadTextFile", LoadTextFileCallback));
IfFalseGo(WScriptJsrt::InstallObjectsOnObject(wscript, "Flag", FlagCallback));
IfFalseGo(WScriptJsrt::InstallObjectsOnObject(wscript, "RegisterModuleSource", RegisterModuleSourceCallback));
IfFalseGo(WScriptJsrt::InstallObjectsOnObject(wscript, "GetProxyProperties", GetProxyPropertiesCallback));

// ToDo Remove
IfFalseGo(WScriptJsrt::InstallObjectsOnObject(wscript, "Edit", EmptyCallback));
Expand Down Expand Up @@ -1371,6 +1372,57 @@ JsValueRef __stdcall WScriptJsrt::SleepCallback(JsValueRef callee, bool isConstr
return returnValue;
}

JsValueRef __stdcall WScriptJsrt::GetProxyPropertiesCallback(JsValueRef callee, bool isConstructCall, JsValueRef *arguments, unsigned short argumentCount, void *callbackState)
{
HRESULT hr = E_FAIL;
JsValueRef returnValue = JS_INVALID_REFERENCE;
JsValueRef undefined = JS_INVALID_REFERENCE;
JsErrorCode errorCode = JsNoError;

IfJsrtErrorSetGo(ChakraRTInterface::JsGetUndefinedValue(&undefined));

returnValue = undefined;

if (argumentCount > 1)
{
bool isProxy = false;
JsValueRef target;
JsValueRef handler;
IfJsrtErrorSetGo(ChakraRTInterface::JsGetProxyProperties(arguments[1], &isProxy, &target, &handler));

if (isProxy)
{
JsPropertyIdRef targetProperty;
JsPropertyIdRef handlerProperty;
JsPropertyIdRef revokedProperty;

IfJsrtErrorSetGo(CreatePropertyIdFromString("target", &targetProperty));
IfJsrtErrorSetGo(CreatePropertyIdFromString("handler", &handlerProperty));
IfJsrtErrorSetGo(CreatePropertyIdFromString("revoked", &revokedProperty));
IfJsrtErrorSetGo(ChakraRTInterface::JsCreateObject(&returnValue));

JsValueRef revoked = JS_INVALID_REFERENCE;

if (target == JS_INVALID_REFERENCE)
{
IfJsrtErrorSetGo(ChakraRTInterface::JsGetTrueValue(&revoked));
target = undefined;
handler = undefined;
}
else
{
IfJsrtErrorSetGo(ChakraRTInterface::JsGetFalseValue(&revoked));
}

IfJsrtErrorSetGo(ChakraRTInterface::JsSetProperty(returnValue, handlerProperty, handler, true));
IfJsrtErrorSetGo(ChakraRTInterface::JsSetProperty(returnValue, targetProperty, target, true));
IfJsrtErrorSetGo(ChakraRTInterface::JsSetProperty(returnValue, revokedProperty, revoked, true));
}
}
Error:
return returnValue;
}

bool WScriptJsrt::PrintException(LPCSTR fileName, JsErrorCode jsErrorCode)
{
LPCWSTR errorTypeString = ConvertErrorCodeToMessage(jsErrorCode);
Expand Down
1 change: 1 addition & 0 deletions bin/ch/WScriptJsrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ class WScriptJsrt
static JsValueRef CALLBACK GetReportCallback(JsValueRef callee, bool isConstructCall, JsValueRef *arguments, unsigned short argumentCount, void *callbackState);
static JsValueRef CALLBACK LeavingCallback(JsValueRef callee, bool isConstructCall, JsValueRef *arguments, unsigned short argumentCount, void *callbackState);
static JsValueRef CALLBACK SleepCallback(JsValueRef callee, bool isConstructCall, JsValueRef *arguments, unsigned short argumentCount, void *callbackState);
static JsValueRef CALLBACK GetProxyPropertiesCallback(JsValueRef callee, bool isConstructCall, JsValueRef *arguments, unsigned short argumentCount, void *callbackState);

static JsErrorCode FetchImportedModuleHelper(JsModuleRecord referencingModule, JsValueRef specifier, __out JsModuleRecord* dependentModuleRecord, LPCSTR refdir = nullptr);

Expand Down
28 changes: 28 additions & 0 deletions lib/Jsrt/ChakraCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -1035,5 +1035,33 @@ CHAKRA_API
JsSetHostPromiseRejectionTracker(
_In_ JsHostPromiseRejectionTrackerCallback promiseRejectionTrackerCallback,
_In_opt_ void *callbackState);

/// <summary>
/// Determines if a provided object is a JavscriptProxy Object and
/// provides references to a Proxy's target and handler.
/// </summary>
/// <remarks>
/// Requires an active script context.
/// If object is not a Proxy object the target and handler parameters are not touched.
/// If nullptr is supplied for target or handler the function returns after
/// setting the isProxy value.
/// If the object is a revoked Proxy target and handler are set to JS_INVALID_REFERENCE.
/// If it is a Proxy object that has not been revoked target and handler are set to the
/// the object's target and handler.
/// </remarks>
/// <param name="object">The object that may be a Proxy.</param>
/// <param name="isProxy">Pointer to a Boolean - is the object a proxy?</param>
/// <param name="target">Pointer to a JsValueRef - the object's target.</param>
/// <param name="handler">Pointer to a JsValueRef - the object's handler.</param>
/// <returns>
/// The code <c>JsNoError</c> if the operation succeeded, a failure code otherwise.
/// </returns>
CHAKRA_API
JsGetProxyProperties(
_In_ JsValueRef object,
_Out_ bool* isProxy,
_Out_opt_ JsValueRef* target,
_Out_opt_ JsValueRef* handler);

#endif // _CHAKRACOREBUILD
#endif // _CHAKRACORE_H_
41 changes: 41 additions & 0 deletions lib/Jsrt/Jsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5329,4 +5329,45 @@ CHAKRA_API JsSetHostPromiseRejectionTracker(_In_ JsHostPromiseRejectionTrackerCa
/*allowInObjectBeforeCollectCallback*/true);
}

CHAKRA_API JsGetProxyProperties (_In_ JsValueRef object, _Out_ bool* isProxy, _Out_opt_ JsValueRef* target, _Out_opt_ JsValueRef* handler)
{
return ContextAPINoScriptWrapper_NoRecord([&](Js::ScriptContext * scriptContext) -> JsErrorCode {
VALIDATE_INCOMING_REFERENCE(object, scriptContext);
PARAM_NOT_NULL(isProxy);

if (target != nullptr)
{
*target = JS_INVALID_REFERENCE;
}

if (handler != nullptr)
{
*handler = JS_INVALID_REFERENCE;
}

*isProxy = Js::JavascriptProxy::Is(object);

if (!*isProxy)
{
return JsNoError;
}

Js::JavascriptProxy* proxy = Js::JavascriptProxy::UnsafeFromVar(object);
bool revoked = proxy->IsRevoked();

if (target != nullptr && !revoked)
{
*target = static_cast<JsValueRef>(proxy->GetTarget());
}

if (handler != nullptr && !revoked)
{
*handler = static_cast<JsValueRef>(proxy->GetHandler());
}

return JsNoError;
},
/*allowInObjectBeforeCollectCallback*/true);
}

#endif // _CHAKRACOREBUILD
5 changes: 5 additions & 0 deletions lib/Runtime/Library/JavascriptProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ namespace Js
return JavascriptOperators::GetTypeId(obj) == TypeIds_Proxy;
}

bool JavascriptProxy::IsRevoked() const
{
return (target == nullptr);
}

RecyclableObject* JavascriptProxy::GetTarget()
{
if (target == nullptr)
Expand Down
1 change: 1 addition & 0 deletions lib/Runtime/Library/JavascriptProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ namespace Js
virtual RecyclableObject* ToObject(ScriptContext * requestContext) override;
virtual Var GetTypeOfString(ScriptContext* requestContext) override;

bool IsRevoked() const;
BOOL SetPropertyTrap(Var receiver, SetPropertyTrapKind setPropertyTrapKind, PropertyId propertyId, Var newValue, ScriptContext* requestContext, BOOL skipPrototypeCheck = FALSE);
BOOL SetPropertyTrap(Var receiver, SetPropertyTrapKind setPropertyTrapKind, Js::JavascriptString * propertyString, Var newValue, ScriptContext* requestContext);

Expand Down
90 changes: 90 additions & 0 deletions test/es6/ProxyPropertiesAPI.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");

let tests = [
{
name: "GetProxyProperties: no argugments",
body: function () {
let properties = WScript.GetProxyProperties();
assert.isUndefined(properties, "ProxyProperties of nothing should be undefined.");
}
},
{
name: "GetProxyProperties: non-proxy arguments",
body: function () {
let properties = WScript.GetProxyProperties(undefined);
assert.isUndefined(properties, "ProxyProperties of undefined should be undefined.");
properties = WScript.GetProxyProperties(1);
assert.isUndefined(properties, "ProxyProperties of number should be undefined.");
properties = WScript.GetProxyProperties({});
assert.isUndefined(properties, "ProxyProperties of non-proxy object should be undefined.");
}
},
{
name: "GetProxyProperties: revocable Proxy",
body: function () {
let revocable = Proxy.revocable({someProperty: true, otherProperty: false}, {otherProperty: true, newProperty: 5});
let proxy = revocable.proxy;
let properties = WScript.GetProxyProperties(proxy);

let names = Object.getOwnPropertyNames(properties);
assert.areEqual(names.length, 3, "proxy properties names should have length 3.");
assert.isTrue(names.includes("target"));
assert.isTrue(names.includes("handler"));
assert.isTrue(names.includes("revoked"));
assert.isFalse(properties.revoked, "Revoked bool should be false.");

names = Object.getOwnPropertyNames(properties.target);
assert.areEqual(names.length, 2, "proxy properties target names should have length 2.");
assert.areEqual(properties.target.someProperty, true);
assert.areEqual(properties.target.otherProperty, false);

names = Object.getOwnPropertyNames(properties.handler);
assert.areEqual(names.length, 2, "proxy properties handler names should have length 2.");
assert.areEqual(properties.handler.newProperty, 5);
assert.areEqual(properties.handler.otherProperty, true);

revocable.revoke();
properties = WScript.GetProxyProperties(proxy);

names = Object.getOwnPropertyNames(properties);
assert.areEqual(names.length, 3, "proxy properties names for revokes proxy should have length 3.");
assert.isTrue(names.includes("target"));
assert.isTrue(names.includes("handler"));
assert.isTrue(properties.revoked, "Revoked bool should be true.");

assert.isUndefined(properties.target, "Target of revoked proxy should be undefined.");
assert.isUndefined(properties.handler, "Handler of revoked proxy should be undefined.");
}
},
{
name: "GetProxyProperties: normal Proxy",
body: function () {
let proxy = new Proxy({someProperty: true, otherProperty: false}, {otherProperty: true, newProperty: 5});
let properties = WScript.GetProxyProperties(proxy);

let names = Object.getOwnPropertyNames(properties);
assert.areEqual(names.length, 3, "proxy properties names should have length 3");
assert.isTrue(names.includes("target"));
assert.isTrue(names.includes("handler"));
assert.isTrue(names.includes("revoked"));
assert.isFalse(properties.revoked, "Revoked bool should be false.");

names = Object.getOwnPropertyNames(properties.target);
assert.areEqual(names.length, 2, "proxy properties target names should have length 2");
assert.areEqual(properties.target.someProperty, true);
assert.areEqual(properties.target.otherProperty, false);

names = Object.getOwnPropertyNames(properties.handler);
assert.areEqual(names.length, 2, "proxy properties handler names should have length 2");
assert.areEqual(properties.handler.newProperty, 5);
assert.areEqual(properties.handler.otherProperty, true);
}
}
];

testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
7 changes: 7 additions & 0 deletions test/es6/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,13 @@
<compile-flags>-args summary -endargs</compile-flags>
</default>
</test>
<test>
<default>
<files>ProxyPropertiesAPI.js</files>
<compile-flags>-args summary -endargs</compile-flags>
<tags>exclude_jshost</tags>
</default>
</test>
<test>
<default>
<files>proxybugs.js</files>
Expand Down

0 comments on commit 6861888

Please sign in to comment.