You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: reject raw '<' and stray '&' in attribute values - #913
XML 1.0's AttValue grammar excludes a literal < character in attribute values, and every literal & must begin a reference. XMLValidator accepted both: <a x="a & b"/> and <a x="a < b"/> returned true, and parsing with validation enabled accepted them too. (Escaped forms such as <, & and < are unaffected and stay valid, before and after.)
validateAttributeString now also checks the extracted attribute value:
a literal < character is rejected;
a literal & that is not followed by a reference start (# or a name start character) is rejected.
Scope of the ampersand check
This is intentionally a start-of-reference check, not full XML reference validation. The parser keeps unknown and malformed references literal instead of rejecting them - verified on master: &#xZZ;, &#;, &unknown; and the HTML-legacy &bar all parse to their literal text, and existing entity/value-processor specs depend on that contract (3 specs fail if references are validated strictly). So name-like runs after &, including malformed numeric references, stay accepted here, consistent with the parser. Resolving or strictly validating references would be a separate behavior decision.
Input (from the issue):
<ax="a & b"/>
<ax="a < b"/>
<ax="a & b"/>
Before: all three validate as true. After: the first two return InvalidAttr ("value contains '&' that doesn't start a reference" / "value can't contain '<'"), the escaped & control stays valid, and parse(xml, true) rejects the malformed inputs.
Type
Bug Fix
Tests
5 new specs in spec/validator_spec.js: 3 negative cases (literal & not starting a reference, literal <, & not followed by a name), one control spec pinning that escaped references (&, <, A, A) stay valid, and one control spec pinning the intentional HTML-legacy &bar leniency.
The 3 negative specs fail on current master and pass with this change; full suite npm test: 345 specs, 0 failures (2 pending, pre-existing).
Perf: node benchmark/XmlParser.mjs before/after this change: 46,443 vs 45,424 requests/second for fxp, within run-to-run noise (fxp v3 moved the same direction between the same two runs); the validator is not on this benchmark's parse path.
Lint: no new findings; the single no-extra-semi report at src/validator.js:191 exists on master.
XML 1.0's AttValue grammar excludes a literal < character in attribute values, and every literal & must begin a reference. XMLValidator accepted both: <a x="a & b"/> and <a x="a < b"/> returned true, and parsing with validation enabled accepted them too. (Escaped forms such as <, & and < are unaffected and stay valid, before and after.)
validateAttributeString now also checks the extracted attribute value:
a literal < character is rejected;
a literal & that is not followed by a reference start (# or a name start character) is rejected.
Scope of the ampersand check
This is intentionally a start-of-reference check, not full XML reference validation. The parser keeps unknown and malformed references literal instead of rejecting them - verified on master: &#xZZ;, &#;, &unknown; and the HTML-legacy &bar all parse to their literal text, and existing entity/value-processor specs depend on that contract (3 specs fail if references are validated strictly). So name-like runs after &, including malformed numeric references, stay accepted here, consistent with the parser. Resolving or strictly validating references would be a separate behavior decision.
Input (from the issue):
<a x="a & b"/>
<a x="a < b"/>
<a x="a & b"/>
Before: all three validate as true. After: the first two return InvalidAttr ("value contains '&' that doesn't start a reference" / "value can't contain '<'"), the escaped & control stays valid, and parse(xml, true) rejects the malformed inputs.
Type
Bug Fix
Tests
5 new specs in spec/validator_spec.js: 3 negative cases (literal & not starting a reference, literal <, & not followed by a name), one control spec pinning that escaped references (&, <, A, A) stay valid, and one control spec pinning the intentional HTML-legacy &bar leniency.
The 3 negative specs fail on current master and pass with this change; full suite npm test: 345 specs, 0 failures (2 pending, pre-existing).
Perf: node benchmark/XmlParser.mjs before/after this change: 46,443 vs 45,424 requests/second for fxp, within run-to-run noise (fxp v3 moved the same direction between the same two runs); the validator is not on this benchmark's parse path.
Lint: no new findings; the single no-extra-semi report at src/validator.js:191 exists on master.
XML 1.0's AttValue grammar excludes a literal < character in attribute values, and every literal & must begin a reference. XMLValidator accepted both: <a x="a & b"/> and <a x="a < b"/> returned true, and parsing with validation enabled accepted them too. (Escaped forms such as <, & and < are unaffected and stay valid, before and after.)
validateAttributeString now also checks the extracted attribute value:
a literal < character is rejected;
a literal & that is not followed by a reference start (# or a name start character) is rejected.
Scope of the ampersand check
This is intentionally a start-of-reference check, not full XML reference validation. The parser keeps unknown and malformed references literal instead of rejecting them - verified on master: &#xZZ;, &#;, &unknown; and the HTML-legacy &bar all parse to their literal text, and existing entity/value-processor specs depend on that contract (3 specs fail if references are validated strictly). So name-like runs after &, including malformed numeric references, stay accepted here, consistent with the parser. Resolving or strictly validating references would be a separate behavior decision.
Input (from the issue):
<a x="a & b"/>
<a x="a < b"/>
<a x="a & b"/>
Before: all three validate as true. After: the first two return InvalidAttr ("value contains '&' that doesn't start a reference" / "value can't contain '<'"), the escaped & control stays valid, and parse(xml, true) rejects the malformed inputs.
Type
Bug Fix
Tests
5 new specs in spec/validator_spec.js: 3 negative cases (literal & not starting a reference, literal <, & not followed by a name), one control spec pinning that escaped references (&, <, A, A) stay valid, and one control spec pinning the intentional HTML-legacy &bar leniency.
The 3 negative specs fail on current master and pass with this change; full suite npm test: 345 specs, 0 failures (2 pending, pre-existing).
Perf: node benchmark/XmlParser.mjs before/after this change: 46,443 vs 45,424 requests/second for fxp, within run-to-run noise (fxp v3 moved the same direction between the same two runs); the validator is not on this benchmark's parse path.
Lint: no new findings; the single no-extra-semi report at src/validator.js:191 exists on master.
XML 1.0's AttValue grammar excludes a literal < character in attribute values, and every literal & must begin a reference. XMLValidator accepted both: <a x="a & b"/> and <a x="a < b"/> returned true, and parsing with validation enabled accepted them too. (Escaped forms such as <, & and < are unaffected and stay valid, before and after.)
validateAttributeString now also checks the extracted attribute value:
a literal < character is rejected;
a literal & that is not followed by a reference start (# or a name start character) is rejected.
Scope of the ampersand check
This is intentionally a start-of-reference check, not full XML reference validation. The parser keeps unknown and malformed references literal instead of rejecting them - verified on master: &#xZZ;, &#;, &unknown; and the HTML-legacy &bar all parse to their literal text, and existing entity/value-processor specs depend on that contract (3 specs fail if references are validated strictly). So name-like runs after &, including malformed numeric references, stay accepted here, consistent with the parser. Resolving or strictly validating references would be a separate behavior decision.
Input (from the issue):
<ax="a & b"/>
<ax="a < b"/>
<ax="a & b"/>
Before: all three validate as true. After: the first two return InvalidAttr ("value contains '&' that doesn't start a reference" / "value can't contain '<'"), the escaped & control stays valid, and parse(xml, true) rejects the malformed inputs.
Type
Bug Fix
Tests
5 new specs in spec/validator_spec.js: 3 negative cases (literal & not starting a reference, literal <, & not followed by a name), one control spec pinning that escaped references (&, <, A, A) stay valid, and one control spec pinning the intentional HTML-legacy &bar leniency.
The 3 negative specs fail on current master and pass with this change; full suite npm test: 345 specs, 0 failures (2 pending, pre-existing).
Perf: node benchmark/XmlParser.mjs before/after this change: 46,443 vs 45,424 requests/second for fxp, within run-to-run noise (fxp v3 moved the same direction between the same two runs); the validator is not on this benchmark's parse path.
Lint: no new findings; the single no-extra-semi report at src/validator.js:191 exists on master.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose / Goal
Fixes #900.
XML 1.0's AttValue grammar excludes a literal
<character in attribute values, and every literal&must begin a reference.XMLValidatoraccepted both:<a x="a & b"/>and<a x="a < b"/>returnedtrue, and parsing with validation enabled accepted them too. (Escaped forms such as<,&and<are unaffected and stay valid, before and after.)validateAttributeStringnow also checks the extracted attribute value:<character is rejected;&that is not followed by a reference start (#or a name start character) is rejected.Scope of the ampersand check
This is intentionally a start-of-reference check, not full XML reference validation. The parser keeps unknown and malformed references literal instead of rejecting them - verified on master:
&#xZZ;,&#;,&unknown;and the HTML-legacy&barall parse to their literal text, and existing entity/value-processor specs depend on that contract (3 specs fail if references are validated strictly). So name-like runs after&, including malformed numeric references, stay accepted here, consistent with the parser. Resolving or strictly validating references would be a separate behavior decision.Input (from the issue):
Before: all three validate as
true. After: the first two returnInvalidAttr("value contains '&' that doesn't start a reference" / "value can't contain '<'"), the escaped&control stays valid, andparse(xml, true)rejects the malformed inputs.Type
Tests
spec/validator_spec.js: 3 negative cases (literal¬ starting a reference, literal<,¬ followed by a name), one control spec pinning that escaped references (&,<,A,A) stay valid, and one control spec pinning the intentional HTML-legacy&barleniency.npm test: 345 specs, 0 failures (2 pending, pre-existing).node benchmark/XmlParser.mjsbefore/after this change: 46,443 vs 45,424 requests/second forfxp, within run-to-run noise (fxp v3moved the same direction between the same two runs); the validator is not on this benchmark's parse path.no-extra-semireport atsrc/validator.js:191exists on master.