Skip to content

Accept documents that only produce warnings and fix leak#159

Open
shunkica wants to merge 2 commits into
jameslan:masterfrom
shunkica:fix/parse-warning-severity-and-leak
Open

Accept documents that only produce warnings and fix leak#159
shunkica wants to merge 2 commits into
jameslan:masterfrom
shunkica:fix/parse-warning-severity-and-leak

Conversation

@shunkica

Copy link
Copy Markdown
Contributor

Fixes #158.

Changes

  • Capture severity. Add XmlErrorStruct.level (xmlError struct offset 12)
    and a public ErrorDetail.level field (1 = warning, 2 = error, 3 =
    fatal); errorCollector now records it.
  • Gate on severity, not count. parse() throws only when a diagnostic is
    >= XML_ERR_ERROR (2) or no document was produced. This keeps rejecting
    genuine errors - including XML_PARSE_DTDVALID validity errors, which
    leave the document well-formed (non-null) but must still fail.
  • Fix the leak. The corrected guard frees the produced document before
    throwing (e.g. a partial doc from XML_PARSE_RECOVER).
  • Surface warnings. Non-fatal diagnostics were previously discarded with no
    way to observe them. They are now exposed on the parsed document via a new
    XmlDocument.warnings: ErrorDetail[] accessor - the same ErrorDetail shape
    used by XmlParseError.details, mirrored onto the success path. It is []
    for clean parses and for documents created with XmlDocument.create().

shunkica added 2 commits July 24, 2026 11:32
XmlDocument.fromString/fromBuffer rejected well-formed documents whenever
libxml2 emitted any diagnostic, including warning-level ones, and leaked the
parsed C document on that throw.

Two defects in parse() (src/document.mts):

1. Failure was decided by presence of any diagnostic (errDetails.length > 0),
   never by severity, so warnings (XML_ERR_WARNING = 1) were treated as fatal.
   Now failure is gated on level >= XML_ERR_ERROR (2), so warnings pass and
   error/fatal diagnostics (incl. XML_PARSE_DTDVALID validity errors) still throw.

2. The pre-throw cleanup guard was inverted (`if (!xml) xmlFreeDoc(xml)`): it
   freed only when the pointer was null and skipped freeing a real document,
   which then leaked since the throw precedes wrapper/finalizer creation. The
   guard is corrected to free the produced document before throwing.

To make surviving warnings observable (they were silently discarded), capture
the diagnostic severity and expose non-fatal diagnostics on the parsed document:

- XmlErrorStruct.level (xmlError offset 12) + ErrorDetail.level
- XmlDocument.warnings: ErrorDetail[] (level-1 diagnostics; [] for clean parses)

Existing details assertions updated for the new level field; adds tests for
warning acceptance, warning surfacing, validity-error rejection, and no-leak.
@github-actions

Copy link
Copy Markdown

Coverage after merging fix/parse-warning-severity-and-leak into master will be

100.00%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   c14n.mts100%100%100%100%
   diag.mts99.51%100%90.91%100%
   disposable.mts100%100%100%100%
   disposeShim.mts100%100%100%100%
   document.mts100%100%100%100%
   dtd.mts100%100%100%100%
   index.mts100%100%100%100%
   libxml2.mts100%100%100%100%
   metadataShim.mts100%100%100%100%
   nodejs.mts100%100%100%100%
   nodes.mts100%100%100%100%
   utils.mts100%100%100%100%
   validates.mts100%100%100%100%
   xpath.mts100%100%100%100%

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.

Valid documents that only produce warnings are rejected (and the parsed document is leaked)

1 participant