Convert scope-object receivers in StringDecoder, node error toString, fs.Stats, Node-API and V8 callbacks

src/jsc/bindings/ErrorCode.cpp

@@ -52,7 +52,7 @@ JSC_DEFINE_HOST_FUNCTION(NodeError_proto_toString, (JSC::JSGlobalObject * global
 {
     auto& vm = JSC::getVM(globalObject);
     auto scope = DECLARE_THROW_SCOPE(vm);
-    auto thisVal = callFrame->thisValue();
+    auto thisVal = callFrame->thisValue().toThis(globalObject, JSC::ECMAMode::strict());
 
     auto name = thisVal.get(globalObject, vm.propertyNames->name);
     RETURN_IF_EXCEPTION(scope, {});

src/jsc/bindings/JSStringDecoder.cpp

@@ -580,9 +580,7 @@ JSC::EncodedJSValue JSStringDecoderConstructor::construct(JSC::JSGlobalObject* l
             return Bun::ERR::UNKNOWN_ENCODING(throwScope, lexicalGlobalObject, view);
         }
     }
-    JSValue thisValue = callFrame->newTarget();
     auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
-    JSObject* newTarget = asObject(thisValue);
     auto* constructor = globalObject->JSStringDecoder();
     Structure* structure = globalObject->JSStringDecoderStructure();
 
@@ -593,9 +591,10 @@ JSC::EncodedJSValue JSStringDecoderConstructor::construct(JSC::JSGlobalObject* l
     // This is a hack to make express' body-parser work
     // It does something weird with the prototype
     // Not exactly a subclass
-    if (constructor != newTarget && callFrame->thisValue().isObject()) {
+    JSValue thisValue = callFrame->thisValue().toThis(lexicalGlobalObject, JSC::ECMAMode::strict());
+    if (thisValue.isObject() && thisValue != constructor) {
         auto clientData = WebCore::clientData(vm);
-        JSObject* thisObject = asObject(callFrame->thisValue());
+        JSObject* thisObject = asObject(thisValue);
 
         thisObject->putDirect(vm, clientData->builtinNames().decodePrivateName(), jsObject, JSC::PropertyAttribute::DontEnum | 0);
         thisObject->putDirect(vm, clientData->builtinNames().encodingPublicName(), convertEnumerationToJS<BufferEncodingType>(*lexicalGlobalObject, encoding), JSC::PropertyAttribute::DontEnum | 0);

src/jsc/bindings/NodeFSStatBinding.cpp

@@ -139,7 +139,7 @@ static JSValue modeStatFunction(JSC::JSGlobalObject* globalObject, CallFrame* ca
 {
     auto& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
-    auto* thisObject = dynamicDowncast<JSObject>(callFrame->thisValue());
+    auto* thisObject = dynamicDowncast<JSObject>(callFrame->thisValue().toThis(globalObject, JSC::ECMAMode::strict()));
     if (!thisObject)
         return JSC::jsUndefined();
 
@@ -217,7 +217,7 @@ inline JSC::JSValue getDateField(JSC::JSGlobalObject* globalObject, JSC::Encoded
     auto& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSC::JSObject* thisObject = dynamicDowncast<JSC::JSObject>(JSC::JSValue::decode(thisValue));
+    JSC::JSObject* thisObject = dynamicDowncast<JSC::JSObject>(JSC::JSValue::decode(thisValue).toThis(globalObject, JSC::ECMAMode::strict()));
     if (!thisObject)
         return JSC::jsUndefined();
 
@@ -279,7 +279,7 @@ JSC_DEFINE_CUSTOM_GETTER(jsBigIntStatsPrototypeGetter_atime, (JSGlobalObject * g
 JSC_DEFINE_CUSTOM_SETTER(jsStatsPrototypeFunction_DatePutter, (JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue encodedValue, JSC::PropertyName propertyName))
 {
     auto& vm = globalObject->vm();
-    JSObject* thisObject = dynamicDowncast<JSObject>(JSValue::decode(thisValue));
+    JSObject* thisObject = dynamicDowncast<JSObject>(JSValue::decode(thisValue).toThis(globalObject, JSC::ECMAMode::strict()));
     if (!thisObject)
         return false;
 

src/jsc/bindings/napi.h

@@ -979,21 +979,11 @@ class NAPICallFrame {
         : m_callFrame(callFrame)
         , m_dataPtr(dataPtr)
     {
-        // Node-API function calls always run in "sloppy mode," even if the JS side is in strict
-        // mode. So if `this` is null or undefined, we use globalThis instead; otherwise, we convert
-        // `this` to an object.
-        // TODO change to global? or find another way to avoid JSGlobalProxy
-        JSC::JSObject* jscThis = globalObject->globalThis();
-        if (!m_callFrame->thisValue().isUndefinedOrNull()) {
-            // TopExceptionScope: this runs before the addon's callback and its
-            // first NAPI_PREAMBLE; a ThrowScope would simulate a throw on
-            // destruction that the next preamble would see as unchecked.
-            auto scope = DECLARE_TOP_EXCEPTION_SCOPE(JSC::getVM(globalObject));
-            jscThis = m_callFrame->thisValue().toObject(globalObject);
-            // https://tc39.es/ecma262/#sec-toobject
-            // toObject only throws for undefined and null, which we checked for
-            scope.assertNoException();
-        }
+        // Node-API function calls always run in "sloppy mode," even if the JS side is in strict mode.
+        // Not a ThrowScope: its simulated throw would reach the addon's first NAPI_PREAMBLE unchecked.
+        auto scope = DECLARE_TOP_EXCEPTION_SCOPE(JSC::getVM(globalObject));
+        JSValue jscThis = m_callFrame->thisValue().toThis(globalObject, JSC::ECMAMode::sloppy());
+        scope.assertNoException();
         m_callFrame->setThisValue(jscThis);
     }
 

src/jsc/bindings/v8/shim/FunctionTemplate.cpp

@@ -145,14 +145,8 @@ JSC::EncodedJSValue FunctionTemplate::functionCall(JSC::JSGlobalObject* globalOb
 {
     auto* callee = dynamicDowncast<Function>(callFrame->jsCallee());
 
-    // V8 function calls always run in "sloppy mode," even if the JS side is in strict mode. So if
-    // `this` is null or undefined, we use globalThis instead; otherwise, we convert `this` to an
-    // object.
-    JSC::JSObject* jscThis = globalObject->globalThis();
-    if (!callFrame->thisValue().isUndefinedOrNull()) {
-        // TODO(@190n) throwscope, assert no exception
-        jscThis = callFrame->thisValue().toObject(globalObject);
-    }
+    // V8 function calls always run in "sloppy mode," even if the JS side is in strict mode.
+    JSC::JSObject* jscThis = JSC::asObject(callFrame->thisValue().toThis(globalObject, JSC::ECMAMode::sloppy()));
 
     JSC::ArgList args(callFrame);
     return JSValue::encode(invokeCallback(globalObject, callee, jscThis, args, false));

src/jsc/bindings/v8/shim/TemplateProperty.cpp

@@ -61,12 +61,8 @@ static JSC::JSValue invokeAccessor(
 
     HandleScope hs(isolate);
 
-    JSC::JSObject* jscThis = globalObject->globalThis();
-    if (!thisObject.isUndefinedOrNull()) {
-        jscThis = thisObject.toObject(globalObject);
-        if (!jscThis) [[unlikely]]
-            return JSC::jsUndefined();
-    }
+    // Sloppy-mode receiver, as in FunctionTemplate::functionCall.
+    JSC::JSObject* jscThis = JSC::asObject(thisObject.toThis(globalObject, JSC::ECMAMode::sloppy()));
     Local<v8::Object> holder = hs.createLocal<v8::Object>(vm, jscThis);
     Local<v8::Name> property = hs.createLocal<v8::Name>(vm, name);
     Local<v8::Value> dataLocal = hs.createLocal<v8::Value>(vm, data);

test/js/node/errors/error-code-toString-receiver.test.ts

@@ -0,0 +1,72 @@
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe } from "harness";
+import { readFileSync } from "node:fs";
+
+// Errors with a Node `code` get their prototype, including `toString`, from the native
+// error table (src/jsc/bindings/ErrorCode.cpp).
+function nodeError(): Error & { code: string } {
+  try {
+    // @ts-expect-error the missing argument is the point
+    readFileSync();
+  } catch (e) {
+    return e as Error & { code: string };
+  }
+  throw new Error("readFileSync() did not throw");
+}
+
+describe("node error toString()", () => {
+  test("formats name, code and message", () => {
+    const err = nodeError();
+    expect({ code: err.code, string: err.toString() }).toEqual({
+      code: "ERR_INVALID_ARG_TYPE",
+      string: `TypeError [ERR_INVALID_ARG_TYPE]: ${err.message}`,
+    });
+  });
+
+  // A call resolved through a binding that a closure captures is compiled with the scope
+  // object itself in the this slot; native functions see it raw. toString() used to read
+  // name/code/message off that scope object and return "undefined [undefined]: undefined".
+  // Node's toString is strict-mode JS, so it throws on the undefined receiver instead.
+  test("called without a receiver through a captured binding throws like Node", () => {
+    const { toString } = nodeError();
+    function keep() {
+      return toString;
+    }
+    expect(() => toString()).toThrow(TypeError);
+    expect(keep()).toBe(toString);
+  });
+
+  // Reading a binding that is still in its temporal dead zone through the scope object yields
+  // an empty value rather than a TDZ error, and toString() crashed the process (SIGSEGV at
+  // address 0x5) when it tried to stringify it.
+  test("called through a scope whose `name` binding is in its TDZ does not crash", async () => {
+    const src = `
+      let toString;
+      try {
+        require("node:fs").readFileSync();
+      } catch (e) {
+        ({ toString } = e);
+      }
+      let result;
+      try {
+        result = toString();
+      } catch (e) {
+        result = e.constructor.name;
+      }
+      console.log(result);
+      let name = "initialized only after the call above";
+      function keep() {
+        return [toString, name];
+      }
+      keep();
+    `;
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", src],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout, stderr, exitCode }).toEqual({ stdout: "TypeError\n", stderr: "", exitCode: 0 });
+  });
+});

test/js/node/fs/fs-stats-constructor.test.ts

@@ -1,4 +1,5 @@
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe } from "harness";
 import { Stats, statSync } from "node:fs";
 
 // Node.js's Stats constructor signature (deprecated, DEP0180):
@@ -54,3 +55,79 @@ test("Stats instances share Stats.prototype", () => {
   expect(Object.getPrototypeOf(bigint).constructor.name).toBe("BigIntStats");
   expect(bigint instanceof Object.getPrototypeOf(bigint).constructor).toBe(true);
 });
+
+// A call resolved through a binding that a closure captures is compiled with the scope object
+// itself in the this slot, and native functions see it raw. isFile() and friends used to read
+// `mode` out of that scope object (the date getters, once pulled out of their property descriptor,
+// `atimeMs` and so on): a missing binding produced a bogus answer, and a binding still in its
+// temporal dead zone crashed the process. Such a receiver now gets the same answer as any other
+// non-object receiver.
+describe("Stats methods and accessors called without a receiver", () => {
+  test("a mode method called through a captured binding is treated like an undefined receiver", () => {
+    const stats = statSync(import.meta.path);
+    const bigintStats = statSync(import.meta.dir, { bigint: true });
+    const { isFile } = stats;
+    const { isDirectory } = bigintStats;
+    function keep() {
+      return [isFile, isDirectory];
+    }
+    expect({
+      bare: [isFile(), isDirectory()],
+      undefinedReceiver: [isFile.call(undefined), isDirectory.call(undefined)],
+      statsReceiver: [isFile.call(stats), isDirectory.call(bigintStats)],
+    }).toEqual({
+      bare: [undefined, undefined],
+      undefinedReceiver: [undefined, undefined],
+      statsReceiver: [true, true],
+    });
+    expect(keep()).toEqual([isFile, isDirectory]);
+  });
+
+  test("a date getter called through a captured binding is treated like an undefined receiver", () => {
+    const stats = statSync(import.meta.path);
+    const bigintStats = statSync(import.meta.path, { bigint: true });
+    const atime = Object.getOwnPropertyDescriptor(Stats.prototype, "atime")!.get!;
+    const bigintMtime = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(bigintStats), "mtime")!.get!;
+    function keep() {
+      return [atime, bigintMtime];
+    }
+    expect({
+      bare: [atime(), bigintMtime()],
+      undefinedReceiver: [atime.call(undefined), bigintMtime.call(undefined)],
+      statsReceiver: [atime.call(stats), bigintMtime.call(bigintStats)],
+    }).toEqual({
+      bare: [undefined, undefined],
+      undefinedReceiver: [undefined, undefined],
+      statsReceiver: [stats.atime, bigintStats.mtime],
+    });
+    expect(keep()).toEqual([atime, bigintMtime]);
+  });
+
+  test("calls through a scope whose `mode` and `atimeMs` bindings are in their TDZ do not crash", async () => {
+    const src = `
+      const { Stats, statSync } = require("node:fs");
+      const { isFile } = statSync(${JSON.stringify(import.meta.path)});
+      const { isDirectory } = statSync(${JSON.stringify(import.meta.dir)}, { bigint: true });
+      const atime = Object.getOwnPropertyDescriptor(Stats.prototype, "atime").get;
+      console.log(isFile(), isDirectory(), atime());
+      let mode = 0;
+      let atimeMs = 0;
+      function keep() {
+        return [isFile, isDirectory, atime, mode, atimeMs];
+      }
+      keep();
+    `;
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", src],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout, stderr, exitCode }).toEqual({
+      stdout: "undefined undefined undefined\n",
+      stderr: "",
+      exitCode: 0,
+    });
+  });
+});

test/js/node/string_decoder/string-decoder.test.js

@@ -264,6 +264,50 @@ it("normalizes the encoding name like Node", () => {
   });
 });
 
+// A call resolved through a binding that a closure captures (or through a module binding) is
+// compiled with the scope object itself in the this slot, and native callees see it raw.
+// StringDecoder called without new used to mistake that scope object for a body-parser style
+// receiver: it wrote the decoder state onto it and returned it instead of a decoder.
+describe("StringDecoder called without new", () => {
+  it("returns a decoder when the callee is resolved through a captured binding", () => {
+    const StringDecoder = RealStringDecoder;
+    function keep() {
+      return StringDecoder;
+    }
+    const decoder = StringDecoder("latin1");
+    expect(decoder).toBeInstanceOf(RealStringDecoder);
+    expect(decoder.encoding).toBe("latin1");
+    expect(decoder.write(Buffer.from([0xe9]))).toBe("é");
+    expect(keep()).toBe(RealStringDecoder);
+  });
+
+  it("still initializes an explicit object receiver (body-parser style)", () => {
+    const receiver = {};
+    expect(RealStringDecoder.call(receiver, "latin1")).toBe(receiver);
+    expect(receiver.encoding).toBe("latin1");
+  });
+
+  // The constructor used to call asObject() on the receiver before checking that it is one, which
+  // aborts a debug build for any non-object receiver. Run it in a child so the abort shows up as a
+  // failed assertion instead of taking the test runner down.
+  it("returns a decoder for an undefined or primitive receiver", async () => {
+    const src = `
+      const { StringDecoder } = require("node:string_decoder");
+      const local = StringDecoder;
+      const results = [local("utf8"), StringDecoder.call(undefined, "utf8"), StringDecoder.call("x", "utf8")];
+      console.log(results.every(d => d instanceof StringDecoder && d.encoding === "utf8"));
+    `;
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", src],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout, stderr, exitCode }).toEqual({ stdout: "true\n", stderr: "", exitCode: 0 });
+  });
+});
+
 // Node's lastTotal getter is MissingBytes + BufferedBytes; Node clears BufferedBytes when a buffered
 // partial is emitted so lastTotal returns to 0. end() resets lastNeed/lastTotal but leaves lastChar.
 // Decoded output was always correct; this is purely about the observable legacy state triple.

test/napi/napi-app/js_test_helpers.cpp

@@ -663,7 +663,19 @@ static napi_value test_reference_unref_underflow(const Napi::CallbackInfo &info)
   return result;
 }
 
+// Returns the this_arg that napi_get_cb_info reports for this call, so JS can
+// check what a callback sees as its receiver for different call shapes.
+static napi_value return_this(const Napi::CallbackInfo &info) {
+  napi_env env = info.Env();
+  napi_value this_arg;
+  NODE_API_CALL(env,
+                napi_get_cb_info(env, static_cast<napi_callback_info>(info),
+                                 nullptr, nullptr, &this_arg, nullptr));
+  return this_arg;
+}
+
 void register_js_test_helpers(Napi::Env env, Napi::Object exports) {
+  REGISTER_FUNCTION(env, exports, return_this);
   REGISTER_FUNCTION(env, exports, create_ref_with_finalizer);
   REGISTER_FUNCTION(env, exports, was_finalize_called);
   REGISTER_FUNCTION(env, exports, call_and_get_exception);

test/napi/napi-app/module.js

@@ -766,6 +766,22 @@ nativeTests.test_type_tag = () => {
   console.log("o2 matches o2:", nativeTests.check_tag(o2, 3, 4));
 };
 
+nativeTests.test_this_value_of_bare_call_through_closure = () => {
+  const { return_this } = nativeTests;
+  // return_this is captured by keep, so the bare call below is resolved through the closure's
+  // scope object, which JSC leaves in the call's this slot. A Node-API callback must still see
+  // the sloppy-mode receiver (globalThis), never that scope object.
+  function keep() {
+    return return_this;
+  }
+  console.log("bare call through closure returned globalThis:", return_this() === globalThis);
+  console.log("call(undefined) returned globalThis:", return_this.call(undefined) === globalThis);
+  console.log("call(5) returned a Number object:", return_this.call(5) instanceof Number);
+  const receiver = {};
+  console.log("call(receiver) returned receiver:", return_this.call(receiver) === receiver);
+  keep();
+};
+
 nativeTests.test_napi_class = () => {
   const NapiClass = nativeTests.get_class_with_constructor();
   const instance = new NapiClass();

test/napi/napi.test.ts

@@ -1133,6 +1133,13 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => {
     });
   });
 
+  describe("napi_get_cb_info this_arg", () => {
+    it("is globalThis for a bare call resolved through a closure scope", async () => {
+      const output = await checkSameOutput("test_this_value_of_bare_call_through_closure", []);
+      expect(output).toContain("bare call through closure returned globalThis: true");
+    });
+  });
+
   describe("bigint conversion to int64/uint64", () => {
     it("works", async () => {
       const tests = [-1n, 0n, 1n];