NativeProxy: fix set trap not passing receiver object by rbri · Pull Request #2347 · mozilla/rhino

rbri · 2026-03-22T10:27:34Z

Co-authored-by: Lai Quang Duong

This

passes the receiver to the set trap
passes the receiver to the get trap
fixes an StackOverflow exception (became visible after the change for 'passes the receiver to the set trap')
adds a bunch of spec hints to the implementation of the NativeReflect class
implements a workaround for the missing put return value in class NativeProxy, this workaround is used by NativeReflect to implement a more correct behaviour
fixes a check in NativeReflect; now checks for Callable in sync with the spec

This does not make so many more 262 test to pass, but i think is at least a small step forward.

rbri · 2026-03-22T10:56:29Z

will work on the failing tests

Co-authored-by: Lai Quang Duong

rbri · 2026-05-28T20:40:24Z

This is much bigger than expected, i have to fix the NativeReflect impl (inf fact bring it closer to the spec) to fix the StackOverflow.
Hope i have some time tomorrow to work on this.

…low in set-mutable-binding-idref-with-proxy-env.js additionally limited return code handling for put for NativeProxy was implemented and used by NativeReflect. This fixes some more tests

rbri · 2026-05-29T14:07:50Z

@duonglaiquang sorry for being late on this, but is was harder than expected. My plan is to make a HtmlUnit 5.1.0 release this weekend, i fear this will not be reviewed/merged for the release. Hope this is ok for you...

Copilot

Pull request overview

Updates Rhino’s Proxy/Reflect behavior around receiver forwarding (notably for Proxy set and Reflect get/set) and expands the ES6 test suite to cover receiver-sensitive semantics, aiming to align more closely with ECMA-262.

Changes:

Add/expand JUnit coverage for Reflect.get, Reflect.set, and Proxy set receiver behavior (including with(proxy) scenarios).
Refactor parts of NativeReflect to forward receivers and inline portions of OrdinarySetWithOwnDescriptor.
Introduce NativeProxy.putAndReturn(...) helpers to expose a boolean result for use by Reflect.set.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 27 comments.

Show a summary per file

