Skip to content

Write a lossy text chunk back from its source bytes - #12

Draft
robobun wants to merge 2 commits into
bunfrom
robobun/72e68030/text-source-writeback
Draft

robobun wants to merge 2 commits into
bunfrom
robobun/72e68030/text-source-writeback

Conversation

@robobun

@robobun robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Parked draft. It needs a maintainer's choice:

  • (a) Land it, on by default.
  • (b) Make it a Settings flag that is off by default.
  • (c) Close it and only give lol-html the Content-Type charset, as workerd does.

Revives oven-sh/bun#35933.

Problem

  • A text handler that changes nothing rewrites the document: <p>caf\xE9</p> leaves as <p>caf\xEF\xBF\xBD</p>. Without a text handler it passes unchanged.
  • TextChunk::serialize_self (tokens/text_chunk.rs) writes the decoded text, which has U+FFFD for malformed input. Tags and comments write their raw bytes.

Fix

  • TextDecoder keeps had_replacements from decode_to_str. A chunk that got a U+FFFD is written from its source bytes, unless a handler replaced, removed or edited it.
  • Every other chunk takes the old path: in UTF-8 its text is its source.
  • Verified: cargo test --features=_integration_test. 32 of 32 seeded faults fail a test.

Background

Downsides

  • Upstream lol-html and Workers write the U+FFFD. Of 150 rows, Bun 1.4.3 differs from workerd on 37, this change on 58, (c) alone on 0, both on 36.
  • Release Bun: +0.3% to +0.4% instructions for a well-formed page with a text handler, +2.2% with 43% malformed chunks.
  • On a multi-byte legacy page read as UTF-8, text before()/after() content can land inside a character.
Notes

Why it is parked. A review of this PR found that the change does what it says, and that it moves Bun away from the implementation that Bun's docs name as the compatibility target. On the repro above, workerd 1.20261015.1 and stock lol-html 3.0.1 write the same EF BF BD that Bun 1.4.3 writes. No report from a user was found in the trackers of Bun, lol-html or workerd. The symptom comes from a fuzz rule ("a handler that only reads leaves the bytes as they were") that the reference does not keep. So the choice between (a), (b) and (c) belongs to a maintainer. The charset change of (c) is separate work and does not build on this PR.

The 150 rows. 5 inputs (windows-1252 text, Shift_JIS text, ill-formed UTF-8, a sequence cut short by the end tag, well-formed UTF-8), 5 Content-Type labels (none, text/html, charset=utf-8, charset=windows-1252, charset=shift_jis), 6 handlers (none, element, text read-only, document text read-only, text before/after, text replace). Each row is the output bytes of transform(new Response(bytes, { headers })).

Rows that differ from workerd 1.20261015.1
Bun 1.4.3 37
This change 58 (15 now equal, 36 newly different, 22 still different)
Charset of the Content-Type label given to lol-html, no other change 0
Both 36

The first two lines are real binaries. The last two are a model: a Rust program that builds the same Settings as Bun's binding and takes the encoding from the label. With UTF-8 for every document the model gives the output of the real binaries on 150 of 150 rows, for both pins. The 15 rows that this change makes equal are rows with a legacy label, and the charset change makes the same 15 equal. The 36 rows are the 4 inputs that are not valid UTF-8, under the 3 labels that mean UTF-8, with the 3 text handlers that keep the chunk: there workerd writes U+FFFD and this change writes the source bytes.

Branches. The Bun side is branch robobun/72e68030/htmlrewriter-text-source-bytes in oven-sh/bun, with no PR. This PR is based on bun (v3.0.1): e1a306d to 42ec40c, two commits. Bun pins the v2.7.2 lineage today, so the same two commits are on robobun/72e68030/text-source-writeback-v2.7.2 (725ce49 to cfc777a). The two differ only where v3 differs: the updated upstream test test_flush_before_charset_switch, and TextDecoder::new. The Bun branch pins cfc777a and adds 33 tests to test/js/workerd/html-rewriter.test.js (31 of them fail on Bun 1.4.3, the other 2 are controls).

Held bytes. The decoder holds up to 3 bytes of an incomplete sequence between write() calls. They belong to the chunk that shows their character, and they are written with it. The bookkeeping is settled before the handler runs, so a handler that suspends cannot lose them.

