Skip to content

Commit 31ed2fa

Browse files
committed
Allow rewriter to gracefully bail out on content handler errors.
This is the symmetric companion to the previous `MemoryLimitExceeded` bail-out. Until now, a content handler returning `Err` would abort the rewriter and leave the output sink in a potentially inconsistent state (the sink has whatever transformed bytes the rewriter had already produced, but the remaining input bytes are lost). Downstream this typically manifests as a truncated, broken HTTP response. This change adds an opt-in `Settings::graceful_bail_out_on_content_handler_error` flag. When set, before propagating `RewritingError::ContentHandlerError` the rewriter flushes every input byte it has received but not yet emitted to the sink, as-is. The caller can then continue the response by writing subsequent bytes directly to its own downstream sink, bypassing the (now poisoned) rewriter. The flag lives on `Settings` rather than `MemorySettings` because the underlying error has nothing to do with memory: a handler returning `Err` is an explicit signal from the application, not an environmental constraint. The two flags are intentionally independent so callers can pick the recovery posture per error class. The implementation requires a small but important refactor of the dispatcher. Previously, `lexeme_consumed()` did two things: emit the chunk of input before the lexeme, and advance `remaining_content_start` past the lexeme. The second step happened before the lexeme's token was actually serialized to the sink, so a handler error from `token_produced()` (or text emission) left `remaining_content_start` past a lexeme whose bytes had never been emitted. Flushing from there as-is would lose the lexeme. We split that into: 1. `emit_chunk_before_lexeme()`: emit the raw input chunk between the previously emitted byte and the start of the lexeme, advance `remaining_content_start` to the lexeme's start. 2. `consume_lexeme()`: advance `remaining_content_start` past the lexeme's end. Called only after the lexeme's token has been successfully emitted. With this split, on a `ContentHandlerError` mid-lexeme `remaining_content_start` still points at the failing lexeme's start, and a graceful bail-out can flush the lexeme + the rest of the chunk raw. The bail-out decision in `TransformStream::write()` and `end()` is centralized in `should_bail_out_for()`, which considers each `RewritingError` variant against its own flag. `ParsingAmbiguity` is never recovered from (the whole point of strict mode is to refuse uncertain markup). The C API is left on the original behavior for now. The previous memory flag fit cleanly into `MemorySettings` (a `#[repr(C)]` struct), so it was an additive change to the C ABI. The new flag lives on `Settings`, which is not `repr(C)`, so exposing it through the C API needs either a new function signature or a new settings struct — a maintainer call. A TODO captures this in `c-api/src/rewriter.rs`, and the CHANGELOG notes it. Tested with 6 new tests in `rewriter::tests::fatal_errors`: 1. `test_graceful_bail_out_on_element_handler_error`: element handler returns `Err`, sink ends up with the transformed prefix + the failing element and everything after it raw. 2. `test_no_graceful_bail_out_on_content_handler_error_by_default`: sanity check that the flag is opt-in. 3. `test_graceful_bail_out_on_comment_handler_error`: comment handler error path (exercises the same `lexeme_consumed` restructure). 4. `test_graceful_bail_out_on_end_handler_error`: `handle_end()` returns `Err` after `flush_remaining_input` has already emitted every input byte; sink already has the complete document. 5. `test_bail_out_reconstruct_handler_error_midstream`: end-to-end reconstruction (`sink_output == original_html`). 6. `test_bail_out_flags_independent`: enabling content-handler bail-out doesn't accidentally enable memory bail-out, and vice versa. Caveats documented on the new field's doc comment: 1. Active content removal at the time of the bail-out can produce mismatched tags. Same caveat as the memory flag; uncommon in practice and not byte-truncating. 2. Multi-chunk text emission where a later chunk's handler errors will re-emit the raw input bytes, duplicating the already-emitted chunks. Rare (only on encoding-converted text); the response is byte-bigger but not truncated.
1 parent 3674c38 commit 31ed2fa

10 files changed

Lines changed: 307 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
flushes every input byte it has received but not yet emitted to the sink (as-is) before
77
returning `MemoryLimitExceededError`, so callers can continue the response by writing
88
subsequent bytes directly to their downstream sink instead of breaking it.
9+
- Added `Settings::graceful_bail_out_on_content_handler_error`: symmetric to the memory flag
10+
above, but for `RewritingError::ContentHandlerError`. When set, the rewriter flushes
11+
remaining input bytes before propagating a handler error, preserving the response.
12+
Currently exposed via the Rust API only; the C API still uses the original behavior.
913