File	Description
tests/testsrc/test262.properties	Updates recorded test262 support/unsupported counts for Proxy/Reflect.
tests/src/test/java/org/mozilla/javascript/tests/es6/NativeReflectTest.java	Adds extensive coverage for `Reflect.get`/`Reflect.set` receiver semantics and related invariants.
tests/src/test/java/org/mozilla/javascript/tests/es6/NativeProxyTest.java	Adds tests asserting Proxy `set` trap receives a receiver and exercises `with(proxy)` behavior.
rhino/src/main/java/org/mozilla/javascript/NativeReflect.java	Adjusts Reflect operations to use receiver-aware access and implements a custom `set` path.
rhino/src/main/java/org/mozilla/javascript/NativeProxy.java	Adds `putAndReturn(...)` overloads and changes `set` trap invocation argument list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

        Function trap = getTrap(TRAP_SET);
        if (trap != null) {
            boolean booleanTrapResult =
-                    ScriptRuntime.toBoolean(callTrap(trap, new Object[] {target, name, value}));
+                    ScriptRuntime.toBoolean(
+                            callTrap(trap, new Object[] {target, name, value, this}));
            if (!booleanTrapResult) {


        Function trap = getTrap(TRAP_SET);
        if (trap != null) {
            boolean booleanTrapResult =
                    ScriptRuntime.toBoolean(
                            callTrap(
                                    trap,
-                                    new Object[] {target, ScriptRuntime.toString(index), value}));
+                                    new Object[] {
+                                        target, ScriptRuntime.toString(index), value, this
+                                    }));


        Function trap = getTrap(TRAP_SET);
        if (trap != null) {
            boolean booleanTrapResult =
-                    ScriptRuntime.toBoolean(callTrap(trap, new Object[] {target, key, value}));
+                    ScriptRuntime.toBoolean(
+                            callTrap(trap, new Object[] {target, key, value, this}));
            if (!booleanTrapResult) {


+        // receiver == target (or target has no own property P):
+        // delegate to [[Set]] on the receiver, which correctly fires any proxy
+        // set trap when receiver/target is a Proxy.
+
+        // For a NativeProxy, we use putAndReturn() to capture the result.
+        if (target instanceof NativeProxy proxyTarget) {
+            if (key instanceof Symbol) {
+                return proxyTarget.putAndReturn((Symbol) key, receiver, args[2]);
            }
+
+            StringIdOrIndex soi = ScriptRuntime.toStringIdOrIndex(key);
+            if (soi.stringId == null) {
+                return proxyTarget.putAndReturn(soi.index, receiver, args[2]);
+            }
+            return proxyTarget.putAndReturn(soi.stringId, receiver, args[2]);
        }

-        if (ScriptRuntime.isSymbol(args[1])) {
-            receiver.put((Symbol) args[1], receiver, args[2]);
+        // For a plain ScriptableObject put() has no return value, we
+        // assume true.
+        if (key instanceof Symbol) {
+            receiver.put((Symbol) key, receiver, args[2]);
        } else {
-            StringIdOrIndex soi = ScriptRuntime.toStringIdOrIndex(args[1]);
+            StringIdOrIndex soi = ScriptRuntime.toStringIdOrIndex(key);
            if (soi.stringId == null) {
                receiver.put(soi.index, receiver, args[2]);
            } else {
                receiver.put(soi.stringId, receiver, args[2]);
            }
        }


+        // Step 3: default receiver to target when not supplied.
+        Scriptable receiver =
+                (args.length > 2 && args[2] instanceof Scriptable) ? (Scriptable) args[2] : target;



+        // Step 3: default receiver to target when not supplied.
+        Scriptable receiver =
+                (args.length > 2 && args[2] instanceof Scriptable) ? (Scriptable) args[2] : target;



+        // Step 3: default receiver to target when not supplied (args[3]).
+        // Note: receiver is intentionally typed as Scriptable, not ScriptableObject,
+        // because the caller may pass any object (e.g. a Proxy) as the receiver.
+        Scriptable receiver = args.length > 3 ? ScriptableObject.ensureScriptable(args[3]) : target;
+


        ScriptableObject target = checkTarget(args);
        if (args.length < 2) {
            return true;
        }


+        // Step 3: default receiver to target when not supplied (args[3]).
+        // Note: receiver is intentionally typed as Scriptable, not ScriptableObject,
+        // because the caller may pass any object (e.g. a Proxy) as the receiver.
+        Scriptable receiver = args.length > 3 ? ScriptableObject.ensureScriptable(args[3]) : target;
+


        ScriptableObject target = checkTarget(args);
        if (args.length < 2) {
            return true;
        }


rbri · 2026-05-30T08:09:24Z

will work on all the sugestions... at least some looking correct ;-)

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

+        if (!(args[0] instanceof Callable)) {
+            throw ScriptRuntime.notFunctionError(args[0]);
        }
+        Scriptable callable = (Scriptable) args[0];


        Object key = args[1];
-
        try {
            if (key instanceof Symbol) {
                return target.defineOwnProperty(cx, key, desc);
-            } else {
-                String propertyKey =
-                        ScriptRuntime.toString(
-                                ScriptRuntime.toPrimitive(key, ScriptRuntime.StringClass));
-                return target.defineOwnProperty(cx, propertyKey, desc);
            }
-
+            String propertyKey =
+                    ScriptRuntime.toString(
+                            ScriptRuntime.toPrimitive(key, ScriptRuntime.StringClass));
+            return target.defineOwnProperty(cx, propertyKey, desc);
        } catch (EcmaError e) {
+            // Rhino throws where the spec says [[DefineOwnProperty]] should return false.
+            // Map those back to the spec-correct false return value.
            return false;
        }


        ScriptableObject target = checkTarget(args);
        if (args.length < 2) {
            return true;
        }



+            key = ScriptRuntime.toString(args[1]); // coerce to String
+        }
+
        if (receiver != target) {


+            if (receiver instanceof ScriptableObject receiverObj) {
+                DescriptorInfo newDesc = new DescriptorInfo(true, true, true, args[2]);
+                receiverObj.defineOwnProperty(cx, key, newDesc);
+            }
+            return true;


+                receiverObj.defineOwnProperty(cx, key, valueDesc);
+                return true;


+            receiverObj.defineOwnProperty(cx, key, newDesc);
+            return true;


…_receiver

rbri · 2026-05-30T13:43:04Z

this seems to be a rabbit hole, i have to think about a good way to fix this step by step. Back to Draft...

rbri mentioned this pull request Mar 22, 2026

Fixes and improvements for proxy and reflect HtmlUnit/htmlunit-rhino-fork#19

Open

rbri marked this pull request as draft March 22, 2026 10:56

NativeProxy: fix set trap not passing receiver object

53d53f4

Co-authored-by: Lai Quang Duong

rbri force-pushed the set_trap_receiver branch from ea6714b to 53d53f4 Compare May 29, 2026 06:16

rbri added 2 commits May 29, 2026 08:22

add docu from the spec

9439264

bring NativeReflect.set closer to the spec, this fixes the StackOverf…

0dab4f7

…low in set-mutable-binding-idref-with-proxy-env.js additionally limited return code handling for put for NativeProxy was implemented and used by NativeReflect. This fixes some more tests

rbri force-pushed the set_trap_receiver branch from 1b266f5 to 0dab4f7 Compare May 29, 2026 10:21

check for Callabe not Scriptable; use the receiver argument

3b65316

rbri force-pushed the set_trap_receiver branch from 2501309 to 0bd9610 Compare May 29, 2026 13:38

Reflect get trap has to take care of the receiver

5fdb19e

rbri force-pushed the set_trap_receiver branch from 0bd9610 to 5fdb19e Compare May 29, 2026 13:39

rbri marked this pull request as ready for review May 29, 2026 14:05

rbri requested a review from Copilot May 30, 2026 07:20

Copilot AI reviewed May 30, 2026

View reviewed changes

rbri added 3 commits May 30, 2026 11:05

fix trap receiver for get/set

2d611bf

the put fallback should invoke the set on the correct target

4a312bf

spotless

972780a

rbri requested a review from Copilot May 30, 2026 10:21

Copilot AI reviewed May 30, 2026

View reviewed changes

rbri added 2 commits May 30, 2026 13:44

fix some border cases

d70fdd0

Merge remote-tracking branch 'origin/set_trap_receiver' into set_trap…

8f7b42e

…_receiver

rbri marked this pull request as draft May 30, 2026 13:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

NativeProxy: fix set trap not passing receiver object#2347

NativeProxy: fix set trap not passing receiver object#2347
rbri wants to merge 10 commits into
mozilla:masterfrom
HtmlUnit:set_trap_receiver

rbri commented Mar 22, 2026 •

edited

Loading

Uh oh!

rbri commented Mar 22, 2026

Uh oh!

rbri commented May 28, 2026

Uh oh!

rbri commented May 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

rbri commented May 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

rbri commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		receiverObj.defineOwnProperty(cx, key, valueDesc);
		return true;

		receiverObj.defineOwnProperty(cx, key, newDesc);
		return true;

Uh oh!

Conversation

rbri commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rbri commented Mar 22, 2026

Uh oh!

rbri commented May 28, 2026

Uh oh!

rbri commented May 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

rbri commented May 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

rbri commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rbri commented Mar 22, 2026 •

edited

Loading