-
Notifications
You must be signed in to change notification settings - Fork 5k
HTMLRewriter: pass text bytes through unchanged when a text handler only observes #35933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
robobun
wants to merge
5
commits into
main
Choose a base branch
from
farm/72e68030/htmlrewriter-text-passthrough
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
a604af6
HTMLRewriter: pass text bytes through unchanged when a text handler o…
robobun f71ac42
carry decoder-buffered tail bytes across chunks so raw stays aligned …
robobun fe41a41
[autofix.ci] apply automated fixes
autofix-ci[bot] 23e2cc6
add lol-html unit tests for raw/text alignment across write boundaries
robobun 9c5e0c8
scope raw-byte passthrough to UTF-8; drop redundant test block
robobun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,282 @@ | ||
| --- a/src/rewritable_units/text_decoder.rs | ||
| +++ b/src/rewritable_units/text_decoder.rs | ||
| @@ -1,6 +1,7 @@ | ||
| use crate::base::{Bytes, SharedEncoding, SourceLocation, Spanned}; | ||
| use crate::rewriter::RewritingError; | ||
| use encoding_rs::{CoderResult, Decoder, Encoding, UTF_8}; | ||
| +use std::borrow::Cow; | ||
|
|
||
| const DEFAULT_BUFFER_LEN: usize = if cfg!(test) { 13 } else { 1024 }; | ||
|
|
||
| @@ -8,11 +9,23 @@ | ||
| encoding: SharedEncoding, | ||
| pending_source_location_bytes_start: usize, | ||
| pending_text_streaming_decoder: Option<Decoder>, | ||
| + // Raw input bytes the decoder has consumed whose decoded characters have | ||
| + // NOT yet been delivered to `output_handler`. For UTF-8 this is exactly | ||
| + // the incomplete-sequence tail encoding_rs keeps in its own state (<= 3 | ||
| + // bytes); it is owned here so the NEXT emitted chunk can expose those | ||
| + // bytes as its `raw` prefix and preserve byte-identical passthrough. | ||
| + pending_raw: Vec<u8>, | ||
| text_buffer: String, | ||
| } | ||
|
|
||
| -pub(crate) type OutputHandlerCallback<'tmp> = | ||
| - dyn FnMut(&str, bool, &'static Encoding, SourceLocation) -> Result<(), RewritingError> + 'tmp; | ||
| +pub(crate) type OutputHandlerCallback<'tmp, 'i> = dyn FnMut( | ||
| + &str, | ||
| + Option<Cow<'i, [u8]>>, | ||
| + bool, | ||
| + &'static Encoding, | ||
| + SourceLocation, | ||
| + ) -> Result<(), RewritingError> | ||
| + + 'tmp; | ||
|
|
||
| impl TextDecoder { | ||
| #[inline] | ||
| @@ -22,6 +35,7 @@ | ||
| pending_source_location_bytes_start: 0, | ||
| encoding, | ||
| pending_text_streaming_decoder: None, | ||
| + pending_raw: Vec::new(), | ||
| // this will be later initialized to DEFAULT_BUFFER_LEN, | ||
| // because encoding_rs wants a slice | ||
| text_buffer: String::new(), | ||
| @@ -31,7 +45,7 @@ | ||
| #[inline] | ||
| pub fn flush_pending( | ||
| &mut self, | ||
| - output_handler: &mut OutputHandlerCallback<'_>, | ||
| + output_handler: &mut OutputHandlerCallback<'_, '_>, | ||
| ) -> Result<(), RewritingError> { | ||
| if self.pending_text_streaming_decoder.is_some() { | ||
| self.feed_text( | ||
| @@ -44,11 +58,11 @@ | ||
| } | ||
|
|
||
| #[inline(never)] | ||
| - pub fn feed_text( | ||
| + pub fn feed_text<'i>( | ||
| &mut self, | ||
| - input_span: Spanned<Bytes<'_>>, | ||
| + input_span: Spanned<Bytes<'i>>, | ||
| last_in_text_node: bool, | ||
| - output_handler: &mut OutputHandlerCallback<'_>, | ||
| + output_handler: &mut OutputHandlerCallback<'_, 'i>, | ||
| ) -> Result<(), RewritingError> { | ||
| let mut raw_input = input_span.as_slice(); | ||
| let mut next_source_location_bytes_start = input_span.source_location().bytes().start; | ||
| @@ -56,6 +70,7 @@ | ||
| let encoding = self.encoding.get(); | ||
|
|
||
| if let Some((utf8_text, rest)) = self.split_utf8_start(raw_input, encoding) { | ||
| + debug_assert!(self.pending_raw.is_empty()); | ||
| raw_input = rest; | ||
| let really_last = last_in_text_node && rest.is_empty(); | ||
|
|
||
| @@ -63,7 +78,13 @@ | ||
| SourceLocation::from_start_len(next_source_location_bytes_start, utf8_text.len()); | ||
| next_source_location_bytes_start = source_location.bytes().end; | ||
|
|
||
| - (output_handler)(utf8_text, really_last, encoding, source_location)?; | ||
| + (output_handler)( | ||
| + utf8_text, | ||
| + Some(Cow::Borrowed(utf8_text.as_bytes())), | ||
| + really_last, | ||
| + encoding, | ||
| + source_location, | ||
| + )?; | ||
|
|
||
| if really_last { | ||
| debug_assert!(self.pending_text_streaming_decoder.is_none()); | ||
| @@ -89,6 +110,28 @@ | ||
| SourceLocation::from_start_len(next_source_location_bytes_start, read); | ||
| next_source_location_bytes_start = source_location.bytes().end; | ||
|
|
||
| + // Raw-byte accounting for this chunk: | ||
| + // raw(chunk) = pending_raw ++ raw_input[..read - tail] | ||
| + // pending_raw' = raw_input[read - tail .. read] | ||
| + // where `tail` is the number of bytes encoding_rs kept as internal | ||
| + // state (an incomplete sequence at end-of-input). OutputFull and | ||
| + // last=true never leave state, so `tail` is only nonzero on the | ||
| + // final InputEmpty of a non-last call. | ||
| + let consumed = raw_input.get(..read).unwrap_or_default(); | ||
| + let tail = if finished_decoding && !last_in_text_node && encoding == UTF_8 { | ||
| + utf8_incomplete_tail_len(consumed) | ||
| + } else { | ||
| + 0 | ||
| + }; | ||
| + let (emit, carry) = consumed.split_at(read - tail); | ||
| + let raw: Cow<'i, [u8]> = if self.pending_raw.is_empty() { | ||
| + Cow::Borrowed(emit) | ||
| + } else { | ||
| + let mut v = std::mem::take(&mut self.pending_raw); | ||
| + v.extend_from_slice(emit); | ||
| + Cow::Owned(v) | ||
| + }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| + | ||
| if written > 0 || last_in_text_node { | ||
| // the last call to feed_text() may make multiple calls to output_handler, | ||
| // but only one call to output_handler can be *the* last one. | ||
| @@ -97,15 +140,25 @@ | ||
| (output_handler)( | ||
| // this will always be in bounds, but unwrap_or_default optimizes better | ||
| buffer.get(..written).unwrap_or_default(), | ||
| + Some(raw), | ||
| really_last, | ||
| encoding, | ||
| source_location, | ||
| )?; | ||
| + self.pending_raw.clear(); | ||
| + self.pending_raw.extend_from_slice(carry); | ||
| + } else { | ||
| + // Handler call skipped: everything we would have emitted as raw | ||
| + // (including any carried prefix) defers to the next chunk. | ||
| + let mut deferred = raw.into_owned(); | ||
| + deferred.extend_from_slice(carry); | ||
| + self.pending_raw = deferred; | ||
| } | ||
|
|
||
| if finished_decoding { | ||
| if last_in_text_node { | ||
| self.pending_text_streaming_decoder = None; | ||
| + debug_assert!(self.pending_raw.is_empty()); | ||
| } else { | ||
| self.pending_source_location_bytes_start = next_source_location_bytes_start; | ||
| } | ||
| @@ -152,3 +205,20 @@ | ||
| } | ||
| } | ||
| } | ||
| + | ||
| +/// Length of the incomplete-but-still-valid UTF-8 sequence at the end of | ||
| +/// `input`: the bytes a WHATWG UTF-8 decoder holds as state when fed `input` | ||
| +/// with `last=false`. At most 3 bytes. | ||
| +#[inline] | ||
| +fn utf8_incomplete_tail_len(input: &[u8]) -> usize { | ||
| + let mut tail = &input[input.len().saturating_sub(3)..]; | ||
| + loop { | ||
| + match std::str::from_utf8(tail) { | ||
| + Ok(_) => return 0, | ||
| + Err(e) => match e.error_len() { | ||
| + None => return tail.len() - e.valid_up_to(), | ||
| + Some(n) => tail = &tail[e.valid_up_to() + n..], | ||
| + }, | ||
| + } | ||
| + } | ||
| +} | ||
| --- a/src/rewritable_units/tokens/text_chunk.rs | ||
| +++ b/src/rewritable_units/tokens/text_chunk.rs | ||
| @@ -74,6 +74,9 @@ | ||
| /// [`last_in_text_node`]: #method.last_in_text_node | ||
| pub struct TextChunk<'i> { | ||
| text: Cow<'i, str>, | ||
| + // Input bytes this chunk was decoded from. Cleared by any API that | ||
| + // mutates `text` so `serialize_self` only uses it for true passthrough. | ||
| + raw: Option<Cow<'i, [u8]>>, | ||
| text_type: TextType, | ||
| last_in_text_node: bool, | ||
| encoding: &'static Encoding, | ||
| @@ -87,6 +90,7 @@ | ||
| #[must_use] | ||
| pub(crate) fn new( | ||
| text: &'i str, | ||
| + raw: Option<Cow<'i, [u8]>>, | ||
| text_type: TextType, | ||
| last_in_text_node: bool, | ||
| encoding: &'static Encoding, | ||
| @@ -94,6 +98,7 @@ | ||
| ) -> Self { | ||
| TextChunk { | ||
| text: text.into(), | ||
| + raw, | ||
| text_type, | ||
| last_in_text_node, | ||
| encoding, | ||
| @@ -130,6 +135,7 @@ | ||
| /// It may be necessary to buffer the text. See [`TextChunk::last_in_text_node`]. | ||
| #[inline] | ||
| pub fn as_mut_str(&mut self) -> &mut String { | ||
| + self.raw = None; | ||
| self.text.to_mut() | ||
| } | ||
|
|
||
| @@ -139,6 +145,7 @@ | ||
| /// See [`TextChunk::text_type`]. | ||
| #[inline] | ||
| pub fn set_str(&mut self, text: String) { | ||
| + self.raw = None; | ||
| self.text = Cow::Owned(text); | ||
| } | ||
|
|
||
| @@ -354,7 +361,11 @@ | ||
|
|
||
| #[inline] | ||
| fn serialize_self(&self, sink: &mut StreamingHandlerSink<'_>) -> Result<(), RewritingError> { | ||
| - if !self.text.is_empty() { | ||
| + if let Some(raw) = &self.raw { | ||
| + if !raw.is_empty() { | ||
| + (sink.output_handler())(raw); | ||
| + } | ||
| + } else if !self.text.is_empty() { | ||
| // The "text" here is actually markup | ||
| sink.write_str(&self.text, ContentType::Html); | ||
| } | ||
| @@ -429,6 +440,7 @@ | ||
| let encoding = Encoding::for_label_no_replacement(b"utf-8").unwrap(); | ||
| let mut chunk = TextChunk::new( | ||
| "original text", | ||
| + None, | ||
| TextType::PlainText, | ||
| true, | ||
| encoding, | ||
| --- a/src/transform_stream/dispatcher.rs | ||
| +++ b/src/transform_stream/dispatcher.rs | ||
| @@ -11,6 +11,7 @@ | ||
| use crate::rewritable_units::{DocumentEnd, Serialize, ToToken, Token, TokenCaptureFlags}; | ||
| use crate::rewriter::RewritingError; | ||
| use encoding_rs::Encoding; | ||
| +use std::borrow::Cow; | ||
|
|
||
| pub(crate) struct AuxStartTagInfo<'i> { | ||
| pub input: &'i Bytes<'i>, | ||
| @@ -149,6 +150,7 @@ | ||
| fn text_token_produced( | ||
| &mut self, | ||
| text: &str, | ||
| + raw: Option<Cow<'_, [u8]>>, | ||
| encoding: &'static Encoding, | ||
| text_type: TextType, | ||
| is_last_in_node: bool, | ||
| @@ -156,6 +158,7 @@ | ||
| ) -> Result<(), RewritingError> { | ||
| let mut token = Token::TextChunk(TextChunk::new( | ||
| text, | ||
| + raw, | ||
| text_type, | ||
| is_last_in_node, | ||
| encoding, | ||
| @@ -219,9 +222,10 @@ | ||
| self.text_decoder.feed_text( | ||
| lexeme.spanned(), | ||
| false, | ||
| - &mut |text, is_last, encoding, source_location| { | ||
| + &mut |text, raw, is_last, encoding, source_location| { | ||
| self.delegate.text_token_produced( | ||
| text, | ||
| + raw, | ||
| encoding, | ||
| self.last_text_type, | ||
| is_last, | ||
| @@ -331,9 +335,10 @@ | ||
| #[inline] | ||
| fn flush_pending_captured_text(&mut self) -> Result<(), RewritingError> { | ||
| self.text_decoder | ||
| - .flush_pending(&mut |text, is_last, encoding, source_location| { | ||
| + .flush_pending(&mut |text, raw, is_last, encoding, source_location| { | ||
| self.delegate.text_token_produced( | ||
| text, | ||
| + raw, | ||
| encoding, | ||
| self.last_text_type, | ||
| is_last, | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.