Skip to content

Use UTF-8 for MySQL JSON binlog value decoding - #39140

Merged
terrymanu merged 2 commits into
apache:masterfrom
thswlsqls:39139
Jul 17, 2026
Merged

Use UTF-8 for MySQL JSON binlog value decoding#39140
terrymanu merged 2 commits into
apache:masterfrom
thswlsqls:39139

Conversation

@thswlsqls

Copy link
Copy Markdown
Contributor

Fixes #39139.

Changes proposed in this pull request:

  • Decode MySQL JSON binlog string values (decodeString) and object keys (decodeKeyEntry) with StandardCharsets.UTF_8 instead of new String(byte[]), which relies on the JVM default charset and corrupts non-ASCII data on non-UTF-8-default JVMs.
  • MySQL JSON is stored as utf8mb4 (already noted in the class Javadoc for the STRING type); this aligns the decoder with MySQLPacketPayload, which always reads bytes with an explicit charset.
  • Add a MySQLJsonValueDecoderTest case decoding a multibyte UTF-8 key and value ({"名前":"café"}) from explicit UTF-8 bytes, asserting the utf8mb4 contract. On a UTF-8-default JVM it acts as a regression/contract guard; it also catches corruption on non-UTF-8-default JVMs.

Before committing this PR, I'm sure that I have checked the following options:

  • My code follows the code of conduct of this project.
  • I have self-reviewed the commit code.
  • I have (or in comment I request) added corresponding labels for the pull request.
  • I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
  • I have made corresponding changes to the documentation.
  • I have added corresponding unit tests for my changes.
  • I have updated the Release Notes of the current development version. For more details, see Update Release Note

terrymanu
terrymanu previously approved these changes Jul 17, 2026

@terrymanu terrymanu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Summary

Review Result: Mergeable

Reason: The root cause is fixed at both byte-to-string conversion sites, and the regression test exercises UTF-8 key and value decoding with correct byte lengths. Scoped verification also passes with a US-ASCII default charset. This is a code-scope result only; CI was not reviewed by request.

Evidence

Review Details

  • Review Focus: Code Correctness Review. CI not reviewed by request.
  • Reviewed Scope: All three GitHub-listed files in the MySQL protocol module and release notes at head a155673ac878bce61a99ec6ba12ba9d32b186dc3; base SHA e249deba59d3583dc5464292672c5c73a92c2832; local merge-base af91b1c34cb041466449eec61ae711ec1f245d92. The local triple-dot file list matched GitHub /pulls/39140/files.
  • Not Reviewed Scope: GitHub Actions/check-runs, full-repository build, E2E scenarios, and unrelated modules.
  • Verification:
    • Focused MySQLJsonValueDecoderTest run: exit 0; 42 tests passed.
    • The same test with -DargLine=-Dfile.encoding=COMPAT: exit 0; 42 tests passed, with the forked JVM reporting file.encoding=US-ASCII.
    • Scoped spotless:check checkstyle:check: exit 0; 0 Spotless changes required and 0 Checkstyle violations.
    • Logs: <system-temp>/shardingsphere-pr39140-{test,compat-test,style}.*.log.
    • GitHub PR, file, commit, comment, and review endpoints were accessible; no public-evidence gap remains.
  • Release Note / User Docs: Release note present and verified. Additional user documentation is not required because the fix changes no configuration, API/SPI, migration, or operational procedure.

@terrymanu
terrymanu merged commit 6b36581 into apache:master Jul 17, 2026
15 checks passed
@thswlsqls
thswlsqls deleted the 39139 branch July 21, 2026 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MySQL JSON binlog string and key values are decoded with the JVM default charset instead of UTF-8

2 participants