Skip to content

Fix literal-prefix search starting inside surrogate pairs. - #208

Open
jemmix wants to merge 1 commit into
google:masterfrom
jemmix:fix-surrogate-pair-interior-prefix
Open

jemmix wants to merge 1 commit into
google:masterfrom
jemmix:fix-surrogate-pair-interior-prefix

Conversation

@jemmix

@jemmix jemmix commented Sep 11, 2026

Copy link
Copy Markdown

Fixes #207.

MachineInput.StringInput.index() jumps to raw indexOf hits; a hit on the low half of a well-formed surrogate pair is not a codepoint boundary — skip it and keep searching. An explicitly given search start is honored as-is: Matcher.find(int) handed a pair-interior position matches AT it (java.util.regex parity), so only hits strictly beyond the start are scan hits subject to the boundary rule.

Adds SurrogatePairTest: no scan match starts inside a well-formed pair regardless of pattern shape (literal / class / alternation — previously the result depended on whether the pattern compiled to a singleton literal prefix, a monotonicity violation); search resumes past skipped interior hits; lone surrogates at real boundaries still match; explicit interior starts are honored; supplementary matching unchanged.

Disclosure: patch and tests composed agentically with GLM 5.3 assistance, as noted in #207; verified locally with ./gradlew check on JDK 8 (1832 tests, 0 failures, format/license gates green).

@google-cla

google-cla Bot commented Sep 11, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@jemmix jemmix closed this Sep 11, 2026
@jemmix jemmix reopened this Sep 11, 2026
@jemmix

jemmix commented Sep 11, 2026

Copy link
Copy Markdown
Author

withdrawing for now to put more thought into this as testing found out scenarios in which this isn't the proper solution

@jemmix jemmix closed this Sep 11, 2026
MachineInput.StringInput.index() jumps to raw String.indexOf hits; a hit
on the low half of a well-formed surrogate pair is not a codepoint
boundary -- skip it and keep searching. An explicitly given search start
is honored as-is: Matcher.find(int) handed a pair-interior position
matches AT it (java.util.regex parity), so only hits strictly beyond the
start are scan hits subject to the boundary rule.

Adds SurrogatePairTest: no scan match starts inside a well-formed pair
regardless of pattern shape (literal / class / alternation -- previously
the result depended on whether the pattern compiled to a singleton
literal prefix, a monotonicity violation); search resumes past skipped
interior hits; lone surrogates at real boundaries still match; explicit
interior starts are honored; supplementary matching unchanged.
@jemmix jemmix reopened this Sep 17, 2026
@jemmix
jemmix force-pushed the fix-surrogate-pair-interior-prefix branch from dfea17f to 420ec9c Compare September 17, 2026 18:25
@jemmix

jemmix commented Sep 17, 2026

Copy link
Copy Markdown
Author

Reopening — the withdrawal note said testing had found scenarios where the original fix was not the proper solution; those are now addressed on the rebuilt branch.

What was wrong with the original patch: it skipped every pair-interior indexOf hit, including a hit exactly at an explicitly given search start. But Matcher.find(start) handed a position that is a pair's low unit should match there: java.util.regex honors an explicit interior start, and so do re2j's own search paths that bypass the literal-prefix fast path. Only hits strictly beyond the given start are scan hits subject to the codepoint-boundary rule. That overcorrection is also why the original patch lacked a story for find(int) parity.

What changed (branch rebuilt on current master, commit 420ec9c):

  • The skip is narrowed to interior hits strictly beyond the given start (i > pos), so an explicit interior start is honored.
  • New test explicitInteriorStartIsHonored pins it: matcher("\uDC21", "a\uD801\uDC21b").find(2) matches at 2.
  • The test class now also covers pattern-shape consistency (literal / singleton class / alternation / range forms all agree — the original report's monotonicity violation) and search resuming past skipped hits.

Verified locally with ./gradlew check on JDK 8: 1832 tests, 0 failures, verifyGoogleJavaFormat + license green. The CLA check on this PR shows green since it was re-run on Sep 11 (I signed it after the failed invocation).

Disclosure (as in #207): the patch and tests were composed agentically with GLM 5.3 assistance; all verification above was run locally by me.

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.

Pattern "\uDC21"` matches inside a surrogate pair; equivalent classes don't

1 participant