Skip to content

fix(parser): incorrect parsing of hexadecimal NCRs - #474

Open
grnch wants to merge 2 commits into
SAP:masterfrom
grnch:fix/hex-ncr-parsing
Open

fix(parser): incorrect parsing of hexadecimal NCRs#474
grnch wants to merge 2 commits into
SAP:masterfrom
grnch:fix/hex-ncr-parsing

Conversation

@grnch

@grnch grnch commented Aug 26, 2026

Copy link
Copy Markdown

When you have a decimal NCR (Numeric Character Reference) in your text:

before Ӓ after

it gets parsed like this:

TEXT => "before"
SEA_WS => " "
CharRef => "Ӓ"
SEA_WS => " "
TEXT => "after"

However, if you have a hexadecimal NCR:

before � after

it gets parsed like this instead:

TEXT => "before"
SEA_WS => " "
CharRef => "&#x1"
TEXT => "23ABC;"
SEA_WS => " "
TEXT => "after"

Note the extra TEXT => "23ABC;" token after the CharRef, it shouldn't be there. The NCR got split into two tokens, instead of emitted as one.

The root cause was an apparent typo in the regex for CharRef tokens. It was missing a plus sign and a semicolon on the hexadecimal side of the alternation, which caused it to match only the first hexadecimal digit, instead of the entire NCR all the way to the semicolon.

I added a hex NCR to the valid/entity-char-ref test case and verified that the full CharRef token appeared correctly in the output before updating the snapshot. I also checked that all subsequent offsets were shifted by 15, which corresponds to the 14 chars added plus the newline.

Finally, I added a new invalild/entity-char-ref test case. The old regex would produce some CharRef tokens on this text, even though none of the NCRs in it were valid. With this fix, the text output is now "chardata" only, no CharRef tokens.

grnch added 2 commits August 25, 2026 19:22
When you have a decimal NCR (Numeric Character Reference) in your text:

    before Ӓ after

it gets parsed like this:

    TEXT => "before"
    SEA_WS => " "
    CharRef => "Ӓ"
    SEA_WS => " "
    TEXT => "after"

However, if you have a hexadecimal NCR:

    before � after

it gets parsed like this instead:

    TEXT => "before"
    SEA_WS => " "
    CharRef => "&#x1"
    TEXT => "23ABC;"
    SEA_WS => " "
    TEXT => "after"

Note the extra `TEXT => "23ABC;"` token after the CharRef, it shouldn't
be there. The NCR got split into two tokens, instead of emitted as one.

The root cause was an apparent typo in the regex for CharRef tokens. It
was missing a plus sign and a semicolon on the hexadecimal side of the
alternation, which caused it to match only the first hexadecimal digit,
instead of the entire NCR all the way to the semicolon.

I added a hex NCR to the `valid/entity-char-ref` test case and verified
that the full CharRef token appeared correctly in the output before
updating the snapshot. I also checked that all subsequent offsets were
shifted by 15, which corresponds to the 14 chars added plus the newline.

Finally, I added a new `invalild/entity-char-ref` test case. The old
regex would produce some CharRef tokens on this text, even though none
of the NCRs in it were valid. With this fix, the text output is now
"chardata" only, no CharRef tokens.
When editing test fixtures and looking at the resulting test output, the
diffs might get large due to a bunch of irrelevant offset changes.

By the time you get to the interesting part of the diff, it gets cut off
due to the 8k limit.
@cla-assistant

cla-assistant Bot commented Aug 26, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.

1 participant