What a handler sees. Nothing changes for handlers. as_str() has the U+FFFD as before, and the chunks are the same chunks. before() and after() content goes around the source bytes. replace(), remove(), set_str() and as_mut_str() write what they wrote before. A document in another encoding is written as before.

What other handlers do. Text handlers that change nothing change no byte of the output, whatever other handlers do. Two tests state it (handlers_that_change_nothing_change_no_byte_whatever_other_handlers_do, bytes_that_a_removed_token_separated_are_written_side_by_side). One effect follows. When a handler removes a tag, a comment or a chunk that stood between two malformed fragments, the bytes on both sides become neighbours, and they can make a character that no text handler was given: b.remove_and_keep_content() on <p>\xE2<b>\x82</b>\xAC</p> writes <p>\xE2\x82\xAC</p>. That is also the output without a text handler, before and after this change.

Bodies that are not HTML. 2,048 random bytes with a PNG signature through a document text handler that changes nothing: Bun 1.4.3 writes 3,760 bytes, this change writes the 2,048 bytes. workerd 1.20261015.1 writes the same 3,760 bytes, and no charset change helps there.

Measurements in release builds of Bun. Bun at 924257d44, ThinLTO, three builds that differ in lol-html: 725ce49, the first commit, and the by-reference form of the second commit (its lol-html differs from cfc777a in a variable name and in where Ok(true) stands). User-mode instructions of the process for one transform() of a Response, as the slope between 200 and 1,200 rewrites, median of 7 runs. The last column is the spread of the 7 runs of the base.

Input Handler 725ce49 First commit Both commits Spread
cloudflare.com.html (119,237 B), one write none 1,190,374 -0.00% -0.01% 1.13%
element 1,710,974 +0.00% +0.02% 0.10%
text, 3,819 chunks 7,534,251 +1.50% +0.40% 0.23%
Non-ASCII UTF-8 page, 1,460-byte writes none 1,065,806 +0.04% +0.10% 0.21%
element 3,053,257 +0.14% +0.17% 0.46%
text, 4,838 chunks 9,387,097 +1.36% +0.30% 0.29%
Malformed page, 2,066 of 4,802 chunks lossy, one write none 963,066 -0.14% -0.06% 0.18%
element 2,967,284 -0.14% -0.03% 0.47%
text, 4,802 chunks 12,208,939 +3.05% +2.15% 0.32%

The release binary grows by 8,192 bytes (106,198,792 to 106,206,984).

Measurements in a harness. Bun's toolchain (nightly-2026-09-15) and Bun's release flags for linux-x64, -Zbuild-std, ThinLTO by lld, and two crates that both use the rewriter, as bun_bundler and bun_runtime do. User-mode instructions of one rewrite with a Rust handler that does nothing, minimum of 5.

v3.0.1: e1a306d 42ec40c v2.7.2: 725ce49 cfc777a
Valid page, text handler, one write (3,819 chunks) 4,959,693 4,975,015 (4.0 more per chunk) 5,035,747 5,054,990 (5.0 more per chunk)
Same, 4,096-byte writes (3,825 chunks) 4,991,719 5,007,255 (4.1) 5,071,600 5,091,065 (5.1)
Same, 15-byte writes (5,910 chunks) 11,160,793 11,249,928 (15.1) 11,275,572 11,374,909 (16.8)
Same page, no handler 1,167,779 1,167,773 1,166,781 1,166,805
Same page, element handler only 2,298,743 2,298,743 2,136,325 2,136,337
1,000 characters split by a write 4,628,182 4,889,186 (261 more per split) 4,686,030 4,952,033 (266)
1,000 chunks with malformed input 2,883,166 3,084,169 (201 more per chunk) 2,927,005 3,133,007 (206)

The sign of the first line depends on what the link inlines. With one crate that uses the rewriter, the same flags give 13.0 fewer instructions per chunk on v3.0.1 and 12.0 fewer on v2.7.2. The table is the worse shape. An earlier version of this body gave "13.8 fewer per chunk" from a build without LTO. That number does not describe a build that Bun ships.

Allocations, from a counting allocator: valid page 990 before and after, 1,000 split characters 2,008 and 2,008, 1,000 malformed chunks 9 and 9. With every chunk suspended, 8,009 and 9,009: one copy per parked chunk with malformed input.

