[Sskift]

Purpose / Goal

Fixes #903. XMLValidator.validate("<a><!--foo--bar--></a>") currently succeeds, although XML 1.0 section 2.5 disallows double hyphens inside comments. Parsing the same input with validation enabled also succeeds.

Reject double hyphens unless they begin the closing -->, and return the existing error shape from both comment-scanning paths. Tests cover comments before, inside and after the root; invalid trailing hyphens; valid single hyphens; error location; and CDATA controls.

Type

• Bug Fix
• Refactoring / Technology upgrade
• New Feature

Validation

• The new regressions produce 8 failures against the original validator.
• Validator suite: 79 specs pass. Full suite: 346 specs, 0 failures, 2 existing pending.
• Type checks and git diff --check pass.
• 1,092 generated comment cases agree with Python ElementTree. A larger 16,383-case comparison fixes 5,554 results without introducing a new disagreement; 253 existing disagreements concern trailing text after a self-closing root and are outside this change.
• ESLint reproduces two existing errors on the original files: unsupported import.meta syntax in the configured parser, and an extra semicolon at the end of the validator function.

Performance

Ran the requested node benchmark/XmlParser.mjs before and after. Requests/sec (before → after): fxp v3 115769 → 136077; fxp 55594 → 48564; fxp preserve-order 69419 → 47493; xmlbuilder2 28148 → 20534; xml2js 28694 → 28303. These shared-host Windows runs were noisy and this benchmark does not enable validation.

An interleaved validator comparison (7 rounds, 3,000 validations per round) measured median times of 33.75 → 34.51 ms without comments and 40.32 → 39.94 ms with comments. These are local observations.

Prepared with Codex assistance, automated tests, and independent agent review.

[amitguptagwl]

can you please check if the changes needed in validator needed on fast-xml-validator?

[Sskift]

Checked fast-xml-validator main at 335fcaf (v1.4.2) with Node 22.23.2. It already rejects <a><!--foo--bar--></a> when invalidCharSequence: { comment: true } is enabled; accepting it by default is explicitly covered by its opt-in tests.

There is still a gap with the option enabled: <a><!--foo---></a> returns true. CommentCdataReader checks only the extracted body for --, so the body's trailing - next to the closing delimiter is missed. Clean comments and empty comments passed the same local checks.

So the basic interior--- check already exists there, but the trailing-hyphen regression from this PR also needs a fix in that repository. Should this PR retain unconditional rejection in fast-xml-parser, or follow the validator's opt-in policy?

[yxshee]

Checked the branch locally against libxml2 (xmllint --noout). <r><!-- a--b --></r>, <r><!--a---></r> and <?xml version="1.0"?><!-- a--b --><r/> are now rejected, and <!---->, <!---leading--> and <!--foo- bar--> stay valid, all matching libxml2. Merged with current master (the branch is 15 commits behind) it merges cleanly and the validator spec runs 79 specs with 0 failures.

One path is not covered: a DOCTYPE goes through the DOCTYPE branch of readCommentAndCDATA, which only counts < and > until the declaration closes, so a comment in the internal subset is skipped without the new check: <!DOCTYPE r [<!-- a--b -->]><r/> still validates on the branch, while libxml2 rejects it ("Double hyphen within comment"). If the goal is to reject -- in every comment, that path needs the same check; otherwise it may be worth saying in the description that the internal subset is out of scope.