Skip to content

Commit

Permalink
fix(nodevm): align behavior with node (#13590)
Browse files Browse the repository at this point in the history
  • Loading branch information
@SunsetTechuila
SunsetTechuila authored Aug 30, 2024
1 parent 682b373 commit 59eb551
Show file tree
Hide file tree
Showing 3 changed files with 227 additions and 150 deletions.
52 changes: 17 additions & 35 deletions src/bun.js/bindings/NodeVM.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,20 @@ class ScriptOptions {
{
auto& vm = globalObject->vm();
ScriptOptions opts;
JSObject* options;
bool any = false;
if (!optionsArg.isUndefined()) {
if (!optionsArg.isObject()) {
if (optionsArg.isObject()) {
options = asObject(optionsArg);
} else if (optionsArg.isString()) {
options = constructEmptyObject(globalObject);
options->putDirect(vm, Identifier::fromString(vm, "filename"_s), optionsArg);
} else {
auto scope = DECLARE_THROW_SCOPE(vm);
throwVMTypeError(globalObject, scope, "options must be an object"_s);
throwVMTypeError(globalObject, scope, "options must be an object or a string"_s);
failed = true;
return std::nullopt;
}
JSObject* options = asObject(optionsArg);

if (JSValue filenameOpt = options->getIfPropertyExists(globalObject, builtinNames(vm).filenamePublicName())) {
if (filenameOpt.isString()) {
Expand Down Expand Up @@ -300,8 +305,7 @@ JSC_DEFINE_HOST_FUNCTION(vmModuleRunInThisContext, (JSGlobalObject * globalObjec
{
auto& vm = globalObject->vm();
auto sourceStringValue = callFrame->argument(0);
JSValue contextObjectValue = callFrame->argument(1);
JSValue optionsObjectValue = callFrame->argument(2);
JSValue optionsObjectValue = callFrame->argument(1);
auto throwScope = DECLARE_THROW_SCOPE(vm);

if (!sourceStringValue.isString()) {
Expand All @@ -311,15 +315,6 @@ JSC_DEFINE_HOST_FUNCTION(vmModuleRunInThisContext, (JSGlobalObject * globalObjec

auto sourceString = sourceStringValue.toWTFString(globalObject);

if (!contextObjectValue || contextObjectValue.isUndefinedOrNull()) {
contextObjectValue = JSC::constructEmptyObject(globalObject);
}

if (UNLIKELY(!contextObjectValue || !contextObjectValue.isObject())) {
throwTypeError(globalObject, throwScope, "Context must be an object"_s);
return JSValue::encode({});
}

ScriptOptions options;
{
bool didThrow = false;
Expand All @@ -334,19 +329,13 @@ JSC_DEFINE_HOST_FUNCTION(vmModuleRunInThisContext, (JSGlobalObject * globalObjec
SourceCode source(
JSC::StringSourceProvider::create(sourceString, JSC::SourceOrigin(WTF::URL::fileURLWithFileSystemPath(options.filename)), options.filename, JSC::SourceTaintedOrigin::Untainted, TextPosition(options.lineOffset, options.columnOffset)),
options.lineOffset.zeroBasedInt(), options.columnOffset.zeroBasedInt());
auto* zigGlobal = reinterpret_cast<Zig::GlobalObject*>(globalObject);
JSObject* context = asObject(contextObjectValue);

auto proxyStructure = zigGlobal->globalProxyStructure();
auto proxy = JSGlobalProxy::create(vm, proxyStructure);
proxy->setTarget(vm, globalObject);
context->setPrototypeDirect(vm, proxy);

auto* executable = JSC::DirectEvalExecutable::create(
globalObject, source, NoLexicallyScopedFeatures, DerivedContextType::None, NeedsClassFieldInitializer::No, PrivateBrandRequirement::None,
false, false, EvalContextType::None, nullptr, nullptr);
RETURN_IF_EXCEPTION(throwScope, {});

JSObject* context = asObject(JSC::constructEmptyObject(globalObject));
JSScope* contextScope = JSWithScope::create(vm, globalObject, globalObject->globalScope(), context);
auto catchScope = DECLARE_CATCH_SCOPE(vm);
JSValue result = vm.interpreter.executeEval(executable, globalObject, contextScope);
Expand Down Expand Up @@ -393,10 +382,10 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInNewContext, (JSGlobalObject * globalObject,
auto* targetContext = NodeVMGlobalObject::create(
vm, zigGlobal->NodeVMGlobalObjectStructure());

// auto proxyStructure = JSGlobalProxy::createStructure(vm, globalObject, JSC::jsNull());
// auto proxy = JSGlobalProxy::create(vm, proxyStructure);
// proxy->setTarget(vm, targetContext);
// context->setPrototypeDirect(vm, proxy);
auto proxyStructure = JSGlobalProxy::createStructure(vm, globalObject, JSC::jsNull());
auto proxy = JSGlobalProxy::create(vm, proxyStructure);
proxy->setTarget(vm, targetContext);
context->setPrototypeDirect(vm, proxy);

JSScope* contextScope = JSWithScope::create(vm, targetContext, targetContext->globalScope(), context);
return runInContext(globalObject, script, targetContext, contextScope, callFrame->argument(0));
Expand All @@ -407,21 +396,14 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInThisContext, (JSGlobalObject * globalObject,
JSValue thisValue = callFrame->thisValue();
auto* script = jsDynamicCast<NodeVMScript*>(thisValue);
auto throwScope = DECLARE_THROW_SCOPE(vm);
// TODO: options
// JSValue optionsObjectValue = callFrame->argument(0);

if (UNLIKELY(!script)) {
return throwVMTypeError(globalObject, throwScope, "Script.prototype.runInThisContext can only be called on a Script object"_s);
}

JSValue contextArg = callFrame->argument(0);
if (!contextArg || contextArg.isUndefinedOrNull()) {
contextArg = JSC::constructEmptyObject(globalObject);
}

if (!contextArg.isObject()) {
return throwVMTypeError(globalObject, throwScope, "context must be an object"_s);
}

JSObject* context = asObject(contextArg);
JSObject* context = asObject(JSC::constructEmptyObject(globalObject));
JSWithScope* contextScope = JSWithScope::create(vm, globalObject, globalObject->globalScope(), context);

return runInContext(globalObject, script, globalObject->globalThis(), contextScope, callFrame->argument(1));
Expand Down
153 changes: 94 additions & 59 deletions test/js/node/crypto/crypto.key-objects.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
generateKey,
} from "crypto";
import { test, it, expect, describe } from "bun:test";
import { createContext, Script } from "node:vm";
import { createContext, Script, runInThisContext, runInContext } from "node:vm";
import fs from "fs";
import path from "path";
import { isWindows } from "harness";
Expand Down Expand Up @@ -1407,74 +1407,105 @@ describe("crypto.KeyObjects", () => {
expect(privateKey.asymmetricKeyDetails?.modulusLength).toBe(513);
});

function testRunInContext(fn: any) {
test("can generate key", () => {
const context = createContext({ generateKeySync });
const result = fn(`generateKeySync("aes", { length: 128 })`, context);
expect(result).toBeDefined();
const keybuf = result.export();
expect(keybuf.byteLength).toBe(128 / 8);
});
test("can be used on another context", () => {
const context = createContext({ generateKeyPairSync, assertApproximateSize, testEncryptDecrypt, testSignVerify });
const result = fn(
`
const { publicKey: publicKeyDER, privateKey: privateKeyDER } = generateKeyPairSync(
"rsa",
{
publicExponent: 0x10001,
modulusLength: 512,
publicKeyEncoding: {
type: "pkcs1",
format: "der",
},
privateKeyEncoding: {
type: "pkcs8",
format: "der",
},
}
type TestRunInContextArg =
| { fn: typeof runInContext; isIsolated: true }
| { fn: typeof runInThisContext; isIsolated?: false };

function testRunInContext({ fn, isIsolated }: TestRunInContextArg) {
if (isIsolated) {
test("can generate key", () => {
const context = createContext({ generateKeySync });
const result = fn(`generateKeySync("aes", { length: 128 })`, context);
expect(result).toBeDefined();
const keybuf = result.export();
expect(keybuf.byteLength).toBe(128 / 8);
});
test("can be used on another context", () => {
const context = createContext({
generateKeyPairSync,
assertApproximateSize,
testEncryptDecrypt,
testSignVerify,
});
const result = fn(
`
const { publicKey: publicKeyDER, privateKey: privateKeyDER } = generateKeyPairSync(
"rsa",
{
publicExponent: 0x10001,
modulusLength: 512,
publicKeyEncoding: {
type: "pkcs1",
format: "der",
},
privateKeyEncoding: {
type: "pkcs8",
format: "der",
},
}
);
assertApproximateSize(publicKeyDER, 74);
const publicKey = {
key: publicKeyDER,
type: "pkcs1",
format: "der",
};
const privateKey = {
key: privateKeyDER,
format: "der",
type: "pkcs8",
passphrase: "secret",
};
testEncryptDecrypt(publicKey, privateKey);
testSignVerify(publicKey, privateKey);
`,
context,
);
assertApproximateSize(publicKeyDER, 74);
const publicKey = {
key: publicKeyDER,
type: "pkcs1",
format: "der",
};
const privateKey = {
key: privateKeyDER,
format: "der",
type: "pkcs8",
passphrase: "secret",
};
testEncryptDecrypt(publicKey, privateKey);
testSignVerify(publicKey, privateKey);
`,
context,
);
});
});
} else {
test("can generate key", () => {
const prop = randomProp();
// @ts-expect-error
globalThis[prop] = generateKeySync;
try {
const result = fn(`${prop}("aes", { length: 128 })`);
expect(result).toBeDefined();
const keybuf = result.export();
expect(keybuf.byteLength).toBe(128 / 8);
} finally {
// @ts-expect-error
delete globalThis[prop];
}
});
}
}
describe("Script", () => {
describe("runInContext()", () => {
testRunInContext((code, context, options) => {
// @ts-expect-error
const script = new Script(code, options);
return script.runInContext(context);
testRunInContext({
fn: (code, context, options) => {
const script = new Script(code, options);
return script.runInContext(context);
},
isIsolated: true,
});
});
describe("runInNewContext()", () => {
testRunInContext((code, context, options) => {
// @ts-expect-error
const script = new Script(code, options);
return script.runInNewContext(context);
testRunInContext({
fn: (code, context, options) => {
const script = new Script(code, options);
return script.runInNewContext(context);
},
isIsolated: true,
});
});
describe("runInThisContext()", () => {
testRunInContext((code, context, options) => {
// @ts-expect-error
const script = new Script(code, options);
return script.runInThisContext(context);
testRunInContext({
fn: (code: string, options: any) => {
const script = new Script(code, options);
return script.runInThisContext();
},
});
});
});
Expand Down Expand Up @@ -1697,3 +1728,7 @@ test("ECDSA should work", async () => {
verify("sha256", Buffer.from("foo"), { key: publicKey, dsaEncoding: "der" }, signature);
}).toThrow(/invalid dsaEncoding/);
});

function randomProp() {
return "prop" + crypto.randomUUID().replace(/-/g, "");
}
Loading

0 comments on commit 59eb551

Please sign in to comment.