[xujiantop-crypto]

What and why

Refs #187 (the bare < text cases).

A DOCX text run such as Pump A < pump B can be consumed as part of a malformed tag. The shared XML parser then recovers its tree and the public conversion API returns success with content missing. This prevents ingestion callers from detecting the failed conversion and choosing a fallback.

Reject empty start-tag names and raw < bytes swallowed inside start, self-closing, or end-tag events with ConvertError::Malformed. The end-tag check covers the same truncation mechanism for input such as Pump A </ pump B. Document the recovery distinction and cover both the parser and public DOCX conversion path. Properly escaped <, CDATA, and existing unclosed/mismatched-tag recovery retain their behavior; a regression checks that mismatched elements keep the available text.

This change targets delimiter-driven content loss. It does not turn the lenient parser into a full XML validator. The separate altChunk observation in #187 is outside this change; the entire issue is not claimed resolved.

Verification

For this update, on Windows with Rust 1.99.0:

• The added parser regression fails on the original end-tag path and passes with the end-tag check.
• With only the new end-tag check disabled, the public DOCX regression fails; restoring the check makes it pass.
• Directly compiling the current src/lib.rs tests with the existing cached dependency artifacts matching Cargo.lock passes all 292 library unit tests. A separate executable importing the actual XML, error, and limits modules passes all 15 tests. Neither run substitutes a rewritten parser.
• cargo fmt --all --check passes.

These direct compiler runs do not replace the Cargo integration suite or workspace Clippy. Local Cargo test attempts stop in dependencies because the Windows GNU toolchain cannot create the required DLL import libraries; full Cargo/Clippy and Node/Python/Wasm checks have not passed for this update. The regressions use constructed XML and DOCX fixtures, not a replay of the reporter's complete documents.

For the earlier revision d9edcdd, the Ubuntu validation run passed formatting, workspace Clippy, and cargo test --locked with 300 tests passing and one existing test ignored. That run predates this end-tag update. The reported PPTX/XLSX files and Node binding have not been replayed.

---

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 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 and end events, including self-closing ones; properly escaped < and CDATA text remain readable.

^{Written for commit b778a37. Summary will update on new commits.}

[cubic-dev-ai]

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>

<sub>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</sub>

[cubic-dev-ai]

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>

[rahul05ranjan]

Good catch on the failure mode — a converter that returns success with content missing is much worse than one that errors, and pointing ingestion callers at a detectable Malformed is the right call. I checked the check itself for false positives and it's clean: a well-formed document can never produce a start tag with an empty name or a raw < inside it (< stays escaped in the raw bytes quick-xml hands over), comments, PIs and CDATA are separate event types that this check never sees, and the test confirming < and CDATA text still round-trip pins that. The placement before the MAX_XML_DEPTH check also means malformed input fails before any limit errors, which reads better.

Two notes:

1. There's now an implicit invariant worth a comment in src/package/xml.rs: recovery is allowed when it loses structure (unclosed tags keep all text), but forbidden when it loses content (bare < swallowed as a start tag). A reader of unclosed_elements_recovered sitting right under the new tests could reasonably wonder why one is recovered and the other is rejected; one sentence on the module or parse_xml doc would prevent that question from being asked again.

2. The check covers Start/Empty events, which is where quick-xml swallows the trailing text. A bare delimiter shaped like an end tag — e.g. <r>Pump A </ pump B</r>, where recover mode might produce an End event with a padded name — isn't covered by the new tests. If quick-xml already errors or recovers that without losing text, a test proving it would close the loop; if it can also silently truncate, Event::End may want a symmetric check.

The verification write-up is unusually solid — real product files, toggled checks proving the regressions fail without the fix, and an explicit statement of what's not claimed (altChunk, PPTX/XLSX, Node binding). That honesty makes the PR much easier to trust.

[xujiantop-crypto]

The end-tag case exposes the same truncation path: quick-xml can include the following raw < inside an End event, bypassing the start-tag checks. The update rejects that event with Malformed, adds parser and public DOCX regressions, and documents why structural recovery is retained.

The new parser case fails before the check. Disabling only the new check also makes the public DOCX regression fail; restoring it makes both pass. Direct compilation of the current library tests passes 292 unit tests, and cargo fmt --all --check passes. Full Cargo/Clippy and binding validation for this update remain unverified; the PR body now distinguishes these results from the earlier Ubuntu run.