Sizes: TextChunk 80 B and Token 136 B before and after. TextDecoder 128 B to 160 B, so a rewriter grows by 32 B.

Why write_from_source borrows and is #[inline]. The first commit had into_bytes_from_source(mut self, ..), compiled into this crate. That cost about 20 instructions for every chunk of every document. The function brought <Vec<StringChunk> as Drop>::drop, a part of the drop of the mutations, into this crate. Bun builds with -Zshare-generics, so the crates that use the rewriter linked to that copy. The drop of a Mutations then looked cheap to the inliner and went into the drop of a TextChunk, which became too large to inline into Token::into_bytes. The LLVM inline remarks show it: cost 190 before, 420 after, threshold 250. The second commit borrows the chunk and compiles the function with its callers.

Fuzz. 200,000 random documents with damaged UTF-8, cut into random writes, through handlers that change nothing. 0 outputs differ from the input, with sync handlers and with handlers that suspend at random. On the base 167,895 differ. The result is the same with the 1,024-byte decode buffer and with a 13-byte one. The chunks that handlers get are the same as on the base in all 14 handler configurations (200,000 documents each). With handlers that insert, replace or edit, the output decodes to the same text as the output of the base. When a handler removes a chunk, the bytes on both sides of it become neighbours, and the decoded text can differ there. A shorter form of this fuzz runs in the test suite. LOL_HTML_FUZZ_DOCUMENTS=200000 runs the long form.

Seeded faults. 32 single faults, each in one place of the new code. Every one makes at least one test fail on both branches. They cover the tail scan, may_hold, the held bytes, the parked source, text_edited, the encoding gate, the fast check, the order in resume_dispatch, the content around a chunk (order, replacement, errors), and what write_from_source returns and what its callers do with it.

Overlap with #3. Commit 6e9ace3 of #3 keeps a text node on the UTF-8 fast path. It merges with this change without a conflict, and then chunks_account_for_every_source_byte fails: its reference model describes the chunks from before 6e9ace3. The PR that lands second has to update the model. No production path conflicts.

Found on the way, not changed here. When a handler suspends on each chunk of a long text lexeme, TextFeedSuspension copies the undecoded rest of the lexeme each time. One 16 MB text node with a byte that is not UTF-8 near its start takes 10.2 s in Bun 1.4.3 with a handler that awaits setImmediate, and 19 ms with a sync handler. This PR adds one more copy of at most 1,024 bytes per parked chunk with malformed input, next to it.

If the ruling is (a). Rebase on the bun tip after #9, #11 and 6e9ace3. Pin the merged commit after oven-sh/bun#43266 and drop the v2.7.2 port. Name this second difference from upstream in the fork README and in the header of scripts/build/deps/lolhtml.ts. Add text-handler cases to bench/snippets/rewriter.mjs. Once Bun gives the charset to lol-html, a page with a legacy label is decoded in that encoding and this change no longer applies to it: the docs sentence on the Bun branch has to say so.

Draft of an issue for cloudflare/lol-html (not filed)

Title: A text handler that changes nothing rewrites malformed bytes to U+FFFD

With a text handler registered, the rewriter writes every text chunk back from its decoded text, whether or not a handler changed the chunk. In a UTF-8 document the decoder puts U+FFFD in place of malformed input, so those bytes change although no handler touched them. Tags, comments and doctypes write their raw bytes when nothing modified them, so an element or comment handler that changes nothing leaves the document as it was, byte for byte.

use lol_html::{HtmlRewriter, Settings, doc_text, element};

fn rewrite(html: &[u8], settings: Settings<'_, '_>) -> Vec<u8> {
    let mut out = Vec::new();
    let mut rewriter = HtmlRewriter::new(settings, |c: &[u8]| out.extend_from_slice(c));
    rewriter.write(html).unwrap();
    rewriter.end().unwrap();
    out
}

fn main() {
    // windows-1252, or any page whose encoding did not reach `Settings::encoding`
    let html = b"<p>caf\xE9</p>";

    let element_only = rewrite(
        html,
        Settings::new().append_element_content_handler(element!("*", |_| Ok(()))),
    );
    assert_eq!(element_only, html);

    let text = rewrite(
        html,
        Settings::new().append_document_content_handler(doc_text!(|_| Ok(()))),
    );
    assert_eq!(text, b"<p>caf\xEF\xBF\xBD</p>"); // expected: equal to `html`
}

