Skip to content

fix: end processing instruction at first ?> (#924) - #925

Open
anubhavlal07 wants to merge 1 commit into
NaturalIntelligence:masterfrom
anubhavlal07:fix/pi-quote-closing
Open

anubhavlal07 wants to merge 1 commit into
NaturalIntelligence:masterfrom
anubhavlal07:fix/pi-quote-closing

Conversation

@anubhavlal07

Copy link
Copy Markdown

Purpose / Goal

Fixes #924.

A quote inside a processing instruction makes the parser skip past the real ?>. In <r><?note don't forget?><a>1</a><b>it's</b><?end?><c>3</c></r>, a, b and ?end are dropped. If there is no later matching quote, the parser throws Pi Tag is not closed.

Root cause: the PI branch in OrderedObjParser.js calls readTagExp(xmlData, i, false, "?>"), which calls tagExpWithClosingIndex. That function treats every ' or " as the start of an attribute value and jumps to the next matching quote. PI content is free text, and XML 1.0 production [16] ends a PI at the first ?>.

Fix: tagExpWithClosingIndex gets a skipQuotes parameter (default true). readTagExp passes closingChar !== "?>", so in PIs it does not skip quoted values or replace tabs. Tabs inside quoted PI pseudo-attribute values stay as they are. PI pseudo-attributes such as <?xml version="1.0"?> and xml-stylesheet are still parsed by buildAttributesMap on the extracted expression. Its regex already splits on any whitespace. Normal tags behave exactly as before.

Tests added to spec/pi_spec.js:

  • an apostrophe in PI content keeps the elements that follow it
  • an unmatched quote in PI content no longer throws
  • a double quote in PI content (preserveOrder)
  • a PI ends at the first ?> even inside a quoted value, which matches the validator's existing expectation
  • tabs inside quoted PI attribute values are kept, which guards against a side effect of the change

The first four fail on master and pass with this change. Full suite: 348 specs, 0 failures.

Benchmark (node benchmark/XmlParser.mjs, spec/assets/ptest.xml). Runs on this machine vary a lot between identical runs, but there is no regression. The change only touches PI scanning.

before after
fxp 9259 req/s 9309 req/s
fxp - preserve order 12325 req/s 16821 req/s

Type

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature

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.

Quote inside a processing instruction drops the elements that follow it

1 participant