Skip to content

Fix null deref in forEachProperty when Proxy prototype getter throws - #30541

Closed
robobun wants to merge 1 commit into
mainfrom
farm/a6d9a57a/fix-foreach-property-proxy-exception
Closed

Fix null deref in forEachProperty when Proxy prototype getter throws#30541
robobun wants to merge 1 commit into
mainfrom
farm/a6d9a57a/fix-foreach-property-proxy-exception

Fix null deref in forEachProperty when Proxy prototype getter throws

089403a
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 12, 2026 in 30m 46s

Code review found 1 potential issue

Found 3 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/bun/util/inspect-proxy-prototype.test.ts:44-46 Test 'Proxy get trap throws' never actually triggers the throw

Annotations

Check warning on line 46 in test/js/bun/util/inspect-proxy-prototype.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Test 'Proxy get trap throws' never actually triggers the throw

This test never actually triggers the throw: the `get` trap is invoked exactly 3 times during `Bun.inspect(obj)` (once for `Symbol(nodejs.util.inspect.custom)`, then once each for `bar` and `baz`), so `++count > 3` is never true and the test passes even without the bindings.cpp fix. Lower the threshold to e.g. `> 1` so the trap actually throws inside `getPropertySlot` and exercises the `CLEAR_IF_EXCEPTION`-before-`continue` path. (Nit — tests 1 and 2 already cover the actual fixes, this one is j