Skip to content

fix: encode special characters in HTMLElement textContent setter - #312

Open
spokodev wants to merge 1 commit into
taoqf:mainfrom
spokodev:fix/textcontent-encode-special-chars
Open

fix: encode special characters in HTMLElement textContent setter#312
spokodev wants to merge 1 commit into
taoqf:mainfrom
spokodev:fix/textcontent-encode-special-chars

Conversation

@spokodev

@spokodev spokodev commented Jul 8, 2026

Copy link
Copy Markdown

The HTMLElement.textContent setter stored the raw string in a text node without escaping, so special characters were re-interpreted as markup:

el.textContent = '<b>Tom & Jerry</b>';
el.innerHTML // '<b>Tom & Jerry</b>'  (expected the escaped text)

el.textContent = '<script>alert(1)</script>'; // re-parses into a real <script> element

The base Node.textContent setter already encodes the value (cb11eab); the HTMLElement override was missed. Encode the value before storing it.

@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) HTMLElement#textContent setter encodes special characters
       escapes HTML metacharacters when serialized:

      AssertionError: expected '<b>Tom & Jerry</b>' to equal '&lt;b&gt;Tom &amp; Jerry&lt;/b&gt;'
      + expected - actual

      -<b>Tom & Jerry</b>
      +&lt;b&gt;Tom &amp; Jerry&lt;/b&gt;
      
      at Assertion.fail (test/node_modules/should/cjs/should.js:275:17)
      at Assertion.value (test/node_modules/should/cjs/should.js:356:19)
      at Context.<anonymous> (test/tests/textcontent-encode.js:8:24)
      at process.processImmediate (node:internal/timers:504:21)

  2) HTMLElement#textContent setter encodes special characters
       round-trips a text value that looks like markup:

      AssertionError: expected 'alert(1)' to equal '<script>alert(1)</script>'
      + expected - actual

      -alert(1)
      +<script>alert(1)</script>
      
      at Assertion.fail (test/node_modules/should/cjs/should.js:275:17)
      at Assertion.value (test/node_modules/should/cjs/should.js:356:19)
      at Context.<anonymous> (test/tests/textcontent-encode.js:17:31)
      at process.processImmediate (node:internal/timers:504:21)

Seems unit test went wrong.

@spokodev

spokodev commented Sep 1, 2026

Copy link
Copy Markdown
Author

Both assertions pass once dist/ is rebuilt from this branch. test:dist runs against the compiled dist/, and neither test nor pretest recompiles, so a stale dist/ shows the old (unencoded) behavior. On current main with this change:

bun install && (cd test && bun install)
bun run build
bun run test:dist

→ 267 passing, including both textContent setter encodes special characters cases. The patch is unchanged; I can rebase onto main if that's easier to verify.

The HTMLElement textContent setter stored the raw string in a TextNode
without escaping, so setting textContent to a value containing '<', '>'
or '&' produced markup that was re-interpreted as HTML on serialization
and re-parsing (e.g. textContent = '<script>...' yielded a real <script>
child). The base Node textContent setter already encodes via entities;
this aligns HTMLElement with that behavior.
@spokodev
spokodev force-pushed the fix/textcontent-encode-special-chars branch from 7c1b393 to 48eb649 Compare September 2, 2026 08:49
@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.

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.

2 participants