1014
## v2.9.0
1115

c-api/src/rewriter.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ fn lol_html_rewriter_build_inner(
6161
strict,
6262
enable_esi_tags,
6363
adjust_charset_on_meta_tag: false,
64+
// TODO: expose `graceful_bail_out_on_content_handler_error` through the C API. Adding
65+
// a new parameter to `lol_html_rewriter_build()` is a breaking ABI change, so it
66+
// belongs behind a new function variant or a settings struct.
67+
graceful_bail_out_on_content_handler_error: false,
6468
};
6569

6670
let output_sink = ExternOutputSink::new(output_sink, output_sink_user_data);

fuzz/test_case/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ fn run_rewriter_iter(data: &[u8], selector: &str, encoding: &'static Encoding) {
176176
memory_settings: MemorySettings::new(),
177177
strict: false,
178178
adjust_charset_on_meta_tag: false,
179+
graceful_bail_out_on_content_handler_error: false,
179180
},
180181
|_: &[u8]| {},
181182
);

src/rewriter/mod.rs

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ impl<'h, O: OutputSink, H: HandlerTypes> HtmlRewriter<'h, O, H> {
167167
let graceful_bail_out_on_memory_limit_exceeded = settings
168168
.memory_settings
169169
.graceful_bail_out_on_memory_limit_exceeded;
170+
let graceful_bail_out_on_content_handler_error =
171+
settings.graceful_bail_out_on_content_handler_error;
170172
let strict = settings.strict;
171173

172174
let encoding = settings.encoding;
@@ -188,6 +190,7 @@ impl<'h, O: OutputSink, H: HandlerTypes> HtmlRewriter<'h, O, H> {
188190
next_encoding,
189191
strict,
190192
graceful_bail_out_on_memory_limit_exceeded,
193+
graceful_bail_out_on_content_handler_error,
191194
});
192195

193196
HtmlRewriter {
@@ -1443,6 +1446,194 @@ mod tests {
14431446
);
14441447
}
14451448

