diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h index 917f0553cca..79bd646a43c 100644 --- a/deps/chakrashim/include/v8.h +++ b/deps/chakrashim/include/v8.h @@ -3005,6 +3005,9 @@ class V8_EXPORT Context { void SetSecurityToken(Handle token); Handle GetSecurityToken(); void AllowCodeGenerationFromStrings(bool allow); + + void CacheGlobalProperties(); + void ResolveGlobalChanges(Local sandbox); }; class V8_EXPORT Locker { diff --git a/deps/chakrashim/lib/chakra_shim.js b/deps/chakrashim/lib/chakra_shim.js index fddf9be901a..d4a2efd35b6 100644 --- a/deps/chakrashim/lib/chakra_shim.js +++ b/deps/chakrashim/lib/chakra_shim.js @@ -24,6 +24,7 @@ const Function_prototype_toString = Function.prototype.toString; const Object_defineProperty = Object.defineProperty; const Object_getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; + const Object_getOwnPropertyDescriptors = Object.getOwnPropertyDescriptors; const Object_getOwnPropertyNames = Object.getOwnPropertyNames; const Object_keys = Object.keys; const Object_prototype_toString = Object.prototype.toString; @@ -671,6 +672,76 @@ return ownPropertyNames; }; + + utils.beforeContext = function (contextGlobal) { + return Object_getOwnPropertyDescriptors(contextGlobal); + } + + function descriptorDiff(a, b) { + if (!a || !b) { + return true; + } + if (a.value !== b.value && a.value === a.value && b.value === b.value) { + // allow for NaN in value + return true; + } + if (a.enumerable !== b.enumerable || + a.configurable !== b.configurable || + a.writable !== b.writable) { + return true; + } + if (a.get !== b.get || a.set !== b.set) { + return true; + } + return false; + } + utils.afterContext = function (beforeDescriptors, contextGlobal, sandbox) { + try { + const afterDescriptors = Object_getOwnPropertyDescriptors(contextGlobal); + const beforeKeys = Object_keys(beforeDescriptors); + const afterKeys = Object_keys(afterDescriptors); + + for (const beforeKey of beforeKeys) { + const beforeDescriptor = beforeDescriptors[beforeKey]; + const afterDescriptor = afterDescriptors[beforeKey]; + const sandboxDescriptor = Object_getOwnPropertyDescriptor(sandbox, beforeKey); + if (!afterDescriptor) { + // The descriptor was removed + if (sandboxDescriptor) { + if (sandboxDescriptor.configurable) { + delete sandbox[beforeKey]; + } else { + // TODO: How to handle this? + } + } + } else if (descriptorDiff(beforeDescriptor, afterDescriptor)) { + // Before and after differ; apply an update if possible + if (!sandboxDescriptor || sandboxDescriptor.configurable) { + Object_defineProperty(sandbox, beforeKey, afterDescriptor); + } else { + // TODO: How can we handle this case? Node tries to avoid it, but we don't always + } + } + } + + for (const afterKey of afterKeys) { + if (afterKey in beforeDescriptors) { + continue; // Handled above + } + + // This property is freshly added + const afterDescriptor = afterDescriptors[afterKey]; + const sandboxDescriptor = Object_getOwnPropertyDescriptor(sandbox, afterKey); + if (!sandboxDescriptor || sandboxDescriptor.configurable) { + Object_defineProperty(sandbox, afterKey, afterDescriptor); + } else { + // TODO: How can we handle this? + } + } + } catch (e) { + // ignored; + } + } } patchErrorTypes(); diff --git a/deps/chakrashim/src/jsrtcachedpropertyidref.inc b/deps/chakrashim/src/jsrtcachedpropertyidref.inc index ce8f73c6063..ffc3dcc7c35 100644 --- a/deps/chakrashim/src/jsrtcachedpropertyidref.inc +++ b/deps/chakrashim/src/jsrtcachedpropertyidref.inc @@ -73,6 +73,8 @@ DEF(ensureDebug) DEF(saveInHandleScope) DEF(getPropertyAttributes) DEF(getOwnPropertyNames) +DEF(beforeContext) +DEF(afterContext) DEF(getFunctionName) DEF(getFileName) DEF(getColumnNumber) diff --git a/deps/chakrashim/src/jsrtcontextshim.cc b/deps/chakrashim/src/jsrtcontextshim.cc index 6559dc18ef8..f5e743e8237 100644 --- a/deps/chakrashim/src/jsrtcontextshim.cc +++ b/deps/chakrashim/src/jsrtcontextshim.cc @@ -98,7 +98,10 @@ ContextShim::ContextShim(IsolateShim * isolateShim, getPropertyAttributesFunction(JS_INVALID_REFERENCE), getOwnPropertyNamesFunction(JS_INVALID_REFERENCE), jsonParseFunction(JS_INVALID_REFERENCE), - jsonStringifyFunction(JS_INVALID_REFERENCE) { + jsonStringifyFunction(JS_INVALID_REFERENCE), + beforeContextFunction(JS_INVALID_REFERENCE), + afterContextFunction(JS_INVALID_REFERENCE), + cachedDescriptors(JS_INVALID_REFERENCE) { memset(globalConstructor, 0, sizeof(globalConstructor)); memset(globalPrototypeFunction, 0, sizeof(globalPrototypeFunction)); } @@ -482,6 +485,33 @@ void ContextShim::SetAlignedPointerInEmbedderData(int index, void * value) { } } +void ContextShim::CacheGlobalProperties() { + JsValueRef beforeContext = this->GetbeforeContextFunction(); + JsValueRef result = JS_INVALID_REFERENCE; + if (jsrt::CallFunction(beforeContext, globalObject, &result) != JsNoError) { + return; + } + this->cachedDescriptors = result; + JsAddRef(this->cachedDescriptors, nullptr); +} + +void ContextShim::ResolveGlobalChanges(JsValueRef sandbox) { + JsValueRef beforeDescriptors = this->cachedDescriptors; + this->cachedDescriptors = JS_INVALID_REFERENCE; + if (beforeDescriptors == JS_INVALID_REFERENCE) { + return; + } + JsRelease(beforeDescriptors, nullptr); + JsValueRef afterContext = this->GetafterContextFunction(); + JsValueRef result = JS_INVALID_REFERENCE; + jsrt::CallFunction( + afterContext, + beforeDescriptors, + globalObject, + sandbox, + &result); +} + // check initialization state first instead of calling // InitializeCurrentContextShim to save a function call for cases where // contextshim is already initialized @@ -600,6 +630,8 @@ CHAKRASHIM_FUNCTION_GETTER(getPropertyAttributes); CHAKRASHIM_FUNCTION_GETTER(getOwnPropertyNames); CHAKRASHIM_FUNCTION_GETTER(jsonParse); CHAKRASHIM_FUNCTION_GETTER(jsonStringify); +CHAKRASHIM_FUNCTION_GETTER(beforeContext); +CHAKRASHIM_FUNCTION_GETTER(afterContext); #define DEF_IS_TYPE(F) CHAKRASHIM_FUNCTION_GETTER(F) #include "jsrtcachedpropertyidref.inc" diff --git a/deps/chakrashim/src/jsrtcontextshim.h b/deps/chakrashim/src/jsrtcontextshim.h index 173c82a4932..bd5b842a903 100644 --- a/deps/chakrashim/src/jsrtcontextshim.h +++ b/deps/chakrashim/src/jsrtcontextshim.h @@ -98,6 +98,9 @@ class ContextShim { void * GetAlignedPointerFromEmbedderData(int index); void SetAlignedPointerInEmbedderData(int index, void * value); + void CacheGlobalProperties(); + void ResolveGlobalChanges(JsValueRef sandbox); + static ContextShim * GetCurrent(); private: @@ -138,6 +141,8 @@ class ContextShim { JsValueRef globalPrototypeFunction[GlobalPrototypeFunction::_FunctionCount]; std::vector embedderData; + JsValueRef cachedDescriptors; + #define DECLARE_CHAKRASHIM_FUNCTION_GETTER(F) \ public: \ JsValueRef Get##F##Function(); \ @@ -165,6 +170,8 @@ class ContextShim { DECLARE_CHAKRASHIM_FUNCTION_GETTER(getOwnPropertyNames); DECLARE_CHAKRASHIM_FUNCTION_GETTER(jsonParse); DECLARE_CHAKRASHIM_FUNCTION_GETTER(jsonStringify); + DECLARE_CHAKRASHIM_FUNCTION_GETTER(beforeContext); + DECLARE_CHAKRASHIM_FUNCTION_GETTER(afterContext); }; } // namespace jsrt diff --git a/deps/chakrashim/src/v8context.cc b/deps/chakrashim/src/v8context.cc index 23b0c16d89a..84fa2dc568e 100644 --- a/deps/chakrashim/src/v8context.cc +++ b/deps/chakrashim/src/v8context.cc @@ -163,4 +163,14 @@ void Context::AllowCodeGenerationFromStrings(bool allow) { // CHAKRA-TODO } +void Context::CacheGlobalProperties() { + jsrt::IsolateShim::GetContextShim( + reinterpret_cast(this))->CacheGlobalProperties(); +} + +void Context::ResolveGlobalChanges(Local sandbox) { + jsrt::IsolateShim::GetContextShim( + reinterpret_cast(this)) + ->ResolveGlobalChanges((JsValueRef)*sandbox); +} } // namespace v8 diff --git a/src/node_contextify.cc b/src/node_contextify.cc index abb1e1cdea6..f9652fdb4e8 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -795,11 +795,23 @@ class ContextifyScript : public BaseObject { // Do the eval within the context Context::Scope context_scope(contextify_context->context()); + +#ifdef NODE_ENGINE_CHAKRACORE + // node-chakracore does not handle interceptors on contexts the same + // way that node-v8 does, so we need to patch things up here + + contextify_context->context()->CacheGlobalProperties(); +#endif + EvalMachine(contextify_context->env(), timeout, display_errors, break_on_sigint, args); + +#ifdef NODE_ENGINE_CHAKRACORE + contextify_context->context()->ResolveGlobalChanges(sandbox); +#endif } static void DecorateErrorStack(Environment* env, const TryCatch& try_catch) { diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 99adf6e9a8c..c485cdf914a 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -188,6 +188,11 @@ test-v8-untrusted-code-mitigations : SKIP # https://github.com/Microsoft/ChakraCore/issues/5196 test-repl-top-level-await : SKIP +# This test fails due to the workaround for vm.runInContext not working +# with "var" declarations. Once we have an alternate fix, this can be +# re-enabled +test-vm-proxy-failure-CP : SKIP + # This test seems to fail under system stress, marking it flaky for now # https://github.com/nodejs/node-chakracore/issues/494 test-async-hooks-destroy-on-gc : PASS,FLAKY