Repository navigation
fix: resolve named HTML entities longer than 10 letters in set_mathml() - #922
Merged
Merged
Conversation
The entity regex in set_mathml() bounded the entity name at 10 letters:
&([a-zA-Z]{2,10});
but 2127 of the names in src/entities.in are longer, up to
"CounterClockwiseContourIntegral" (31 letters). Those names were never
substituted, so they reached the XML parser unresolved and set_mathml()
rejected the whole expression with "Tag/attribute not found" instead of
translating it.
MathML produced by MathJax uses ⁢ (14 letters) for
invisible multiplication, which is why the regression was reported by
JAWS users as soon as 0.7.7 shipped (issue daisy#896).
Widen the bound to 32 letters, matching the {1,32} bound already used
for namespace declarations and element prefixes. Unknown names are still
rejected by the parser, so the DoS guard is unaffected.
Add tests/entities.rs covering names at the old limit, one letter over
it, the longest name in the table, numeric character references, and
that unknown names are still rejected.
Linux library size: 0.59 MiB (0.00%)
Built with default features, Rust 1.96.0, and Ubuntu 24.04. Workflow run. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #896.
Problem
set_mathml()substitutes named HTML entities before handing the string to theXML parser. The regex that finds them bounded the name at 10 letters:
2127 of the 2235 names in
src/entities.inare longer than that, up toCounterClockwiseContourIntegral(31 letters). Those names were neversubstituted, so they reached the parser unresolved and
set_mathml()rejectedthe entire expression instead of translating it.
MathML emitted by MathJax uses
⁢(14 letters) for invisiblemultiplication, which is why this surfaced immediately after 0.7.7 shipped.
Reproduction (before this change)
Fix
Widen the bound to 32 letters in
src/interface.rs, matching the{1,32}bound already used for the namespace and element-prefix regexes right above it.
Unknown names are still rejected by the XML parser, so the DoS length guard
behaviour is unchanged — the bound only decides how many characters are offered
to the entity lookup, and the lookup itself is a static
phfmap.Tests
New integration test file
tests/entities.rs(6 tests) covering:⁢— 14 letters, the exact case from 0.7.7 rejects entity names longer than 10 letters (for example ⁢) #896⊗— 11 letters, one over the old bound∳— 31 letters, the longest name in the table⁢,⁡,π,→,ⅈ— names at or under theold bound keep resolving (regression guard)
⁢— numeric character references are unaffected&NotAnEntityNameAtAll;— unknown names are still rejectedRed before the change (
4 failed), green after. Full suite:cargo test→5210 passed, 0 failed, 33 ignored (the new file adds 6 of those 5210).