1449+
// --- Content-handler-error bail-out tests ---
1450+
//
1451+
// These mirror the memory bail-out tests but with handler errors as the trigger. The
1452+
// contract is the same: when `graceful_bail_out_on_content_handler_error = true`, the
1453+
// sink receives every input byte the rewriter had been given before the error.
1454+
1455+
/// An element handler that returns `Err` aborts the rewriter. With graceful bail-out
1456+
/// enabled, the sink keeps every byte the caller had fed in, with the failing element
1457+
/// and everything after it flushed raw (no transformation), and earlier elements
1458+
/// transformed normally.
1459+
#[test]
1460+
fn test_graceful_bail_out_on_element_handler_error() {
1461+
let html = b"<a>first</a><stop>middle</stop><b>last</b>";
1462+
1463+
let mut output = Vec::<u8>::new();
1464+
let mut rewriter = HtmlRewriter::new(
1465+
Settings {
1466+
element_content_handlers: vec![
1467+
element!("a", |el| {
1468+
el.set_attribute("rewritten", "yes").unwrap();
1469+
Ok(())
1470+
}),
1471+
element!("stop", |_| Err("handler refused".into())),
1472+
],
1473+
graceful_bail_out_on_content_handler_error: true,
1474+
..Settings::new()
1475+
},
1476+
|c: &[u8]| output.extend_from_slice(c),
1477+
);
1478+
1479+
let err = rewriter.write(html).unwrap_err();
1480+
1481+
assert!(
1482+
matches!(err, RewritingError::ContentHandlerError(_)),
1483+
"expected ContentHandlerError, got {err}",
1484+
);
1485+
1486+
// The full original bytes from `<stop>` onwards are present raw, and the `<a>` tag
1487+
// before that has been transformed.
1488+
let output_str = std::str::from_utf8(&output).unwrap();
1489+
1490+
assert!(
1491+
output_str.starts_with("<a rewritten=\"yes\">first</a>"),
1492+
"expected transformed prefix, got {output_str:?}",
1493+
);
1494+
assert!(
1495+
output_str.ends_with("<stop>middle</stop><b>last</b>"),
1496+
"expected raw bytes from the failing tag onwards, got {output_str:?}",
1497+
);
1498+
}
1499+
1500+
/// Without the opt-in flag, an element-handler error still aborts processing without
1501+
/// flushing remaining bytes (existing behavior preserved).
1502+
#[test]
1503+
fn test_no_graceful_bail_out_on_content_handler_error_by_default() {
1504+
let html = b"<a>first</a><stop>middle</stop>";
1505+
1506+
let mut output = Vec::<u8>::new();
1507+
let mut rewriter = HtmlRewriter::new(
1508+
Settings {
1509+
element_content_handlers: vec![element!("stop", |_| Err(
1510+
"handler refused".into()
1511+
))],
1512+
..Settings::new()
1513+
},
1514+
|c: &[u8]| output.extend_from_slice(c),
1515+
);
1516+
1517+
let err = rewriter.write(html).unwrap_err();
1518+
1519+
assert!(matches!(err, RewritingError::ContentHandlerError(_)));
1520+
assert!(
1521+
!output.ends_with(b"<stop>middle</stop>"),
1522+
"without graceful bail-out the sink must NOT contain the failing tag, got {output:?}",
1523+
);
1524+
}
1525+
1526+
/// A comment handler that returns `Err` is recoverable too. Comments live on the same
1527+
/// `lexeme_consumed` path as elements, so this exercises the same restructured ordering.
1528+
#[test]
1529+
fn test_graceful_bail_out_on_comment_handler_error() {
1530+
let html = b"<div>Before<!--FAIL-->After</div>";
1531+
1532+
let mut output = Vec::<u8>::new();
1533+
let mut rewriter = HtmlRewriter::new(
1534+
Settings {
1535+
element_content_handlers: vec![comments!("div", |_| {
1536+
Err("comment refused".into())
1537+
})],
1538+
graceful_bail_out_on_content_handler_error: true,
1539+
..Settings::new()
1540+
},
1541+
|c: &[u8]| output.extend_from_slice(c),
1542+
);
1543+
1544+
let err = rewriter.write(html).unwrap_err();
1545+
assert!(matches!(err, RewritingError::ContentHandlerError(_)));
1546+
1547+
// The whole document is in the sink (handler error doesn't lose bytes).
1548+
assert_eq!(output, html);
1549+
}
1550+
1551+
/// A handler error from `handle_end` arrives after `flush_remaining_input` has already
1552+
/// emitted every input byte, so the sink already has the complete document. Bail-out
1553+
/// just propagates the error without losing anything.
1554+
#[test]
1555+
fn test_graceful_bail_out_on_end_handler_error() {
1556+
let html = b"<div>content</div>";
1557+
1558+
let mut output = Vec::<u8>::new();
1559+
let mut rewriter = HtmlRewriter::new(
1560+
Settings {
1561+
document_content_handlers: vec![end!(|_| Err("end refused".into()))],
1562+
graceful_bail_out_on_content_handler_error: true,
1563+
..Settings::new()
1564+
},
1565+
|c: &[u8]| output.extend_from_slice(c),
1566+
);
1567+
1568+
rewriter.write(html).unwrap();
1569+
1570+
let err = rewriter.end().unwrap_err();
1571+
1572+
assert!(matches!(err, RewritingError::ContentHandlerError(_)));
1573+
// All input bytes already in sink before `handle_end()` runs.
1574+
assert_eq!(output, html);
1575+
}
1576+
1577+
/// Reconstruction test: when a handler in the middle of a document errors, the sink
1578+
/// output plus any unfed bytes must equal the original document.
1579+
#[test]
1580+
fn test_bail_out_reconstruct_handler_error_midstream() {
1581+
let html = b"<p>before</p><div>middle</div><span>after</span>";
1582+
1583+
let mut output = Vec::<u8>::new();
1584+
let mut rewriter = HtmlRewriter::new(
1585+
Settings {
1586+
element_content_handlers: vec![element!("div", |_| {
1587+
Err("div refused".into())
1588+
})],
1589+
graceful_bail_out_on_content_handler_error: true,
1590+
..Settings::new()
1591+
},
1592+
|c: &[u8]| output.extend_from_slice(c),
1593+
);
1594+
1595+
let err = rewriter.write(html).unwrap_err();
1596+
assert!(matches!(err, RewritingError::ContentHandlerError(_)));
1597+
1598+
assert_eq!(
1599+
output, html,
1600+
"response must be reconstructable byte-for-byte when handler errors midstream",
1601+
);
1602+
}
1603+
1604+
/// The two bail-out flags are independent: enabling content-handler bail-out does not
1605+
/// affect memory-limit behavior, and vice versa.
1606+
#[test]
1607+
fn test_bail_out_flags_independent() {
1608+
// Memory limit error with content-handler bail-out only: should NOT bail out.
1609+
const MAX: usize = 100;
1610+
let mut output = Vec::<u8>::new();
1611+
let mut rewriter = HtmlRewriter::new(
1612+
Settings {
1613+
element_content_handlers: vec![element!("*", |_| Ok(()))],
1614+
memory_settings: MemorySettings {
1615+
max_allowed_memory_usage: MAX,
1616+
preallocated_parsing_buffer_size: 0,
1617+
graceful_bail_out_on_memory_limit_exceeded: false,
1618+
},
1619+
graceful_bail_out_on_content_handler_error: true,
1620+
..Settings::new()
1621+
},
1622+
|c: &[u8]| output.extend_from_slice(c),
1623+
);
1624+
1625+
let chunk_1 = format!("<img alt=\"{}", "l".repeat(MAX / 2));
1626+
let chunk_2 = format!("{}\" />", "r".repeat(MAX / 2));
1627+
rewriter.write(chunk_1.as_bytes()).unwrap();
1628+
let err = rewriter.write(chunk_2.as_bytes()).unwrap_err();
1629+
1630+
assert!(matches!(err, RewritingError::MemoryLimitExceeded(_)));
1631+
assert!(
1632+
output.is_empty(),
1633+
"content-handler flag must not enable memory bail-out, got {output:?}",
1634+
);
1635+
}
1636+
14461637
#[test]
14471638
fn content_handler_error_propagation() {
14481639
fn assert_err<'h>(

src/rewriter/settings.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,6 +1034,54 @@ pub struct Settings<'handlers, 'selectors, H: HandlerTypes = LocalHandlerTypes>
10341034
///
10351035
/// `false` when constructed with `Settings::new()`.
10361036
pub adjust_charset_on_meta_tag: bool,
1037+
1038+
/// Controls how the rewriter recovers when a content handler returns an `Err`.
1039+
///
1040+
/// When `false` (the default), the rewriter aborts processing the response, returns the
1041+
/// handler's [`RewritingError::ContentHandlerError`], and leaves the output sink in a
1042+
/// potentially inconsistent state. Downstream this typically manifests as a truncated,
1043+
/// broken response.
1044+
///
1045+
/// When `true`, before propagating [`RewritingError::ContentHandlerError`] the rewriter
1046+
/// flushes every input byte it has received but not yet emitted to the sink, *as-is*. The
1047+
/// caller can then continue the response by writing any subsequent input bytes directly
1048+
/// to its own downstream sink, bypassing the (now poisoned) rewriter. The resulting
1049+
/// response will have the rewriter's transformations applied up to some boundary,
1050+
/// followed by the original bytes after that boundary, but the response will not be
1051+
/// broken.
1052+
///
1053+
/// The rewriter is still poisoned after the error and must not be used again, regardless
1054+
/// of this setting.
1055+
///
1056+
/// This is symmetric with
1057+
/// [`MemorySettings::graceful_bail_out_on_memory_limit_exceeded`], but kept as a separate
1058+
/// flag because the underlying error has different semantics: a memory limit is an
1059+
/// environmental constraint, whereas a content handler returning `Err` is an explicit
1060+
/// signal from the application that something is wrong with the input. Some callers will
1061+
/// want graceful recovery for one but not the other.
1062+
///
1063+
/// ### Caveats
1064+
///
1065+
/// 1. If a handler was actively removing element content (e.g. via [`Element::remove()`])
1066+
/// at the time of the bail-out, the surrounding tags can end up mismatched in the
1067+
/// resulting response. In practice removing content is uncommon, and a
1068+
/// well-formed-but-imperfect response is still much better than a truncated one.
1069+
/// 2. If a text content handler returns an error after some chunks of the same text node
1070+
/// have already been emitted (rare; typically only happens with multi-chunk
1071+
/// encoding-converted text), the bail-out flush will re-emit the input bytes raw,
1072+
/// duplicating the already-emitted chunks. The response is byte-bigger but not
1073+
/// truncated.
1074+
///
1075+
/// ### Default
1076+
///
1077+
/// `false` when constructed with `Settings::new()`.
1078+
///
1079+
/// [`MemorySettings::graceful_bail_out_on_memory_limit_exceeded`]:
1080+
/// struct.MemorySettings.html#structfield.graceful_bail_out_on_memory_limit_exceeded
1081+
/// [`RewritingError::ContentHandlerError`]:
1082+
/// errors/enum.RewritingError.html#variant.ContentHandlerError
1083+
/// [`Element::remove()`]: html_content/struct.Element.html#method.remove
1084+
pub graceful_bail_out_on_content_handler_error: bool,
10371085
}
10381086

