diff --git a/src/jsc/bindings/NodeVM.cpp b/src/jsc/bindings/NodeVM.cpp index 976c08c36479..c802bb93f76f 100644 --- a/src/jsc/bindings/NodeVM.cpp +++ b/src/jsc/bindings/NodeVM.cpp @@ -130,6 +130,16 @@ JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, c VM& vm = globalObject->vm(); auto throwScope = DECLARE_THROW_SCOPE(vm); + // wrap the arguments in an anonymous function expression + int startOffset = 0; + String program = stringifyAnonymousFunction(globalObject, args, throwScope, &startOffset); + EXCEPTION_ASSERT(!!throwScope.exception() == program.isNull()); + RETURN_IF_EXCEPTION(throwScope, nullptr); + + // The wrapper is the longest text parsed below (the body alone is parsed first). + options.lineOffset = clampOffsetForSource(options.lineOffset, program.length()); + options.columnOffset = clampOffsetForSource(options.columnOffset, program.length()); + TextPosition position(options.lineOffset, options.columnOffset); LexicallyScopedFeatures lexicallyScopedFeatures = globalObject->globalScopeExtension() ? TaintedByWithScopeLexicallyScopedFeature : NoLexicallyScopedFeatures; @@ -180,11 +190,6 @@ JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, c } } - // wrap the arguments in an anonymous function expression - int startOffset = 0; - String code = stringifyAnonymousFunction(globalObject, args, throwScope, &startOffset); - EXCEPTION_ASSERT(!!throwScope.exception() == code.isNull()); - // The user's body starts on line 2 of the wrapped program (after the // "(function () {\n" prefix). Shift the provider's start position up one // line so reported positions line up with the body the way V8's @@ -196,7 +201,7 @@ JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, c TextPosition wrappedPosition(OrdinalNumber::fromZeroBasedInt(lineZeroBased > 0 ? lineZeroBased - 1 : lineZeroBased), position.m_column); SourceCode sourceCode( - JSC::StringSourceProvider::create(code, sourceOrigin, WTF::move(options.filename), sourceTaintOrigin, wrappedPosition, SourceProviderSourceType::Program), + JSC::StringSourceProvider::create(program, sourceOrigin, WTF::move(options.filename), sourceTaintOrigin, wrappedPosition, SourceProviderSourceType::Program), wrappedPosition.m_line.oneBasedInt(), wrappedPosition.m_column.oneBasedInt()); CodeCache* cache = vm.codeCache(); @@ -638,6 +643,14 @@ void decorateParseErrorStack(JSGlobalObject* globalObject, VM& vm, JSObject* err writeArrowHeaderStack(vm, errorInstance, url, reportedLine, sourceLineText, caretColumn, stack); } +OrdinalNumber clampOffsetForSource(OrdinalNumber offset, unsigned sourceLength) +{ + int64_t maxOffset = std::max(static_cast(std::numeric_limits::max()) - 1 - sourceLength, 0); + if (offset.zeroBasedInt() <= maxOffset) + return offset; + return OrdinalNumber::fromZeroBasedInt(static_cast(maxOffset)); +} + void getNodeVMContextOptions(JSGlobalObject* globalObject, JSC::VM& vm, JSC::ThrowScope& scope, JSValue optionsArg, NodeVMContextOptions& outOptions, ASCIILiteral codeGenerationKey, JSValue* importer) { if (importer) { @@ -1405,122 +1418,6 @@ void NodeVMGlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) visitor.append(thisObject->m_dynamicImportCallback); } -JSC_DEFINE_HOST_FUNCTION(vmModuleRunInNewContext, (JSGlobalObject * globalObject, CallFrame* callFrame)) -{ - VM& vm = globalObject->vm(); - auto scope = DECLARE_THROW_SCOPE(vm); - - JSValue code = callFrame->argument(0); - if (!code.isString()) - return ERR::INVALID_ARG_TYPE(scope, globalObject, "code"_s, "string"_s, code); - - JSValue contextArg = callFrame->argument(1); - bool notContextified = getContextArg(globalObject, contextArg); - - if (!contextArg.isObject()) { - return ERR::INVALID_ARG_TYPE(scope, globalObject, "context"_s, "object"_s, contextArg); - } - - JSObject* sandbox = asObject(contextArg); - - JSValue contextOptionsArg = callFrame->argument(2); - NodeVMContextOptions contextOptions {}; - - JSValue globalObjectDynamicImportCallback; - - getNodeVMContextOptions(globalObject, vm, scope, contextOptionsArg, contextOptions, "contextCodeGeneration", &globalObjectDynamicImportCallback); - RETURN_IF_EXCEPTION(scope, {}); - - contextOptions.notContextified = notContextified; - - // Create context and run code - auto* context = NodeVMGlobalObject::create(vm, - defaultGlobalObject(globalObject)->NodeVMGlobalObjectStructure(), - contextOptions, globalObjectDynamicImportCallback); - - context->setContextifiedObject(sandbox); - - JSValue optionsArg = callFrame->argument(2); - JSValue scriptDynamicImportCallback; - - ScriptOptions options(optionsArg.toWTFString(globalObject), OrdinalNumber::fromZeroBasedInt(0), OrdinalNumber::fromZeroBasedInt(0)); - if (optionsArg.isString()) { - options.filename = optionsArg.toWTFString(globalObject); - RETURN_IF_EXCEPTION(scope, {}); - } else if (!options.fromJS(globalObject, vm, scope, optionsArg, &scriptDynamicImportCallback)) { - RETURN_IF_EXCEPTION(scope, {}); - } - - RefPtr fetcher(NodeVMScriptFetcher::create(vm, scriptDynamicImportCallback, jsUndefined())); - - SourceCode sourceCode( - JSC::StringSourceProvider::create( - code.toString(globalObject)->value(globalObject), - JSC::SourceOrigin(WTF::URL::fileURLWithFileSystemPath(options.filename), *fetcher), - options.filename, - JSC::SourceTaintedOrigin::Untainted, - TextPosition(options.lineOffset, options.columnOffset)), - options.lineOffset.zeroBasedInt(), - options.columnOffset.zeroBasedInt()); - - NakedPtr exception; - JSValue result = JSC::evaluate(context, sourceCode, context, exception); - - if (exception) [[unlikely]] { - if (handleException(globalObject, vm, exception, scope)) { - return {}; - } - JSC::throwException(globalObject, scope, exception.get()); - return {}; - } - - return JSValue::encode(result); -} - -JSC_DEFINE_HOST_FUNCTION(vmModuleRunInThisContext, (JSGlobalObject * globalObject, CallFrame* callFrame)) -{ - VM& vm = JSC::getVM(globalObject); - auto sourceStringValue = callFrame->argument(0); - auto throwScope = DECLARE_THROW_SCOPE(vm); - - if (!sourceStringValue.isString()) { - return ERR::INVALID_ARG_TYPE(throwScope, globalObject, "code"_s, "string"_s, sourceStringValue); - } - - String sourceString = sourceStringValue.toWTFString(globalObject); - RETURN_IF_EXCEPTION(throwScope, encodedJSUndefined()); - - JSValue importer; - - JSValue optionsArg = callFrame->argument(1); - ScriptOptions options(optionsArg.toWTFString(globalObject), OrdinalNumber::fromZeroBasedInt(0), OrdinalNumber::fromZeroBasedInt(0)); - if (optionsArg.isString()) { - options.filename = optionsArg.toWTFString(globalObject); - RETURN_IF_EXCEPTION(throwScope, {}); - } else if (!options.fromJS(globalObject, vm, throwScope, optionsArg, &importer)) { - RETURN_IF_EXCEPTION(throwScope, encodedJSUndefined()); - } - - RefPtr fetcher(NodeVMScriptFetcher::create(vm, importer, jsUndefined())); - - SourceCode source( - JSC::StringSourceProvider::create(sourceString, JSC::SourceOrigin(WTF::URL::fileURLWithFileSystemPath(options.filename), *fetcher), options.filename, JSC::SourceTaintedOrigin::Untainted, TextPosition(options.lineOffset, options.columnOffset)), - options.lineOffset.zeroBasedInt(), options.columnOffset.zeroBasedInt()); - - WTF::NakedPtr exception; - JSValue result = JSC::evaluate(globalObject, source, globalObject, exception); - - if (exception) [[unlikely]] { - if (handleException(globalObject, vm, exception, throwScope)) { - return {}; - } - JSC::throwException(globalObject, throwScope, exception.get()); - return {}; - } - - return JSValue::encode(result); -} - JSC_DEFINE_HOST_FUNCTION(vmModuleCompileFunction, (JSGlobalObject * globalObject, CallFrame* callFrame)) { VM& vm = globalObject->vm(); @@ -1819,12 +1716,6 @@ JSC::JSValue createNodeVMBinding(Zig::GlobalObject* globalObject) obj->putDirect( vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "isContext"_s)), JSC::JSFunction::create(vm, globalObject, 0, "isContext"_s, vmModule_isContext, ImplementationVisibility::Public), 0); - obj->putDirect( - vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "runInNewContext"_s)), - JSC::JSFunction::create(vm, globalObject, 0, "runInNewContext"_s, vmModuleRunInNewContext, ImplementationVisibility::Public), 0); - obj->putDirect( - vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "runInThisContext"_s)), - JSC::JSFunction::create(vm, globalObject, 0, "runInThisContext"_s, vmModuleRunInThisContext, ImplementationVisibility::Public), 0); obj->putDirect( vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "compileFunction"_s)), JSC::JSFunction::create(vm, globalObject, 0, "compileFunction"_s, vmModuleCompileFunction, ImplementationVisibility::Public), 0); @@ -1923,13 +1814,6 @@ BaseVMOptions::BaseVMOptions(String filename) { } -BaseVMOptions::BaseVMOptions(String filename, OrdinalNumber lineOffset, OrdinalNumber columnOffset) - : filename(WTF::move(filename)) - , lineOffset(lineOffset) - , columnOffset(columnOffset) -{ -} - bool BaseVMOptions::fromJS(JSC::JSGlobalObject* globalObject, JSC::VM& vm, JSC::ThrowScope& scope, JSC::JSValue optionsArg) { JSObject* options = nullptr; diff --git a/src/jsc/bindings/NodeVM.h b/src/jsc/bindings/NodeVM.h index 7796b7941aa2..4a99a1997b77 100644 --- a/src/jsc/bindings/NodeVM.h +++ b/src/jsc/bindings/NodeVM.h @@ -33,6 +33,8 @@ bool handleException(JSGlobalObject* globalObject, VM& vm, NakedPtr // when no filename was provided; compileFunction has no such default. void decorateParseErrorStack(JSGlobalObject* globalObject, VM& vm, JSObject* error, StringView sourceString, const String& url, const JSC::ParserError& parseError, OrdinalNumber lineOffset); +// Lowers offset if needed so that offset + 1 + sourceLength fits in an int, JSC's position type. +OrdinalNumber clampOffsetForSource(OrdinalNumber offset, unsigned sourceLength); void getNodeVMContextOptions(JSGlobalObject* globalObject, JSC::VM& vm, JSC::ThrowScope& scope, JSValue optionsArg, NodeVMContextOptions& outOptions, ASCIILiteral codeGenerationKey, JSValue* importer); NodeVMGlobalObject* getGlobalObjectFromContext(JSGlobalObject* globalObject, JSValue contextValue, bool canThrow); JSC::EncodedJSValue INVALID_ARG_VALUE_VM_VARIATION(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, WTF::ASCIILiteral name, JSC::JSValue value); @@ -55,7 +57,6 @@ class BaseVMOptions { BaseVMOptions() = default; BaseVMOptions(String filename); - BaseVMOptions(String filename, OrdinalNumber lineOffset, OrdinalNumber columnOffset); bool fromJS(JSC::JSGlobalObject* globalObject, JSC::VM& vm, JSC::ThrowScope& scope, JSC::JSValue optionsArg); bool validateProduceCachedData(JSC::JSGlobalObject* globalObject, JSC::VM& vm, JSC::ThrowScope& scope, JSObject* options, bool& outProduceCachedData); @@ -172,7 +173,5 @@ void configureNodeVM(JSC::VM&, Zig::GlobalObject*); // VM module functions JSC_DECLARE_HOST_FUNCTION(vmModule_createContext); JSC_DECLARE_HOST_FUNCTION(vmModule_isContext); -JSC_DECLARE_HOST_FUNCTION(vmModuleRunInNewContext); -JSC_DECLARE_HOST_FUNCTION(vmModuleRunInThisContext); } // namespace Bun diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index 9d8dc9de7954..c03eb3fb9bcc 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -117,6 +117,8 @@ constructScript(JSGlobalObject* globalObject, CallFrame* callFrame, JSValue newT } else if (!options.fromJS(globalObject, vm, scope, optionsArg, &importer)) { RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined())); } + options.lineOffset = clampOffsetForSource(options.lineOffset, sourceString.length()); + options.columnOffset = clampOffsetForSource(options.columnOffset, sourceString.length()); auto* zigGlobalObject = defaultGlobalObject(globalObject); Structure* structure = zigGlobalObject->NodeVMScriptStructure(); diff --git a/src/jsc/bindings/NodeVMSourceTextModule.cpp b/src/jsc/bindings/NodeVMSourceTextModule.cpp index 2076d21145d8..cae93410ed9a 100644 --- a/src/jsc/bindings/NodeVMSourceTextModule.cpp +++ b/src/jsc/bindings/NodeVMSourceTextModule.cpp @@ -96,10 +96,14 @@ NodeVMSourceTextModule* NodeVMSourceTextModule::create(VM& vm, JSGlobalObject* g WTF::String sourceText = sourceTextValue.toWTFString(globalObject); RETURN_IF_EXCEPTION(scope, nullptr); - Ref sourceProvider = StringSourceProvider::create(WTF::move(sourceText), sourceOrigin, String {}, SourceTaintedOrigin::Untainted, - TextPosition { OrdinalNumber::fromZeroBasedInt(lineOffset), OrdinalNumber::fromZeroBasedInt(columnOffset) }, SourceProviderSourceType::Module); + TextPosition startPosition { + clampOffsetForSource(OrdinalNumber::fromZeroBasedInt(lineOffset), sourceText.length()), + clampOffsetForSource(OrdinalNumber::fromZeroBasedInt(columnOffset), sourceText.length()), + }; - SourceCode sourceCode(WTF::move(sourceProvider), lineOffset, columnOffset); + Ref sourceProvider = StringSourceProvider::create(WTF::move(sourceText), sourceOrigin, String {}, SourceTaintedOrigin::Untainted, startPosition, SourceProviderSourceType::Module); + + SourceCode sourceCode(WTF::move(sourceProvider), startPosition.m_line.zeroBasedInt(), startPosition.m_column.zeroBasedInt()); auto* zigGlobalObject = defaultGlobalObject(globalObject); WTF::String identifier = identifierValue.toWTFString(globalObject); diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index 1676eab88caf..8f5b858a0a15 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -1735,3 +1735,84 @@ test.concurrent("timeout during a nested event-loop wait beneath the script", as expect(stdout).toBe("ERR_SCRIPT_EXECUTION_TIMEOUT\n"); expect(exitCode).toBe(0); }); + +describe("node:vm lineOffset/columnOffset at the edge of int32", () => { + // Node's validator accepts any int32 here. JSC stores positions as ints, + // converts the offset to one-based and counts the source's own lines on top + // of it, so an offset this large used to overflow in the parser: assertion + // builds abort in JSTextPosition::checkConsistency ("line >= 0"), release + // builds report wrapped negative line numbers. Each case gets its own + // process because the failure mode is an abort. + const INT32_MAX = 2147483647; + + async function runFixture(body: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `const vm = require("node:vm");\n${body}`], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + return stdout; + } + + test.concurrent.each([ + ["new Script, one-line source", `new vm.Script("1", { lineOffset: ${INT32_MAX} })`], + [ + "new Script, second line steps past INT32_MAX", + `new vm.Script(${JSON.stringify("1;\n2;")}, { lineOffset: ${INT32_MAX - 1} })`, + ], + ["new Script, columnOffset", `new vm.Script(${JSON.stringify("1;\n2;")}, { columnOffset: ${INT32_MAX} })`], + ["compileFunction", `vm.compileFunction("return 1", [], { lineOffset: ${INT32_MAX} })`], + [ + "compileFunction with params, a multi-line body and both offsets", + `vm.compileFunction(${JSON.stringify("a;\nreturn a;")}, ["a"], { lineOffset: ${INT32_MAX - 1}, columnOffset: ${INT32_MAX} })`, + ], + // Three lines so the counter steps past INT32_MAX whether the module's + // first line is taken as lineOffset or, like Script, as lineOffset + 1. + ["SourceTextModule", `new vm.SourceTextModule(${JSON.stringify("1;\n2;\n3;")}, { lineOffset: ${INT32_MAX - 1} })`], + ])("%s compiles", async (_, expression) => { + const stdout = await runFixture(`${expression};\nconsole.log("ok");`); + expect(stdout).toBe("ok\n"); + }); + + test.concurrent.each([ + [ + "line of a runtime error thrown by a Script", + `new vm.Script(${JSON.stringify('1;\nthrow new Error("q")')}, { filename: "big.js", lineOffset: ${INT32_MAX - 1} }).runInThisContext()`, + /big\.js:(-?\d+)/, + ], + [ + "line of a compile-time SyntaxError from a Script", + `new vm.Script(${JSON.stringify("1;\n%%")}, { filename: "big.js", lineOffset: ${INT32_MAX - 1} })`, + /big\.js:(-?\d+)/, + ], + [ + "column of a runtime error thrown on the first line of a Script", + `new vm.Script('throw new Error("q")', { filename: "big.js", columnOffset: ${INT32_MAX} }).runInThisContext()`, + /big\.js:1:(-?\d+)/, + ], + [ + "line of a runtime error thrown by a compileFunction body", + `vm.compileFunction('throw new Error("q")', [], { filename: "big.js", lineOffset: ${INT32_MAX} })()`, + /big\.js:(-?\d+)/, + ], + [ + "line of a compile-time SyntaxError from compileFunction", + `vm.compileFunction("%%", [], { filename: "big.js", lineOffset: ${INT32_MAX} })`, + /big\.js:(-?\d+)/, + ], + ])("%s stays near the requested offset", async (_, expression, pattern) => { + const stdout = await runFixture(`try { ${expression}; } catch (e) { console.log(e.stack); }`); + const match = pattern.exec(stdout); + expect(match).not.toBeNull(); + // The offset is only pulled down by as much as the (tiny) source could + // possibly add to it, so the reported position stays just below INT32_MAX + // rather than wrapping negative or being dropped. + const position = Number(match![1]); + expect(position).toBeGreaterThan(INT32_MAX - 100); + expect(position).toBeLessThanOrEqual(INT32_MAX); + }); +});