Skip to content

fix: keep attributes whose name starts with an underscore - #314

Merged
taoqf merged 1 commit into
taoqf:mainfrom
spokodev:fix/leading-underscore-attributes
Sep 3, 2026
Merged

fix: keep attributes whose name starts with an underscore#314
taoqf merged 1 commit into
taoqf:mainfrom
spokodev:fix/leading-underscore-attributes

Conversation

@spokodev

@spokodev spokodev commented Jul 8, 2026

Copy link
Copy Markdown

Attribute names beginning with an underscore are dropped, because the attribute regex's first-character class omits _:

parse('<div _foo="bar">').firstChild.getAttribute('_foo') // undefined (expected 'bar')

_ngcontent-… becomes ngcontent-… and Hyperscript's _ attribute is lost. _ was excluded to avoid __proto__ prototype pollution (#129); allow _ and reject only __proto__ explicitly, as suggested on #206. Attribute values are always strings, so obj['__proto__'] = <string> is a no-op and the pollution guarantees are kept (the #129 test is updated to assert them directly). Closes #206.

@taoqf taoqf left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

  1. attributes starting with an underscore
    keeps the leading underscore in the attribute name:
    TypeError: Cannot read properties of undefined (reading 'should')
    at Context. (test/tests/issues/206.js:8:27)
    at process.processImmediate (node:internal/timers:504:21)

  2. attributes starting with an underscore
    supports the hyperscript "_" attribute:
    TypeError: Cannot read properties of undefined (reading 'should')
    at Context. (test/tests/issues/206.js:15:27)
    at process.processImmediate (node:internal/timers:504:21)

@spokodev

spokodev commented Sep 1, 2026

Copy link
Copy Markdown
Author

Same here — rebuilding dist/ from the branch makes it green. test:dist runs against the compiled dist/ and the test/pretest scripts don't recompile, so a stale build parses the attribute the old way and the test's parsed node comes back undefined. On current main with this change, bun run build && bun run test:dist → 267 passing, including attributes starting with an underscore (both cases). Patch unchanged.

The attribute-name regex disallowed a leading underscore, so names such
as Angular's _ngcontent-* / _nghost-* and hyperscript's _ were parsed
with the underscore stripped (getAttribute('_foo') returned undefined).
This was originally excluded to avoid a __proto__ prototype-pollution
vector (taoqf#129). Allow the leading underscore and instead reject the
literal __proto__ key explicitly, as suggested in taoqf#129, which keeps the
pollution guard while fixing taoqf#206. Values are strings, so no other key
can reach Object.prototype.
@spokodev
spokodev force-pushed the fix/leading-underscore-attributes branch from d472809 to 3aff8be Compare September 2, 2026 08:50
@spokodev

spokodev commented Sep 2, 2026

Copy link
Copy Markdown
Author

Rebased onto main — CI is green now (build 22/24/26.x). The earlier red was a stale base (the old yarn lockfile), not the change itself. Ready for another look.

@taoqf
taoqf merged commit 79008d4 into taoqf:main Sep 3, 2026
3 checks passed
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.

Omits _ attribute used by Hyperscript

2 participants