Skip to content

fix: skip unknown elements that are not part of the content model - #294

Open
rcbevans wants to merge 4 commits into
Bergmann89:masterfrom
AZX-PBC-OSS:fix/skip-unknown-sequence-elements
Open

rcbevans wants to merge 4 commits into
Bergmann89:masterfrom
AZX-PBC-OSS:fix/skip-unknown-sequence-elements

Conversation

@rcbevans

Copy link
Copy Markdown
Contributor

Closes #293 (follow-up with the corrected, red-teamed fix — see the issue for the initial RCA and the discussion of why a driver-side blanket skip is unsound).

Problem

Deserializers generated for xs:sequence content cannot skip unknown child elements. When a start event matches no element of the content model, every generated state machine declines the event (allow_any: false) and the driver raises ErrorKind::UnexpectedEvent. Unknown content is only tolerated at explicit xs:any positions.

Real-world HPXML files hard-fail because of this: the main HPXML producer (OS-HPXML, used by NREL's ResStock) emits legacy v2-style elements (Site/SiteID, Site/Latitude, Site/Longitude) that exist in no HPXML XSD (v2–v5). In a 489-file corpus (OCHRE samples, BEopt examples, ResStock buildings), 33 files fail to parse.

Example input

<HPXML xmlns='http://hpxmlonline.com/2023/09' schemaVersion='4.0'>
  <!-- ... valid required header ... -->
  <Building>
    <BuildingID id='MyBuilding'/>
    <Site>
      <SiteID id='SiteID'/>          <!-- unknown in v4 content model -->
      <Latitude>40.01</Latitude>     <!-- unknown in v4 content model -->
    </Site>
    <ProjectStatus><EventType>audit</EventType></ProjectStatus>
  </Building>
</HPXML>

Before: Unexpected event: Start(BytesStart { name: "Latitude" }) — the first unknown (SiteID) is declined, the second raises. With v3 types the same file fails because Latitude is unknown there too (it lives at Site/GeoLocation/Latitude).

Root cause

The driver (xsd-parser-types/src/quick_xml/deserialize.rs::handle_event) can only skip a subtree when the deserializer chain reports allow_any: true. Generated xs:sequence state machines never report that for unmatched tags, and the driver cannot distinguish "unknown tag" from "known tag violating cardinality/placement" — a naive blanket skip silently accepts maxOccurs violations and drops known-but-misplaced elements (found by red-teaming the naive fix against group_ref_exact_occurs_boundary).

Fix

Content-model knowledge now lives in the generated code:

  1. New defaulted trait method Deserializer::is_known_start_tag(helper, &BytesStart) -> bool (default true, so hand-written deserializers stay strict).
  2. The renderer emits an override per generated struct/enum listing its content-model tags (namespace-aware, following group refs and content types).
  3. The driver skips an unconsumed Start/Empty only when is_known_start_tag returns false — i.e. the tag is genuinely outside the root type's content model. Cardinality/duplicate/order violations of known tags error exactly as before.
  4. The sequence next() template captures its entry state and restores it when an event surfaces unconsumed, so optional/advance-past slots stay open after a skip and in-flight child deserializers remain coherent.

Test

  • New feature test tests/feature/unknown_element_in_sequence/: minimal schema + 9 XML variants (unknown between/after known children, nested unknown subtrees, unknown Empty, maxOccurs boundary with and without unknowns, missing-required-with-unknown still errors).
  • Driver unit tests (8) covering skip state transitions incl. EOF mid-skip.
  • 97 regenerated fixture files (pure additions from the new trait method).
  • Full workspace: 699 passed / 0 failed; clippy/fmt/doc clean (excluding the pre-existing benchmark crate compile issue, outside default-members).

Impact

HPXML documents from the primary ecosystem writer (OS-HPXML/ResStock) parse successfully; strict enforcement of the content model is preserved for everything the schema actually declares.

Generated deserializers raised `ErrorKind::UnexpectedEvent` when a document
contained an element that no generated state machine consumed (e.g. legacy
or vendor specific child elements inside an `xs:sequence`). The driver
errored on these unconsumed start/empty events, because the generated code
conservatively reported them as not allowed (`allow_any: false`).

The driver now consults the new `Deserializer::is_known_start_tag` method
to decide how to handle an unconsumed element:

- If the tag is part of the content model of the type, the element violates
  the model (duplicated, misplaced or exceeding `maxOccurs`) and an
  `UnexpectedEvent` error is raised, preserving the previous behavior.
- If the tag is not part of the content model, the element is unknown and
  its subtree is skipped (`skip_depth`), so that real-world documents with
  extensions outside of `xs:any` positions can be processed.

The default implementation of `is_known_start_tag` considers every tag as
known, which keeps built-in types strict. The quick-xml renderer emits a
precise override for every generated struct and enum deserializer that
lists the tags of the content model, delegating to the deserializers of
group references and content types for nested elements.

The sequence state machine now also restores its entry state when an event
is returned to the caller unconsumed. Without this, the handler loop could
advance past slots that did not match the event, losing optional slots
after an unknown element was skipped.

Fixes the rejection of valid documents that contain unknown elements
(upstream issue Bergmann89#293).
Add the `unknown_element_adversarial` feature test that probes the
skip behavior of the deserialization driver with hostile documents:

- Cardinality masking: unknown elements must not satisfy minOccurs,
  must not hide maxOccurs violations of known elements and must not
  swallow duplicated known elements.
- Namespaces: foreign namespaces with known local names, rebound or
  undeclared default namespaces and duplicated prefix declarations
  inside skipped subtrees must keep the namespace scope stack balanced.
- State coherence: consecutive unknowns, unknowns before the root end
  and unknowns while a nested deserializer is in-flight must not corrupt
  the state machines of following elements.
- Recursion: unknowns inside recursive types and indirectly recursive
  model groups (group -> element -> type -> group) must terminate.
- Robustness: 1000 levels of unknown nesting, EOF in the middle of a
  skipped subtree, unknown as last element before EOF, empty documents
  and unknown elements with many attributes.
- Boundaries: case sensitive tag matching in `is_known_start_tag` and
  coverage of the async driver path.
Probe two additional edge cases of the unknown element skip logic:

- Attributes that merely start with `xmlns` (e.g. `xmlnsx`) are not
  namespace declarations and must neither break the namespace scope
  stack nor the parse when they appear inside a skipped subtree.
- Unknown elements that surface from a nested child deserializer while
  it is staged in the fallback slot must not prevent the child from
  completing correctly after the skip.

Also document the upstream event bubbling semantics with a dedicated
test: an unknown element whose tag matches a still open slot of the
root type is claimed by the state machine of the root type and never
surfaces at the driver. This behavior is independent of the skip logic
and stays unchanged.
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.

Generated deserializer raises UnexpectedEvent for unknown elements inside xs:sequence (breaks real-world HPXML parsing)

1 participant