This is v3.0.1 (608cc4a). TextChunk::serialize_self writes the decoded text (src/rewritable_units/tokens/text_chunk.rs:332), and TextDecoder::feed_text drops the had_replacements result of decode_to_str (src/rewritable_units/text_decoder.rs:91).

Where it shows:

  1. A page that is not UTF-8 and does not say so, or a UTF-8 page with a stray byte. A handler that only reads text, for example to collect a title, damages the page it forwards.
  2. The graceful bail-out. The reconstruction tests in src/rewriter/mod.rs state the contract as sink_output + unfed_remaining_bytes == original_html, and say "The handlers used are no-ops, so serialized tokens are byte-for-byte identical to the original input". That holds for the element! and comments! handlers those tests use. With doc_text!(|_| Ok(())) added and <p>caf\xE9</p> in front of the huge comment of test_bail_out_reconstruct_huge_comment, the reconstructed response starts with <p>caf\xEF\xBF\xBD</p>.
  3. A body that is not HTML. The 256 bytes 0..=255 come out as 512 bytes through doc_text!(|_| Ok(())), and unchanged through an element handler.

Is the write-back of the decoded text intended for a chunk that no handler changed? Cloudflare Workers has this behaviour today (workerd 1.20261015.1), and so does Bun, so a change would be visible there. A patch exists (this PR). It can be the default, or a Settings flag that defaults to today's output.

A text handler that changes nothing rewrote the document. With a text
handler active, each text chunk is decoded for the handler, and the
decoded text is what gets written back. In a UTF-8 document the decode
puts U+FFFD in place of malformed input, so `<p>caf\xE9</p>` left as
`<p>caf\xEF\xBF\xBD</p>` although no handler touched the chunk. Comments,
tags and doctypes keep their raw bytes and write those back when they
are not modified.

The decoder now says which chunks are lossy (`decode_to_str` already
reports whether it replaced anything) and hands their source bytes to
the dispatcher. A lossy chunk that no handler edited, replaced or
removed is written from those bytes. Every other chunk is written from
its text as before. In a UTF-8 document that text is the source, so
well-formed input takes the path it took before and `TextChunk` stays
80 bytes.

The decoder holds up to 3 bytes of an incomplete sequence across
`write()` calls. Those bytes belong to the chunk that shows their
character. The bookkeeping is settled before the handler runs, so a
handler that suspends cannot lose them. A suspended lossy chunk gets a
copy of its source bytes, because the input buffer is gone by the time
it resumes.

Handlers get the same chunks with the same text as before. Documents in
other encodings are written as before.

This differs from upstream lol-html and from Cloudflare Workers, which
write the U+FFFD.
`TextChunk::into_bytes_from_source` took the chunk by value and was
compiled into this crate. In a ThinLTO build shaped like Bun's, that made
every text chunk about 20 instructions slower, in well-formed documents
too. The function brought the drop glue of the mutations into this crate.
A crate built with `-Zshare-generics` then linked to that copy, and the
drop of a chunk was no longer inlined into `Token::into_bytes`.

`TextChunk::write_from_source` borrows the chunk and is `#[inline]`, so
it is compiled with its callers. It returns `false` for a chunk that a
handler edited, and the caller serializes that chunk as before.

New tests tie the function to `impl_serialize!`. A lossy chunk and a
well-formed one get the same content around them for every kind of
mutation, and an error from that content fails the rewrite for both.
Two more tests state what holds when other handlers change the document:
text handlers that change nothing change no byte of the output.

The comment on `account_source_bytes` now says that its rule about what
the decoder holds is an observed behaviour of encoding_rs, and names the
test that checks it.
@robobun

robobun commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Status: parked draft. It waits for a maintainer's choice between (a), (b) and (c) in the description.

Reproduced on Bun 1.4.3 and on main:

const page = Buffer.from("<p>caf\xE9</p>", "latin1");
const out = await new HTMLRewriter().on("p", { text() {} }).transform(new Response(page)).arrayBuffer();
console.log(Buffer.from(out).toString("hex"));
// 3c703e636166efbfbd3c2f703e, and 3c703e636166e93c2f703e with this change

workerd 1.20261015.1 prints the first line too.

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