Skip to content

Fix crash constructing jest.fn() via Reflect.construct #30212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 4 commits into from
Closed

Fix crash constructing jest.fn() via Reflect.construct #30212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
920bc96
Fix crash constructing jest.fn() via Reflect.construct
robobun May 4, 2026
512bebd
Always use createSubclassStructure so fn.prototype is honored on new …
robobun May 4, 2026
0fb8200
Add regression test for constructing a spy
robobun May 24, 2026
daaa8d9
Add regression test for constructing a spy on a missing property
robobun May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions src/jsc/bindings/JSMockFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ inline To tryJSDynamicCast(JSC::WriteBarrier<WriteBarrierT>& from)
}

JSC_DECLARE_HOST_FUNCTION(jsMockFunctionCall);
JSC_DECLARE_HOST_FUNCTION(jsMockFunctionConstruct);
JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_protoImpl);
JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_mock);
JSC_DECLARE_HOST_FUNCTION(jsMockFunctionGetter_mockGetLastCall);
Expand Down Expand Up @@ -462,7 +463,7 @@ class JSMockFunction : public JSC::InternalFunction {
}

JSMockFunction(JSC::VM& vm, JSC::Structure* structure, CallbackKind wrapKind)
: Base(vm, structure, jsMockFunctionCall, jsMockFunctionCall)
: Base(vm, structure, jsMockFunctionCall, jsMockFunctionConstruct)
{
initMock();
}
Expand Down Expand Up @@ -826,7 +827,7 @@ static JSValue createMockResult(JSC::VM& vm, Zig::GlobalObject* globalObject, co
return result;
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
static EncodedJSValue jsMockFunctionCallOrConstruct(JSGlobalObject* lexicalGlobalObject, CallFrame* callframe, bool isConstructCall)
{
Zig::GlobalObject* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
auto& vm = JSC::getVM(globalObject);
Expand All @@ -839,6 +840,26 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObje

JSC::ArgList args = JSC::ArgList(callframe);
JSValue thisValue = callframe->thisValue();

if (isConstructCall) {
JSValue newTarget = callframe->newTarget();
if (JSObject* newTargetObject = newTarget.getObject()) {
JSGlobalObject* functionGlobalObject = getFunctionRealm(globalObject, newTargetObject);
RETURN_IF_EXCEPTION(scope, {});
Structure* structure = InternalFunction::createSubclassStructure(globalObject, newTargetObject, functionGlobalObject->objectStructureForObjectConstructor());
RETURN_IF_EXCEPTION(scope, {});
thisValue = constructEmptyObject(vm, structure);
} else {
thisValue = constructEmptyObject(globalObject);
}
}
Comment on lines +844 to +855

@claude claude Bot May 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing nit (not introduced by this PR): now that the construct path allocates a fresh thisValue, this would be the natural place to also push it onto fn->instances so that mock.instances is populated on new fn() like Jest does — currently instances is exposed but never written to anywhere, so it always stays empty. Fine to leave for a follow-up since this PR is scoped to the crash fix.

Extended reasoning...

What the gap is

JSMockFunction declares a mutable JSC::WriteBarrier<JSC::JSArray> instances field, lazily creates it in getInstances(), clears it in clear(), visits it for GC, and exposes it as mock.instances on the mock object structure. However, nothing in jsMockFunctionCallOrConstruct (or anywhere else in the file) ever pushes into it. Grepping the file confirms the only references to instances are the declaration, clear(), getInstances(), the GC visitor, and the mockObjectStructure slot — none of them are writes during invocation. So mock.instances is always an empty array.

In Jest, mockFn.mock.instances is "an array that contains all the object instances that have been instantiated from this mock function using new" — every new fn() pushes the constructed this onto it.

Code path

In the new construct branch:

if (isConstructCall) {
    ...
    thisValue = constructEmptyObject(...);
}

the freshly allocated thisValue is later pushed onto fn->contexts (alongside calls, invocationCallOrder, and returnValues), but there is no corresponding push onto fn->instances. The fix would be to add the same lazy-create-then-push pattern for fn->instances inside the isConstructCall block (or right after contexts is pushed, gated on isConstructCall), pushing thisValue.

Why nothing currently prevents it

There simply is no write site. The infrastructure (storage, getter, GC visiting, mock-object slot) is all wired up, but the population step was never implemented.

Step-by-step proof

  1. const fn = jest.fn();
  2. new fn(); → enters jsMockFunctionConstruct → jsMockFunctionCallOrConstruct(..., true).
  3. isConstructCall is true, so thisValue = constructEmptyObject(globalObject).
  4. calls, contexts, invocationCallOrder are pushed; returnValues is pushed via setReturnValue. instances is not touched.
  5. fn.mock.instances → getInstances() lazily creates an empty array and returns it.
  6. expect(fn.mock.instances).toHaveLength(1) fails in Bun, passes in Jest.

Why this is pre-existing

Before this PR, instances was already never populated, and new (jest.fn(() => ({})))() already worked without crashing (the impl returned an object, so asObject() succeeded), so the empty mock.instances was already observable. This PR is a narrow Fuzzilli crash fix and doesn't change instances behavior at all — it just adds the construct branch where the missing push would naturally live, which is why it's worth flagging here as a follow-up rather than a blocker.

Comment on lines +844 to +855

@claude claude Bot May 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing (not introduced by this PR): even with isConstructCall true, the Kind::Call branch still invokes the implementation via Bun::call(...) — a regular [[Call]] — so spying on / mocking an ES class constructor and then new-ing it throws Class constructor Foo cannot be invoked without 'new', whereas Jest uses new impl(...args) on construct. The new "constructing a spy works" test only passes because it uses a plain function() {} rather than a class. Since this PR adds the isConstructCall flag and a proper thisValue right at this site, it'd be the natural place to branch to JSC::construct(globalObject, impl, args, newTarget) when isConstructCall and the impl is constructible — fine to leave for a follow-up alongside the mock.instances note since this PR is scoped to the crash fix.

Extended reasoning...

What the gap is

In the JSMockImplementation::Kind::Call branch of jsMockFunctionCallOrConstruct, the implementation is always invoked as:

JSC::CallData callData = JSC::getCallData(result);
...
JSValue returnValue = Bun::call(globalObject, result, callData, thisValue, args);

Bun::call performs an ordinary [[Call]], never [[Construct]], regardless of isConstructCall. ES class constructors have valid CallData (so the Type::None guard doesn't trip), but per spec they throw TypeError: Class constructor X cannot be invoked without 'new' when invoked via [[Call]]. So a mock or spy whose implementation is an ES class can be called fine, but throws as soon as it's used with new / Reflect.construct.

In Jest, mock functions are ordinary JS functions whose body checks new.target and does new impl(...args) when invoked as a constructor, so spying on a class and then new-ing it works.

Concrete trigger / step-by-step

class Foo { constructor() { this.x = 1 } }
const obj = { Foo };
jest.spyOn(obj, 'Foo');
new obj.Foo();
  1. spyOn(obj, 'Foo') creates a JSMockFunction and pushes a Kind::Call implementation whose underlyingValue is the original class Foo.
  2. new obj.Foo() enters jsMockFunctionConstruct → jsMockFunctionCallOrConstruct(..., /* isConstructCall */ true).
  3. isConstructCall is true, so a fresh thisValue is allocated via createSubclassStructure — good so far.
  4. The Kind::Call branch fetches result = class Foo, then getCallData(result) returns a JS call data (classes are callable in the getCallData sense), so the Type::None check passes.
  5. Bun::call(globalObject, result, callData, thisValue, args) performs [[Call]] on class Foo → throws TypeError: Class constructor Foo cannot be invoked without 'new'.
  6. The exception is recorded in mock.results as a throw and re-thrown.

The same applies to jest.fn(class Foo {}) followed by new fn().

Why nothing currently prevents it

The new isConstructCall flag is consulted only for allocating thisValue and for the encodedReturnValue non-object fixup; it does not influence how the implementation is invoked. There is no JSC::getConstructData / JSC::construct path anywhere in this function. The "constructing a spy works" test added in this PR uses function () { this.ok = true }, which is happy to be [[Call]]ed with the allocated thisValue, so it passes — but swapping it for class { constructor() { this.ok = true } } would fail.

Why this is pre-existing

Before this PR the construct entry point was the same jsMockFunctionCall host function and hit the exact same Bun::call line, so new obj.Foo() on a class spy already threw the same TypeError (when it didn't crash earlier for the unrelated isCell() reason this PR fixes). The PR does not change this behavior; it refactors the surrounding code and adds the isConstructCall flag right next to where the fix would go, which is why it's worth flagging here as a follow-up rather than a regression.

Suggested fix

Inside Kind::Call, when isConstructCall and JSC::getConstructData(result).type != JSC::CallData::Type::None, use JSC::construct(globalObject, result, args, callframe->newTarget()) instead of Bun::call(...), then run the result through encodedReturnValue. Optionally, when the impl is constructible, prefer the constructor-allocated instance (the value construct returns) over the pre-allocated thisValue so fn.prototype of the implementation class is honored. This pairs naturally with the existing pre-existing note about populating mock.instances.

Severity

Pre-existing, follow-up. The PR is scoped to the Fuzzilli isCell() crash and doesn't claim to fix class-constructor spying, so this should not block merge — filing alongside the existing mock.instances pre-existing note.

@robobun robobun May 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — good catch. Invoking the implementation with [[Construct]] (via JSC::getConstructData/JSC::construct) when isConstructCall and the impl is constructible is the right follow-up, and this is the natural spot for it now that the flag and thisValue exist here. Keeping this PR scoped to the isCell() crash fix; the class-constructor spying gap and the mock.instances population can go together in a follow-up.


auto encodedReturnValue = [&](JSValue value) -> EncodedJSValue {
if (isConstructCall && !value.isObject())
return JSValue::encode(thisValue);
return JSValue::encode(value);
};

JSC::JSArray* argumentsArray = nullptr;
{
JSC::ObjectInitializationScope object(vm);
Expand Down Expand Up @@ -954,22 +975,22 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObje
fn->returnValues.set(vm, fn, returnValuesArray);
}

return JSValue::encode(returnValue);
return encodedReturnValue(returnValue);
}
case JSMockImplementation::Kind::ReturnValue: {
JSValue returnValue = impl->underlyingValue.get();
setReturnValue(createMockResult(vm, globalObject, "return"_s, returnValue));
return JSValue::encode(returnValue);
return encodedReturnValue(returnValue);
}
case JSMockImplementation::Kind::ReturnThis: {
setReturnValue(createMockResult(vm, globalObject, "return"_s, thisValue));
return JSValue::encode(thisValue);
return encodedReturnValue(thisValue);
}
case JSMockImplementation::Kind::RejectedValue: {
JSValue rejectedPromise = JSC::JSPromise::rejectedPromise(globalObject, impl->underlyingValue.get());
RETURN_IF_EXCEPTION(scope, {});
setReturnValue(createMockResult(vm, globalObject, "return"_s, rejectedPromise));
return JSValue::encode(rejectedPromise);
return encodedReturnValue(rejectedPromise);
}
default: {
RELEASE_ASSERT_NOT_REACHED();
Expand All @@ -978,7 +999,17 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObje
}

setReturnValue(createMockResult(vm, globalObject, "return"_s, jsUndefined()));
return JSValue::encode(jsUndefined());
return encodedReturnValue(jsUndefined());
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
{
return jsMockFunctionCallOrConstruct(lexicalGlobalObject, callframe, false);
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionConstruct, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
{
return jsMockFunctionCallOrConstruct(lexicalGlobalObject, callframe, true);
}

void JSMockFunctionPrototype::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
Expand Down
68 changes: 68 additions & 0 deletions test/js/bun/test/mock-fn.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,51 @@ describe("mock()", () => {
const obj = { fn };
expect(obj.fn()).toBe(obj);
});
test("Reflect.construct on mock with no implementation returns an object", () => {
const fn = jest.fn();
const result = Reflect.construct(fn, []);
expect(typeof result).toBe("object");
expect(result).not.toBeNull();
});
test("Reflect.construct on mock returning a primitive returns an object", () => {
const fn = jest.fn(() => 42);
const result = Reflect.construct(fn, []);
expect(typeof result).toBe("object");
expect(result).not.toBeNull();

const fn2 = jest.fn().mockReturnValue("hello");
const result2 = Reflect.construct(fn2, []);
expect(typeof result2).toBe("object");
expect(result2).not.toBeNull();
});
test("Reflect.construct on mock returning an object returns that object", () => {
const obj = { foo: "bar" };
const fn = jest.fn(() => obj);
expect(Reflect.construct(fn, [])).toBe(obj);
expect(new fn()).toBe(obj);
});
test("Reflect.construct on mock uses newTarget prototype", () => {
function NewTarget() {}
NewTarget.prototype = { marker: true };
const fn = jest.fn();
const result = Reflect.construct(fn, [], NewTarget);
expect(Object.getPrototypeOf(result)).toBe(NewTarget.prototype);
});
test("new on mock uses assigned prototype", () => {
const fn = jest.fn();
const proto = { marker: true };
fn.prototype = proto;
const result = new fn();
expect(Object.getPrototypeOf(result)).toBe(proto);
});
test("new on mock passes this to the implementation", () => {
const fn = jest.fn(function () {
this.x = 1;
});
const result = new fn();
expect(result).toEqual({ x: 1 });
expect(fn.mock.contexts[0]).toBe(result);
});
if (isBun) {
test("jest.fn(10) return value shorthand", () => {
expect(jest.fn(10)()).toBe(10);
Expand Down Expand Up @@ -828,6 +873,29 @@ describe("spyOn", () => {
expect(fn).not.toHaveBeenCalled();
});

test("constructing a spy works", () => {
var obj = {
Original: function () {
this.ok = true;
},
};
const fn = spyOn(obj, "Original");
const instance = Reflect.construct(obj.Original, []);
expect(typeof instance).toBe("object");
expect(instance.ok).toBe(true);
expect(fn).toHaveBeenCalledTimes(1);
});

if (isBun) {
test("constructing a spy on a missing property returns an object", () => {
const target = {};
const fn = spyOn(target, "doesNotExist");
expect(typeof Reflect.construct(fn, [])).toBe("object");
expect(typeof new fn()).toBe("object");
fn.mockRestore();
});
}

test("override impl after doesnt break restore", () => {
var obj = {
original() {
Expand Down
Loading