Skip to content

Fix accessing properties defined by a bound function #2299

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
lkosson wants to merge 1 commit into from
Closed

Fix accessing properties defined by a bound function #2299

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions Jint/Native/Object/ObjectInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,8 @@ private static JsValue UnwrapFromGetter(PropertyDescriptor desc, JsValue thisObj
return Undefined;
}

if (getter is Function.BindFunction bindFunction) return bindFunction._engine.Call(bindFunction, thisObject);

Comment on lines +422 to +423

Copilot AI Mar 1, 2026

Copy link

Choose a reason for hiding this comment

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

This change fixes a specific regression scenario (accessor getter is a bound function), but the PR doesn’t add a regression test to ensure it stays fixed. Please add a test that defines an accessor via Object.defineProperty(..., { get: someFn.bind(...) }) and asserts the property access returns the expected value (and ideally that arguments.length inside the getter remains 0).

Copilot uses AI. Check for mistakes.
var functionInstance = (Function.Function) getter;
return functionInstance._engine.Call(functionInstance, thisObject);
Comment on lines +422 to 425

Copilot AI Mar 1, 2026

Copy link

Choose a reason for hiding this comment

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

bindFunction._engine.Call(bindFunction, thisObject) resolves to Engine.Call(JsValue, params JsValue[] arguments), meaning thisObject is passed as a function argument and the getter is invoked with this=undefined. That changes observable behavior (e.g., arguments.length inside the getter becomes 1) and isn’t the correct way to invoke an accessor getter.

Call the overload that sets the receiver as this and passes no arguments (e.g., Call(callable, thisObject, Arguments.Empty) / Call((ICallable)callable, thisObject, Arguments.Empty, ...)).

Suggested change
if (getter is Function.BindFunction bindFunction) return bindFunction._engine.Call(bindFunction, thisObject);
var functionInstance = (Function.Function) getter;
return functionInstance._engine.Call(functionInstance, thisObject);
if (getter is Function.BindFunction bindFunction) return bindFunction._engine.Call(bindFunction, thisObject, Arguments.Empty);
var functionInstance = (Function.Function) getter;
return functionInstance._engine.Call(functionInstance, thisObject, Arguments.Empty);

Copilot uses AI. Check for mistakes.
Comment on lines +422 to 425

Copilot AI Mar 1, 2026

Copy link

Choose a reason for hiding this comment

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

This accessor invocation path is still tightly coupled to specific concrete function types (BindFunction vs Function.Function) and will continue to throw InvalidCastException for any other callable getter that isn’t Function.Function. Elsewhere in this file (e.g., TryGetValue) getters are treated as ICallable.

Consider switching UnwrapFromGetter to use IsCallable/ICallable (and then invoking via Engine.Call) so all valid callable getter types are supported without adding more special-cases over time.

Suggested change
if (getter is Function.BindFunction bindFunction) return bindFunction._engine.Call(bindFunction, thisObject);
var functionInstance = (Function.Function) getter;
return functionInstance._engine.Call(functionInstance, thisObject);
if (!getter.IsCallable())
{
return Undefined;
}
var callable = (ICallable) getter;
return callable.Call(thisObject, Arguments.Empty);

Copilot uses AI. Check for mistakes.
}
Expand Down
Loading