Skip to content

fix(xml): reject delimiters that silently truncate text - #188

Open
xujiantop-crypto wants to merge 1 commit into
firecrawl:mainfrom
xujiantop-crypto:fix/reject-bare-xml-delimiters
Open

xujiantop-crypto wants to merge 1 commit into
firecrawl:mainfrom
xujiantop-crypto:fix/reject-bare-xml-delimiters

Conversation

@xujiantop-crypto

@xujiantop-crypto xujiantop-crypto commented Oct 9, 2026 •

Copy link
Copy Markdown

What and why

Refs #187 (the bare < text case).

A DOCX text run such as Pump A < pump B can 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 with ConvertError::Malformed, including self-closing events. Properly escaped &lt; 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:

  • With the new delimiter checks disabled, both parser and public DOCX regressions fail with assertion failures; restoring the checks makes them pass.
  • cargo fmt --all --check passes.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings passes.
  • 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 altChunk observation 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 like Pump A < pump B returns ConvertError::Malformed instead 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 &lt; and CDATA text remain readable.

Written for commit d9edcdd. Summary will update on new commits.

View guided diff

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files

Confidence score: 4/5

  • In src/package/xml.rs, parse_xml silently 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

Comment thread src/package/xml.rs
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'<') {

@cubic-dev-ai cubic-dev-ai Bot Oct 9, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Fix with cubic

This branch has not been deployed

No deployments
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.

1 participant