Repository navigation
fix(xml): reject delimiters that silently truncate text - #188
Open
xujiantop-crypto wants to merge 1 commit into
Open
xujiantop-crypto wants to merge 1 commit into
xujiantop-crypto wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 4/5
- In
src/package/xml.rs,parse_xmlsilently drops text containing a bare<followed by?or!. Preserve or reject that content instead of discarding it.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/package/xml.rs">
<violation number="1" location="src/package/xml.rs:286">
P3: The check only intercepts `Event::Start`/`Event::Empty`. A bare `<` in text followed by `?` or `!` is consumed as a PI/declaration event, and the `_ => {}` arm in `parse_xml` drops those silently, so input like `<r>Pump <? x ?> B</r>` still returns success with the `<? x ?>` body lost — the same silent-truncation class #187 describes. Consider failing (or preserving) text swallowed into `Event::PI`/`Event::Comment` too, or at least covering `<?`/`<!` in the parser regressions.</violation>
</file>
Click Fix with cubic on a comment, or reply @cubic-dev-ai fix this, and cubic writes the fix.
View guided diff | Re-trigger cubic
| fn check_start_delimiters(e: &BytesStart<'_>) -> Result<(), ConvertError> { | ||
| // quick-xml can consume a bare '<' in text together with the next end | ||
| // tag as one start event. Recovering that tree silently loses content. | ||
| if e.name().as_ref().is_empty() || e.contains(&b'<') { |
There was a problem hiding this comment.
P3: The check only intercepts Event::Start/Event::Empty. A bare < in text followed by ? or ! is consumed as a PI/declaration event, and the _ => {} arm in parse_xml drops those silently, so input like <r>Pump <? x ?> B</r> still returns success with the <? x ?> body lost — the same silent-truncation class #187 describes. Consider failing (or preserving) text swallowed into Event::PI/Event::Comment too, or at least covering <?/<! in the parser regressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/package/xml.rs, line 286:
<comment>The check only intercepts `Event::Start`/`Event::Empty`. A bare `<` in text followed by `?` or `!` is consumed as a PI/declaration event, and the `_ => {}` arm in `parse_xml` drops those silently, so input like `<r>Pump <? x ?> B</r>` still returns success with the `<? x ?>` body lost — the same silent-truncation class #187 describes. Consider failing (or preserving) text swallowed into `Event::PI`/`Event::Comment` too, or at least covering `<?`/`<!` in the parser regressions.</comment>
<file context>
@@ -278,6 +280,15 @@ pub fn parse_xml(bytes: &[u8]) -> Result<Element, ConvertError> {
+fn check_start_delimiters(e: &BytesStart<'_>) -> Result<(), ConvertError> {
+ // quick-xml can consume a bare '<' in text together with the next end
+ // tag as one start event. Recovering that tree silently loses content.
+ if e.name().as_ref().is_empty() || e.contains(&b'<') {
+ return Err(ConvertError::malformed("unparseable xml: invalid start-tag delimiter"));
+ }
</file context>
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What and why
Refs #187 (the bare
<text case).A DOCX text run such as
Pump A < pump Bcan be consumed as a malformed start tag. The shared XML parser then recovers its tree and the public conversion API returns success with content missing. This makes it difficult for ingestion callers to detect the failed conversion and use a fallback.Reject empty start-tag names and raw
<delimiters inside start events withConvertError::Malformed, including self-closing events. Properly escaped<and CDATA text remain readable. Existing unclosed/mismatched-tag recovery is unchanged.Verification
Validated on Ubuntu in this run, using the exact product source files:
cargo fmt --all --checkpasses.cargo clippy --workspace --all-targets --all-features -- -D warningspasses.cargo test --locked: 300 tests pass, 1 existing test ignored.The checks use constructed XML and DOCX fixtures based on the report. Parser regressions cover spaced, leading and adjacent bare delimiters, empty start/empty tags and a raw delimiter in an attribute. Public Rust DOCX conversion tests cover malformed text and preservation of later paragraphs with escaped text.
The separate
altChunkobservation in #187 is outside this change. PPTX/XLSX reported files and the Node binding have not been replayed; this PR does not claim the entire issue is resolved.AI-assisted implementation and independent review using Codex.
Summary by cubic
Rejects bare
<delimiters in XML so DOCX text likePump A < pump BreturnsConvertError::Malformedinstead of being silently truncated. Previously the parser recovered malformed start tags and the conversion API returned success with content missing, making failures hard for ingestion callers to detect. The parser now rejects empty start-tag names and raw<delimiters inside start events, including self-closing ones; properly escaped<and CDATA text remain readable.Written for commit d9edcdd. Summary will update on new commits.