Skip to content

DOM: scope getElementById LLVM cache by document (#20830) - #35578

Merged
PurHur merged 1 commit into
masterfrom
fix/20830-dom-importnode-setid-gebi-doc-scope
Aug 28, 2026
Merged

DOM: scope getElementById LLVM cache by document (#20830)#35578
PurHur merged 1 commit into
masterfrom
fix/20830-dom-importnode-setid-gebi-doc-scope

Conversation

@PurHur

@PurHur PurHur commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • AOT getElementById() consulted DomUserScriptElementCacheLlvm by id string only; a prior setIdAttribute() on document A made getElementById('y') on document B return the wrong element (setid_leak) after importNode().
  • lookupObject() now requires GLOBAL_DOC to match the requesting document when a receiver is provided.
  • Adds DomImportNodeGetElementByIdReindex20830AotTest covering test/repro/maintainer_gap_dom_importnode_getelementbyid_reindex.php.

Root cause

php-src ext/dom/node.csetIdAttribute does not survive importNode (only copyable libxml ID atypes do). VM path was correct; thin-AOT user-script cache ignored document scope.

Test plan

./script/aot-smoke.sh
# aot-smoke: 8 passed, 0 failed

./script/docker-exec.sh -- bash -lc 'f=test/repro/maintainer_gap_dom_importnode_getelementbyid_reindex.php; php bin/vm.php "$f"; tmp=/tmp/dom.bin; php bin/compile.php -o "$tmp" "$f" && "$tmp"'
# ok (VM + AOT)

./script/phpunit.sh --filter DomImportNodeGetElementByIdReindex20830AotTest
# OK (1 test, 5 assertions)

Closes #20830

Made with Cursor

setIdAttribute on document A was leaking into getElementById on document B
via the single-slot DomUserScriptElementCacheLlvm — importNode setIdAttribute
survivor check failed AOT with setid_leak while VM/Zend returned ok.

Co-authored-by: Cursor <cursoragent@cursor.com>
@PurHur
PurHur merged commit aa49a03 into master Aug 28, 2026
1 check failed
@PurHur
PurHur deleted the fix/20830-dom-importnode-setid-gebi-doc-scope branch August 28, 2026 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: DOMDocument::importNode() + appendChild — target getElementById() null (re-#19212, ext/dom/node.c)

1 participant