GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Replace numeric character references to surrogates with U+FFFD by simpleqt · Pull Request #474 · commonmark/commonmark-java

Conversation

@simpleqt

@simpleqt simpleqt commented Nov 4, 2026

Copy link
Copy Markdown

Problem

A numeric character reference to a code point in the surrogate range is kept as a lone surrogate:

Parser.builder().build().parse("&#xD800;");   // <p>\uD800</p>

The spec, Entity and numeric character references: "Invalid Unicode code points will be replaced by the REPLACEMENT CHARACTER (U+FFFD)". A code point in the surrogate range (U+D800–U+DFFF) is not a valid Unicode code point. The spec.txt used by the tests only exercises &#0;, so the spec tests do not cover this.

Both reference implementations replace the range: cmark in houdini_unescape_ent (src/houdini_html_u.c: codepoint == 0 || (codepoint >= 0xD800 && codepoint < 0xE000) || codepoint >= 0x110000 → 0xFFFD), and commonmark.js 0.31.2 renders <p> + U+FFFD + </p> for the same input.

Because a lone surrogate is not valid UTF-16, the corruption is visible downstream as well: writing the rendered HTML as UTF-8 replaces it with ?.

Fix

Html5Entities.entityToString only special-cased 0 and relied on Character.toChars throwing for values above U+10FFFF, but Character.toChars(0xD800) succeeds. The surrogate range is now rejected in the same way as 0, using Character.MIN_SURROGATE/MAX_SURROGATE, so valid supplementary code points are not affected.

entityToString is also used when unescaping link destinations and link reference definitions, so those are covered by the same change.

Tests

Added SpecialInputTest.surrogateCharacterReferenceShouldBeReplaced, next to the existing nullCharacterEntityShouldBeReplaced:

  • &#xD800;, &#55296;, &#xDFFF; and before&#xD800;after → U+FFFD
  • &#x1F600; (U+1F600) and &#x10FFFF; unchanged

Verification:

  • Base commit without the fix: Tests run: 1, Failures: 1
  • With the fix: SpecialInputTest → Tests run: 33, Failures: 0
  • mvn -B package on all modules with JDK 17: BUILD SUCCESS, including the spec tests (1959 tests, 0 failures)
  • mvn spotless:check: clean

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@simpleqt
Replace numeric character references to surrogates with U+FFFD
7cb9ff6
A numeric character reference to a code point in the surrogate range, such as
`&#xD800;`, produced a lone surrogate in the text. The spec says that invalid
Unicode code points are replaced by the REPLACEMENT CHARACTER, and a surrogate
is not a valid Unicode code point; cmark and commonmark.js both replace the
whole range.

The resulting string is not valid UTF-16 and cannot be encoded, so it turned
into "?" when the rendered HTML was written as UTF-8.

Html5Entities.entityToString only special-cased 0 and relied on
Character.toChars throwing for values above U+10FFFF, but Character.toChars
accepts the surrogate range.
Copilot AI balanced review requested due to automatic review settings November 4, 2026 17:22
Copilot AI reviewed Nov 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@simpleqt
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.