10391087
impl Default for Settings<'_, '_, LocalHandlerTypes> {
@@ -1074,6 +1122,7 @@ impl<H: HandlerTypes> Settings<'_, '_, H> {
10741122
strict: true,
10751123
enable_esi_tags: false,
10761124
adjust_charset_on_meta_tag: false,
1125+
graceful_bail_out_on_content_handler_error: false,
10771126
}
10781127
}
10791128
}

src/selectors_vm/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ mod vm_tests {
9090
memory_limiter: SharedMemoryLimiter::new(2048),
9191
strict: true,
9292
graceful_bail_out_on_memory_limit_exceeded: false,
93+
graceful_bail_out_on_content_handler_error: false,
9394
});
9495

9596
transform_stream.write(&html).unwrap();

src/transform_stream/dispatcher.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,18 @@ where
124124
Ok(())
125125
}
126126

127-
fn lexeme_consumed<T>(&mut self, lexeme: &Lexeme<'_, T>) {
127+
/// Emit the raw chunk of input that lies between the previously emitted byte and the
128+
/// start of `lexeme`, and move `remaining_content_start` to the start of `lexeme`.
129+
///
130+
/// This is the first half of what used to be `lexeme_consumed()`. It is split out so that
131+
/// the second half ([`consume_lexeme()`]) can be deferred until after the lexeme's token
132+
/// has been successfully emitted; that way, if token emission fails (e.g. a content
133+
/// handler returns an error), `remaining_content_start` still points at the failing
134+
/// lexeme's start and a graceful bail-out can flush the lexeme bytes raw rather than
135+
/// losing them.
136+
///
137+
/// [`consume_lexeme()`]: Self::consume_lexeme
138+
fn emit_chunk_before_lexeme<T>(&mut self, lexeme: &Lexeme<'_, T>) {
128139
let lexeme_range = lexeme.raw_range();
129140

130141
let chunk_range = Range {
@@ -138,7 +149,16 @@ where
138149
self.output_sink.handle_chunk(&chunk);
139150
}
140151

141-
self.remaining_content_start = lexeme_range.end;
152+
self.remaining_content_start = lexeme_range.start;
153+
}
154+
155+
/// Advance `remaining_content_start` past the end of `lexeme`, marking it as committed.
156+
/// Must only be called after the lexeme's token has been successfully emitted; see
157+
/// [`emit_chunk_before_lexeme()`].
158+
///
159+
/// [`emit_chunk_before_lexeme()`]: Self::emit_chunk_before_lexeme
160+
fn consume_lexeme<T>(&mut self, lexeme: &Lexeme<'_, T>) {
161+
self.remaining_content_start = lexeme.raw_range().end;
142162
}
143163

144164
#[inline]
@@ -234,13 +254,14 @@ where
234254
{
235255
match lexeme.to_token(&mut self.delegate.capture_flags, self.encoding.get()) {
236256
ToTokenResult::Token(token) => {
237-
self.delegate.lexeme_consumed(lexeme);
257+
self.delegate.emit_chunk_before_lexeme(lexeme);
238258
self.delegate.token_produced(token)?;
259+
self.delegate.consume_lexeme(lexeme);
239260
// handler for <meta charset> may have changed encoding
240261
self.flush_encoding_change();
241262
}
242263
ToTokenResult::Text(text_type) => {
243-
self.delegate.lexeme_consumed(lexeme);
264+
self.delegate.emit_chunk_before_lexeme(lexeme);
244265
self.last_text_type = text_type;
245266
self.text_decoder.feed_text(
246267
lexeme.spanned(),
@@ -255,6 +276,7 @@ where
255276
)
256277
},
257278
)?;
279+
self.delegate.consume_lexeme(lexeme);
258280
}
259281
ToTokenResult::None => {}
260282
}

0 commit comments

Comments
 (0)