cloudflare / cloudflare/lol-html

ContentType::Text entity-escapes content inserted into <script>/<style>, where entities are never decoded

Open
#338 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2.1k
Forks
111
PR merge metrics
No merged PRs in 30d

Description

`ContentType::Text` escapes `<`, `>` and `&` wherever the content lands. Inside a raw-text element that is the wrong thing to do: the tokenizer never decodes character references in ``, `<style>`, `xmp`, `iframe`, `noembed`, `noframes` or `plaintext`, so the inserted text keeps the entities and the program or stylesheet breaks.

```rust
use lol_html::{element, html_content::ContentType, rewrite_str, RewriteStrSettings};

let out = rewrite_str(
"<script>var a=1;b{}",
RewriteStrSettings::new()
.append_element_content_handler(element!("script", |el| {
el.append("if(a<2&&a>0)a&=1;", ContentType::Text);
Ok(())
}))
.append_element_content_handler(element!("style", |el| {
el.prepend("ul>li{color:red} ", ContentType::Text);
Ok(())
})),
)
.unwrap();
assert_eq!(
out,
"var a=1;if(a&lt;2&amp;&amp;a&gt;0)a&amp;=1;ul&gt;li{color:red} b{}"
); // the browser runs `a<2&&...` (SyntaxError) and drops the `ul>li` rule
```

The DOM equivalent (`script.textContent = …`, `style.append(text)`) gives working code, and the HTML fragment serialization algorithm writes text children of exactly these elements unescaped. Inside RCDATA (``, `<title>`) the current escaping is correct, since those do decode character references.

This came out of differential fuzzing of Bun's `HTMLRewriter` (lol-html underneath) against Chromium: 10 of 13 script/style text insertions containing `<`/`>`/`&` diverged from the DOM, 10/10 without them agreed, and 842/842 text insertions into every other element agreed. It was also reported against Bun as oven-sh/bun#8061. Today the only working route is `ContentType::Html`, which makes the `</script>` breakout the caller's problem, so for now Bun only documents that (oven-sh/bun#41971).

Would a change along these lines be welcome? I am happy to write it, but the shape needs a maintainer call first:

1. When `ContentType::Text` content is written *inside* a raw-text element (`Element::{prepend, append, set_inner_content}` on one, `EndTag::before` on its end tag, `TextChunk::{before, after, replace}` on a chunk whose `text_type()` is `ScriptData`/`RawText`), write it literally instead of entity-escaping, as the spec serializer does. `before`/`after`/`replace` on the element itself land in the parent and keep today's escaping, as does everything in RCDATA and `Data`.
2. Keep the "text never injects markup" property by refusing content that would end the raw text early: an ASCII-case-insensitive `</script` (resp. `</style`, ...) followed by whitespace, `/` or `>`, and for script also `<!--` and `<script` (the [restrictions for contents of script elements](https://html.spec.whatwg.org/multipage/scripting.html#restrictions-for-contents-of-script-elements)). There is no lossless escape for these in arbitrary JS/CSS, so this would be an error, like `CommentTextError` for `Comment::set_text`. The content methods return `()` today, so that part is an API question: fallible `try_*` variants, or a rewriting error surfaced from `write()`/`end()`.

One wrinkle: the parser treats `<noscript>` as raw text (scripting enabled), while a scripting-disabled consumer parses its content as markup, so `noscript` should probably keep escaping.

If the answer is "`ContentType::Text` is escaping by definition, use `Html`", a sentence in the `ContentType::Text` docs about raw-text elements would still help.

Contributor guide

Open the contributing guide

Research direction

Start with ContentType::Text handling and the listed element and text mutation entry points: Element::{prepend, append, set_inner_content}, EndTag::before, and TextChunk::{before, after, replace}. Compare raw-text serialization with RCDATA and Data behavior, then resolve the fallible-API and noscript decisions with a maintainer. Done means the agreed API preserves working script/style text, rejects raw-text termination sequences, and has coverage for the listed contexts.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
tooling, web-dev
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.