[mysql] Clean-room auth handshake codec (scramble + handshake + packet, Stage 1a of #2093) by rajvarun77 · Pull Request #3310 · apache/brpc

rajvarun77 · 2026-05-25T18:35:25Z

Clean-room MySQL client connection-phase auth handshake (Stage 1a of #2093): the scramble functions (mysql_native_password + caching_sha2_password fast path and the RSA / SSL-cleartext slow paths), the length-encoded / packet-header wire codec, and the HandshakeV10 / HandshakeResponse41 / AuthSwitchRequest / AuthMoreData handshake codec — all written from the public MySQL spec to replace the GPL-flagged mysql_auth_hash.cpp. Pure functions, no Socket wiring yet. Verified by 75 golden-vector unit tests plus integration tests that bring up a real mysqld (the which-then-spawn pattern from brpc_redis_unittest.cpp) and drive live logins end to end: fast-auth, RSA full-auth over plain TCP, cleartext full-auth over SSL, auth-plugin switch, and cache reuse.

Companion to the policy/mysql_auth/ codec landed in the previous commit. Tables every algorithm reference, every upstream MySQL/ MariaDB/Percona unit test (24 total) and integration test (5 from libmariadb), maps each to its brpc equivalent or marks it server-side-only, and lists the scope limits of the auth-codec CL.

rajvarun77 · 2026-05-27T17:31:58Z

Hi @wwbmmm @chenBright @yanglimingcn — process question on integration testing for this codec, before I write more code.

The pure-codec unit tests (59 across scramble / handshake / packet) are in place. For end-to-end auth verification I'd like to add an integration test that drives the full handshake against a real mysqld or mariadbd.

brpc already has a precedent for this in test/brpc_redis_unittest.cpp (L62–L87) — system("which redis-server") at startup, spawn it with a temp datadir if found, otherwise each test does puts("Skipped due to absence of redis-server"); return;. That gives real-protocol coverage on contributor machines and CI runners that have the binary, while staying green on lean environments.

Two questions before I mirror the pattern:

Is the redis-style system()-spawn approach the one you'd recommend for MySQL, or would you rather something different — Docker-based service, mtr-style harness, mock server only, etc.?
Is CI expected to have mysqld/mariadbd available (so the tests will actually run in Apache CI), or should the test always self-skip in CI and only execute locally?

The upstream MariaDB integration tests I'd want to mirror (test_auth256 cache-hit + cache-miss + RSA pubkey paths, test_conc312 mid-handshake auth-switch) are listed at the bottom of the spec gist. Happy to mirror whatever pattern you'd want long-term.

Also flagging — CI runs on this branch are currently in action_required state (workflow-approval gate for outside contributors). Could either of you click Approve and run on the Checks tab when convenient?

rajvarun77 · 2026-05-27T17:50:47Z

Update — pushed 38a01f33 (force-push, rebased on master at 477fa492).

Two changes since the last comment:

TLS shortcut for caching_sha2_password slow path is now supported. Added CachingSha2PasswordCleartext() + a dispatcher CachingSha2PasswordSlowPath(password, salt, server_pubkey_pem, bool is_ssl = false). Default is_ssl=false preserves the existing RSA-OAEP behavior; callers that have confirmed the channel is secure pass is_ssl=true and skip the 3-round-trip RSA path. The codec layer stays pure-functional — Stage-1c's Socket integration just threads Socket::is_ssl() straight into the trailing argument. Full rationale + the libmysqlclient reference (sql-common/client_authentication.cc:770-787) is in the PR description under "TLS shortcut via is_ssl flag".
Test count: 75 (+11 from the previous push) — 5 new in MysqlCachingSha2CleartextTest + 6 new in MysqlCachingSha2SlowPathTest. PR description has the by-name mapping table to all 28 upstream unit tests in mysql/mysql-server plus the 6 client-relevant MariaDB integration tests (deferred).

Asks:

@wwbmmm @chenBright @yanglimingcn — could you click Approve and run on the Checks tab? The previous 17-job suite all passed on bc95c031; the new commit is gated on the outside-contributor approval workflow.
Still open: the integration-test approach question from the previous comment (redis-style system()-spawn of mysqld vs. Docker vs. mtr — whichever pattern you prefer).

PR ready for first-pass review whenever you have bandwidth.

rajvarun77 · 2026-05-29T20:47:11Z

Hi @wwbmmm @chenBright @yanglimingcn — could one of you take a look at this Stage-1a codec and shepherd it through review? @wwbmmm you're the natural fit, but anyone with bandwidth is welcome. Happy to keep the staged split and turn comments around fast.

wwbmmm · 2026-05-30T03:32:03Z

Hi @wwbmmm @chenBright @yanglimingcn — process question on integration testing for this codec, before I write more code.

The pure-codec unit tests (59 across scramble / handshake / packet) are in place. For end-to-end auth verification I'd like to add an integration test that drives the full handshake against a real mysqld or mariadbd.

brpc already has a precedent for this in test/brpc_redis_unittest.cpp (L62–L87) — system("which redis-server") at startup, spawn it with a temp datadir if found, otherwise each test does puts("Skipped due to absence of redis-server"); return;. That gives real-protocol coverage on contributor machines and CI runners that have the binary, while staying green on lean environments.

Two questions before I mirror the pattern:

Is the redis-style system()-spawn approach the one you'd recommend for MySQL, or would you rather something different — Docker-based service, mtr-style harness, mock server only, etc.?

Is CI expected to have mysqld/mariadbd available (so the tests will actually run in Apache CI), or should the test always self-skip in CI and only execute locally?

The upstream MariaDB integration tests I'd want to mirror (test_auth256 cache-hit + cache-miss + RSA pubkey paths, test_conc312 mid-handshake auth-switch) are listed at the bottom of the spec gist. Happy to mirror whatever pattern you'd want long-term.

Also flagging — CI runs on this branch are currently in action_required state (workflow-approval gate for outside contributors). Could either of you click Approve and run on the Checks tab when convenient?

I think you can follow the brpc_redis_unittest.cpp way for mysql: install a mysqld locally, and connect the mysqld server in the test code.
For future improvement, I think we can create a new CI job, install redis and mysqld in the CI server and run the corresponding test cases.

…packet) Picks up the connection-phase authentication layer of MySQL protocol support per the staged plan announced on dev@brpc.apache.org and PR apache#2093. Three modules under src/brpc/policy/mysql_auth/: - mysql_auth_scramble: mysql_native_password (SHA1 XOR), plus caching_sha2_password fast path (SHA256 XOR) and slow path with both branches: - RSA-OAEP via OpenSSL EVP_PKEY_encrypt + PKCS1_OAEP_PADDING - TLS-cleartext (NUL-terminated password) selected at runtime via CachingSha2PasswordSlowPath(..., bool is_ssl = false). Default is_ssl=false preserves RSA behavior for callers not yet threaded with the connection's TLS state. - mysql_auth_packet: length-encoded int/string, 4-byte packet header, NUL-terminated string. - mysql_auth_handshake: HandshakeV10 parse, HandshakeResponse41 build, AuthSwitchRequest parse, AuthMoreData parse. All written clean-room from MySQL's public protocol documentation; no GPL-licensed source was consulted. SHA-256 and RSA paths use OpenSSL EVP -- works under both OpenSSL and BoringSSL. 75 unit tests across three files under test/mysql_auth/: 36 in brpc_mysql_auth_scramble_unittest.cpp 19 in brpc_mysql_auth_handshake_unittest.cpp 20 in brpc_mysql_auth_packet_unittest.cpp Mirrors every client-relevant case from mysql/mysql-server's GPLv2 unittest/gunit/sha2_password-t.cc + sha256_scramble_t.cc with independently re-derived hex vectors. Server-side cases (cache, storage format, *_verification_*) are out of scope for a client library; full mapping table linked from the PR description. Scope limits in this CL: auth codec only. No PROTOCOL_MYSQL registration, no Socket integration, no compressed packets, no prepared statements, no transactions. All land in the follow-up CL. TLS state plumbing in Stage 1c flows the dispatcher's Socket::is_ssl() into the trailing argument here -- no further API change. Replaces the earlier src/brpc/policy/mysql_auth_hash.{h,cpp} + test/brpc_mysql_auth_hash_unittest.cpp; those files are moved into the new mysql_auth/ subdirectory and renamed.

rajvarun77 · 2026-06-01T20:12:48Z

Hi @wwbmmm — this is ready for review whenever you have a moment. Could you please take a look? Happy to adjust anything. Thanks!

Copilot

Pull request overview

This PR introduces a clean-room implementation of the MySQL client connection-phase authentication handshake (scramble algorithms + wire codecs for packet framing and handshake packets), along with extensive unit and server-integration tests to validate behavior against real mysqld instances (self-spawned or user-provided).

Changes:

Added MySQL auth scramble implementations for mysql_native_password and caching_sha2_password (fast path + RSA/cleartext slow paths).
Added MySQL wire-format helpers (packet header + length-encoded int/string + NUL-terminated string) and handshake packet codec (HandshakeV10, HandshakeResponse41, AuthSwitchRequest, AuthMoreData).
Added golden-vector unit tests and end-to-end integration tests (including SSL upgrade and auth-plugin switch), plus build integration for CMake/Bazel test targets.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/mysql/README.md	Documents how to run the new MySQL handshake integration tests in spawned vs. running-server modes.
test/mysql/brpc_mysql_auth_scramble_unittest.cpp	Golden-vector unit tests for scramble algorithms and RSA/cleartext slow paths.
test/mysql/brpc_mysql_auth_packet_unittest.cpp	Unit tests for length-encoded integer/string and packet header codecs.
test/mysql/brpc_mysql_auth_handshake_unittest.cpp	Unit tests for handshake packet codec plus end-to-end login integration tests (plain + SSL + auth-switch + cache reuse).
test/CMakeLists.txt	Ensures MySQL unit tests under `test/mysql/` are built by the CMake test glob.
test/BUILD.bazel	Adds a Bazel `cc_test` target to build/run the MySQL unit tests.
src/brpc/policy/mysql/mysql_auth_scramble.h	Declares clean-room auth scramble APIs/constants.
src/brpc/policy/mysql/mysql_auth_scramble.cpp	Implements the scramble algorithms and RSA/cleartext slow paths.
src/brpc/policy/mysql/mysql_auth_packet.h	Declares MySQL packet framing and basic wire-type codecs.
src/brpc/policy/mysql/mysql_auth_packet.cpp	Implements MySQL packet framing and basic wire-type codecs.
src/brpc/policy/mysql/mysql_auth_handshake.h	Declares handshake packet structs and parse/build functions.
src/brpc/policy/mysql/mysql_auth_handshake.cpp	Implements greeting parsing, response building, and auth continuation packet parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rajvarun77 · 2026-06-04T22:20:48Z

@chenBright @wwbmmm — re-review request for the clean-room auth handshake codec (Stage 1a of #2093): mysql_native_password + caching_sha2_password scrambles, the HandshakeV10 / HandshakeResponse41 codec, and the length-encoded wire helpers — all derived from the public MySQL protocol documentation (no GPL lineage).

The full MySQL client protocol (text protocol + transactions + prepared statements) built on top of this codec is up as #3330.

wwbmmm · 2026-06-05T03:58:58Z

LGTM

chenBright

The test/Makefile is used by CI, so it also needs to be updated, right?

brpc/test/Makefile

Lines 237 to 243 in 72adb6e

    
           brpc_%_unittest:$(TEST_PROTO_OBJS) brpc_%_unittest.o | libbrpc.dbg.$(SOEXT) 
        
           	@echo "> Linking $@" 
        
           ifeq ($(SYSTEM),Linux) 
        
           	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) -Xlinker "-(" $^ -Xlinker "-)" $(STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS) 
        
           else ifeq ($(SYSTEM),Darwin) 
        
           	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) $^ $(GTEST_STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS) 
        
           endif

Address review findings on the Stage-1a auth codec (apache#3310): - DecodeLengthEncodedInt/String: 0xFB is the protocol NULL marker, not an error. Decode it as NULL (1 byte consumed) via a new optional bool* is_null out-param instead of folding it into the failure path, so the shared codec can faithfully parse resultsets containing NULL. Define *out/*is_null on every path and use an overflow-safe bounds check. - BuildHandshakeResponse41: the CLIENT_SECURE_CONNECTION 1-byte length prefix cannot represent an auth_response >255 bytes. Fail hard (return bool false, write nothing, LOG(ERROR)) instead of silently truncating, which produced an invalid response and desynchronized the packet stream. Callers with larger payloads must negotiate CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA. Adds unit tests for the NULL marker (int + string) and for the oversize reject / 255-byte boundary cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Per review (chenBright on apache#3310): the handshake/packet parsers returned false/0 silently on malformed, truncated, or unsupported input, giving no clue why a connection failed. Add a LOG(ERROR) at each failure path naming the function and the concrete cause (truncated <field>, pre-4.1 server, reserved 0xFF marker, length mismatch, etc.). Logic unchanged — only logs inserted; the lenenc NULL (0xFB) success path is intentionally not logged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The length-encoded int/string/header decode failures fire during normal resultset parsing and are not catastrophic, so log them at WARNING rather than ERROR. Handshake-parse failures stay at LOG(ERROR) (a malformed handshake is a fatal connection-setup failure). Severity-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@chenBright

test/Makefile and test/run_tests.sh drive the make-based CI lane (clang-unittest: `cd test && make` then `sh ./run_tests.sh`). Both used non-recursive globs anchored at test/, so the new tests under test/mysql/ were neither compiled nor executed there — only the CMake and Bazel lanes picked them up. The make lane was silently green without running them. - Makefile: add `$(wildcard mysql/brpc_*unittest.cpp)` to TEST_BRPC_SOURCES, plus a `mysql/brpc_%_unittest:` link rule (the existing `brpc_%_unittest:` pattern anchors on a literal `brpc_` prefix and won't match a `mysql/` target). - run_tests.sh: add `mysql/brpc*unittest` to the executed set and enable `nullglob` so a missing subdir expands to nothing instead of a bogus name. Addresses @chenBright's review note that test/Makefile is used by CI and must be updated for the new MySQL tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CI compiles the unittests with -std=c++0x (C++11). The test helper struct AuthCase had a default member initializer (bool use_ssl = false), which makes a class a non-aggregate under C++11 (the rule was only relaxed in C++14). That broke every g_auth_cases.push_back({label,user,password,ssl}) with "no matching member function for call to 'push_back'", failing the clang-unittest and clang-unittest-asan compile-tests step. Drop the default member initializer; all five init sites already pass use_ssl explicitly, so behavior is unchanged and the struct is again an aggregate that accepts braced-init under C++11. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CI compiles unittests with -std=c++0x (C++11), where a class with a default member initializer is not an aggregate, so g_auth_cases.push_back({...}) fails to compile. Drop the default member initializer on AuthCase::use_ssl; all init sites pass it explicitly. Mirrors the same fix on the apache#3310 codec branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The Makefile builds libbrpc by globbing $(d)/* over BRPC_DIRS, and the glob is non-recursive, so src/brpc/policy/mysql/*.cpp was never compiled into the library. The make-based CI lanes (clang-unittest, clang-unittest-asan) therefore failed to link the mysql unittests with "undefined reference to brpc::policy::mysql::*" for every codec symbol. CMake (file(GLOB_RECURSE src/brpc/*.cpp)) and Bazel already pick the directory up, which is why only the make lanes broke once the mysql tests were wired into that lane. Add src/brpc/policy/mysql to BRPC_DIRS so the codec sources land in libbrpc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 · 2026-06-09T03:11:23Z

@wwbmmm @chenBright — CI is green on the latest revision (all 17 jobs), and every review thread on this PR is resolved: the lenenc NULL (0xFB) handling, the oversize auth_response hard-fail, the namespace rename, and the diagnostic failure logs you asked for.

I also confirmed from the job logs that the codec unit tests actually execute (not just compile) in the make-based lanes — brpc_mysql_auth_handshake_unittest (28 tests), brpc_mysql_auth_packet_unittest (27) and brpc_mysql_auth_scramble_unittest all run and pass under both clang-unittest and clang-unittest-asan.

This Stage-1a clean-room MySQL auth codec is ready to merge whenever you are happy with it — could one of you merge it when you get a chance? Thanks again for the reviews.

chenBright

Please move brpc_mysql_*_unittest.cpp to the test folder. No changes are needed to the build script.

rajvarun77 · 2026-06-09T20:24:36Z

@wwbmmm — friendly follow-up: thanks again for the LGTM. Since CI is green and all review threads are resolved, would you mind submitting a formal Approve and merging when you have a moment? Happy to address anything else if needed.

Move the three clean-room auth unittests (handshake, packet, scramble) and their test-plan doc out of test/mysql/ into test/, per review on apache#3310. The existing brpc_*_unittest glob in the Makefile, CMakeLists.txt and BUILD.bazel now picks them up, so all three build scripts revert to master with no mysql-specific additions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Move all seven mysql unittests (auth handshake/packet/scramble, connection_type, pool_concurrency, prepared/txn integration) and the test-plan doc out of test/mysql/ into test/, per review on apache#3310. The existing brpc_*_unittest glob in the Makefile and CMakeLists.txt now picks them up, so both revert to master with no mysql-specific lines. In BUILD.bazel they likewise join the brpc_unittests glob; the five tests that fork a real mysqld keep their ["external", "local"] tags by folding those entries into that target's per_test_tags instead of a separate mysql target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The auth unittests now live in test/ and match the existing brpc*unittest glob in run_tests.sh, so revert the script to master. The removed mysql/brpc*unittest entry needed `shopt -s nullglob` to skip a missing subdir, but CI runs the script under `sh` (dash), where shopt is absent; nullglob stayed off and the now-empty glob was executed as a literal path, exiting 127. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 · 2026-06-11T00:39:09Z

@chenBright Done — moved the MySQL auth unittests out of test/mysql/ into test/. As you noted, no build-script changes were needed: the existing brpc_*_unittest glob in test/Makefile, test/CMakeLists.txt, and test/BUILD.bazel picks them up automatically, and test/run_tests.sh is back to its master version as well.

CI is green — all three auth unittests run and pass in the make/cmake unittest jobs:

brpc_mysql_auth_handshake_unittest — 28 passed
brpc_mysql_auth_packet_unittest — 27 passed
brpc_mysql_auth_scramble_unittest — 35 passed

Thanks for the review!

chenBright

LGTM

…ements) on clean-room auth (#3330) * feat(mysql): clean-room MySQL authentication codec Clean-room implementation of the MySQL connection-phase authentication handshake, derived from the public MySQL protocol documentation with no GPL lineage: mysql_native_password and caching_sha2_password scrambles, HandshakeV10/ HandshakeResponse41 codec, and length-encoded integer/string plus packet-header wire helpers. Handles the lenenc NULL (0xFB) marker and rejects an oversize auth_response. * feat(mysql): full MySQL text protocol with transactions and prepared statements Port the MySQL protocol client (issue #2093) onto the clean-room auth codec and protobuf 3.21 (NonreflectableMessage): COM_QUERY text protocol, interactive transactions via connection affinity, and prepared statements. Wire caching_sha2_password (fast-auth, full-auth RSA, and secure-transport cleartext) into the live client. Fix the lenenc 9-byte length marker in pack_encode_length (0xFD -> 0xFE) per the MySQL protocol spec. * test(mysql): clean-room integration tests + prepared-stmt error fix + Controller cleanup - Add clean-room integration tests (transactions, prepared statements, pooled connection concurrency, connection-type) run against a self-spawned mysqld. - Fix: a failed COM_STMT_PREPARE now returns the ERR packet to the caller and keeps the connection alive, instead of closing the socket. - Warn when a prepared statement runs on a 'short' connection (re-prepares on every execute; prefer 'pooled'). - Replace Controller's mysql-specific _mysql_stmt with a generic opaque per-RPC slot so no protocol type leaks onto the shared Controller. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mysql): consolidate sources under policy/mysql/; drop inherited images Move all MySQL sources (mysql.*, mysql_command/reply/common/transaction/ statement*, mysql_protocol.*, mysql_authenticator.*) into src/brpc/policy/mysql/ alongside the clean-room auth codec; update all includes, build globs, and install rules. Remove three benchmark images inherited from the #2093 port and the doc section referencing them. No behavior change: full build green; all 19 mysql unit/integration tests pass; a 30-case standalone end-to-end run was independently verified against mysqld. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mysql): address code-review findings (binary protocol, auth, edge cases) + ASF headers - Binary DATETIME/TIME: gate the microsecond bytes on the packet length, not the column's declared decimals (over-read / result-set desync). - COM_STMT_SEND_LONG_DATA: frame stmt_id/param_id inside the packet; fix chunk offset. - COM_STMT_EXECUTE: emit the trailing 0-length packet for 16MiB-aligned payloads. - OK/EOF status & warnings: decode via mysql_uint2korr (big-endian safe). - Row NULL-bitmap: arena-allocate instead of a stack VLA; cap column_count. - Auth: bounds-check the parsed auth string; size-bound StringPiece uses. - Prepared stmt: prune stale per-socket stmt_id map entries; count only real '?' placeholders (skip quotes/comments). - MysqlResponse::Clear and MysqlRequest copy/Swap: reset/copy all members. - Controller::ResetPods: release _bind_sock on controller reuse. - Standardize mysql file license headers to the ASF form; clarify auth comment. All 19 unit/integration tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mysql): address review findings — license header, flag typo, header-global, doc/comment drift - example/mysql_c++/mysql_go_press.go: add ASF Apache-2.0 license header (the last file failing the License Check; other 6 already had headers). - mysql_common.{h,cpp}: move the MysqlCollations map definition out of the header into the .cpp behind an `extern` declaration, so each translation unit no longer gets its own copy (C++11-safe; avoids the header-defined global flagged in review). - mysql_statement.{cpp,inl.h}: rename the misspelled gflag mysql_statment_map_size -> mysql_statement_map_size (user-facing name). - docs/cn/mysql_client.md: prepared statements ARE supported now — drop the stale "不支持Prepared statement". - brpc_mysql_connection_type_unittest.cpp: rewrite the stale header comment that described a removed "MustError" test; the prepared-statement path now transparently re-prepares under CONNECTION_TYPE_SHORT and succeeds, matching the actual PreparedStatementUnderShortRePreparesAndSucceeds test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mysql): use the standard ASF license header on new example/test files The new MySQL example sources and brpc_mysql_unittest.cpp carried the old "Copyright (c) Baidu, Inc." Apache-2.0 header, which skywalking-eyes (the repo's License Check, configured copyright-owner = Apache Software Foundation) does not accept — so all 7 files failed the gate. Replace with the canonical ASF header used by the other 549 sources in the tree, and drop the stale Baidu copyright/date attribution lines. Verified locally with `license-eye -c .licenserc.yaml header check`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(mysql): add diagnostic error logs on silent failure paths Extend the failure-path logging from the auth codec to the full client protocol. Parsers and request/response handlers returned false/0/nullptr silently on malformed wire data, truncated packets, bad state, or missing prepared statements, giving no clue why a query/connection failed. Add a LOG(ERROR) at each silent failure path naming the function and the concrete cause. - mysql_auth_handshake/packet.cpp: handshake + lenenc codec failure paths (truncated <field>, pre-4.1 server, reserved 0xFF marker, length mismatch). - mysql.cpp: request command/param guards. - mysql_reply.cpp: result-set parse (column/row/error packet truncation, arena alloc failure, bad binary-row header). - mysql_protocol.cpp: serialize/process type + serialization failures. - mysql_statement.cpp: statement-id lookup misses (not-prepared / stale conn). Logic unchanged — logs inserted only. Normal control-flow returns (need-more-data, lenenc NULL 0xFB, short-connection no-cache) are not logged. mysql_authenticator.cpp and mysql_transaction.cpp already logged every failure path and were left unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(mysql): demote non-fatal diagnostic logs to LOG(WARNING) Tune the severity of the failure-path logs. LOG(ERROR) is reserved for total failures — handshake/connection corruption, authentication failure, out-of-memory, and fundamental request/response type or serialization misuse. Operational and statement-data failures are demoted to LOG(WARNING): - mysql_auth_packet.cpp: lenenc int/string/header decode failures (these fire during normal resultset parsing). - mysql.cpp: request command/param API guards. - mysql_reply.cpp: column/row/field/ERR-packet truncation (statement data). The arena out-of-memory and Auth::Parse auth-plugin failures stay ERROR. - mysql_statement.cpp: prepared-statement-id lookup misses (recoverable, trigger a re-prepare). handshake parsing and the protocol serialize/process type checks remain LOG(ERROR). Severity-only change; messages and logic unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): make AuthCase a C++11 aggregate so push_back{...} compiles CI compiles unittests with -std=c++0x (C++11), where a class with a default member initializer is not an aggregate, so g_auth_cases.push_back({...}) fails to compile. Drop the default member initializer on AuthCase::use_ssl; all init sites pass it explicitly. Mirrors the same fix on the #3310 codec branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mysql,ci): run integration tests against a real CI-provisioned mysqld The MySQL integration tests were not exercising a live server in CI: - Merge master to pick up #3323, which installs mysql-server in the install-essential-dependencies action used by the unittest lanes, so the mysqld binary is present for the self-spawning test/mysql/* suites. - Delete the legacy test/brpc_mysql_unittest.cpp: it hardcoded an external host (db4free.net) with embedded credentials and asserted on connect failure (no skip), so it timed out and hard-failed in CI. The test/mysql/*_integration suites supersede it, spawning a throwaway local mysqld and GTEST_SKIP-ing when none is available (the redis precedent). - Build the test/mysql/* suites in the make lane: extend test/Makefile's source glob and add a mysql/-prefixed link rule; add the subdir to run_tests.sh with nullglob so it runs them. - Bazel: build one cc_test per mysql test file via generate_unittests instead of globbing them all into a single brpc_mysql_test target, which duplicated main() and the FLAGS_mysql_* definitions (ld: duplicate symbol). Drop the per-file main()s so every suite relies on gtest_main, matching brpc_redis_unittest; tag the server-spawning suites external+local so bazel runs them unsandboxed with the real mysqld. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * address review comments: bind-sock flag, non-copyable response, naming - Controller: store the mysql-transaction BindSockAction in two bits of _flags (FLAGS_BIND_SOCK_RESERVE / FLAGS_BIND_SOCK_USE) via set_bind_sock_action()/bind_sock_action(), instead of a dedicated member, per review. No behavior change. - MysqlResponse: make it explicitly non-copyable (= delete copy ctor and assignment) and turn the previously no-op MergeFrom into a hard CHECK-failure, so an accidental copy/CopyFrom is caught instead of silently dropping parsed replies. - MysqlRequest: rename the trivial getters get_tx()/get_stmt() to tx()/stmt() per review. - ControllerPrivateAccessor: add set_mysql_statement_type() as a clearly named alias over the pipelined_count slot the mysql protocol reuses, and call it from the mysql protocol instead of set_pipelined_count() directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mysql): move unittests from mysql/ subdir into test/ Move all seven mysql unittests (auth handshake/packet/scramble, connection_type, pool_concurrency, prepared/txn integration) and the test-plan doc out of test/mysql/ into test/, per review on #3310. The existing brpc_*_unittest glob in the Makefile and CMakeLists.txt now picks them up, so both revert to master with no mysql-specific lines. In BUILD.bazel they likewise join the brpc_unittests glob; the five tests that fork a real mysqld keep their ["external", "local"] tags by folding those entries into that target's per_test_tags instead of a separate mysql target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mysql): drop run_tests.sh mysql/ subdir entry after move The mysql unittests now live in test/ and match the existing brpc*unittest glob in run_tests.sh, so revert the script to master. The removed mysql/brpc*unittest entry needed `shopt -s nullglob` to skip a missing subdir, but CI runs the script under `sh` (dash), where shopt is absent; nullglob stayed off and the now-empty glob was executed as a literal path, exiting 127. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix backup/retry call hang: default-initialize bind_sock_action The new bind_sock_action member of Controller::Call had no in-class default, and the Call(Call*) copy constructor used for backup requests and retries did not initialize it in its member-init list. The backup call therefore read an indeterminate value in Controller::Call::OnComplete, which branches on it before the normal pool-return / SetFailed path; when the garbage matched BIND_SOCK_RESERVE/BIND_SOCK_USE the backup call's socket was diverted to _bind_sock or held instead of returned, leaving the in-flight RPC unconcluded and hanging ChannelTest.backup_request until the test timeout (nondeterministic, hence flaky-looking). Give the member an in-class default initializer (BIND_SOCK_NONE) so every Call construction path inherits it; a backup/retry never inherits transaction connection-affinity. This closes the whole class of "new init path forgets the member" rather than the single copy-ctor instance. * Initialize Call::bind_sock_action explicitly in every constructor Per review: drop the in-class default initializer for Call::bind_sock_action and set it explicitly in each Call constructor (the copy ctor used for backup/retry) and Call::Reset(). Leaving it uninitialized was the cause of the backup/retry-request hang; explicit per-constructor init keeps every path safe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: remove redundant comments and dead code, move _bind_sock.reset() to ResetNonPods() * Remove commented-out code in mysql_stmt example --------- Co-authored-by: rajvarun77 <287367605+rajvarun77@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from bc95c03 to d8752f1 Compare May 27, 2026 17:06

rajvarun77 changed the title ~~[DRAFT][mysql] Clean-room mysql_native_password scramble (Stage 1a of #2093 takeover)~~ [DRAFT][mysql] Clean-room auth handshake codec (scramble + handshake + packet, Stage 1a of #2093) May 27, 2026

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from 9a44a6d to d8752f1 Compare May 27, 2026 17:30

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from d8752f1 to 38a01f3 Compare May 27, 2026 17:49

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from 38a01f3 to a06f2b9 Compare May 29, 2026 20:42

wwbmmm reviewed May 30, 2026

View reviewed changes

Comment thread src/brpc/policy/mysql_auth/mysql_auth_handshake.h Outdated

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch 2 times, most recently from 8370692 to ae7580e Compare May 31, 2026 19:38

rajvarun77 mentioned this pull request May 31, 2026

[DRAFT][ci] Install mysql-server for MySQL handshake integration tests #3318

Closed

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from ae7580e to 6254f7a Compare June 1, 2026 20:09

rajvarun77 marked this pull request as ready for review June 1, 2026 20:11

This was referenced Jun 1, 2026

ci(linux): bring up redis-server and mysql-server for unittests rajvarun77/brpc#1

Open

ci(linux): bring up redis-server and mysql-server for unittests #3323

Merged

chenBright requested a review from Copilot June 2, 2026 02:21

Copilot started reviewing on behalf of chenBright June 2, 2026 02:22 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

Comment thread src/brpc/policy/mysql/mysql_auth_packet.cpp

Comment thread src/brpc/policy/mysql/mysql_auth_packet.h Outdated

Comment thread src/brpc/policy/mysql/mysql_auth_handshake.cpp Outdated

Comment thread src/brpc/policy/mysql/mysql_auth_packet.h Outdated

rajvarun77 mentioned this pull request Jun 4, 2026

Add MySQL client protocol (text protocol, transactions, prepared statements) on clean-room auth (#2093) #3330

Merged

chenBright reviewed Jun 6, 2026

View reviewed changes

Comment thread src/brpc/policy/mysql/mysql_auth_handshake.cpp

rajvarun77 and others added 3 commits June 8, 2026 20:54

rajvarun77 and others added 2 commits June 8, 2026 21:37

chenBright reviewed Jun 9, 2026

View reviewed changes

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from 9568c00 to 4a187e2 Compare June 10, 2026 23:44

chenBright approved these changes Jun 11, 2026

View reviewed changes

chenBright changed the title ~~[DRAFT][mysql] Clean-room auth handshake codec (scramble + handshake + packet, Stage 1a of #2093)~~ [mysql] Clean-room auth handshake codec (scramble + handshake + packet, Stage 1a of #2093) Jun 11, 2026

chenBright merged commit 77e2907 into apache:master Jun 11, 2026
15 checks passed

	brpc_%_unittest:$(TEST_PROTO_OBJS) brpc_%_unittest.o \| libbrpc.dbg.$(SOEXT)
	@echo "> Linking $@"
	ifeq ($(SYSTEM),Linux)
	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) -Xlinker "-(" $^ -Xlinker "-)" $(STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS)
	else ifeq ($(SYSTEM),Darwin)
	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) $^ $(GTEST_STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS)
	endif

Uh oh!

Conversation

rajvarun77 commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajvarun77 commented May 27, 2026

Uh oh!

rajvarun77 commented May 27, 2026

Uh oh!

rajvarun77 commented May 29, 2026

Uh oh!

wwbmmm commented May 30, 2026

Uh oh!

Uh oh!

rajvarun77 commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajvarun77 commented Jun 4, 2026

Uh oh!

wwbmmm commented Jun 5, 2026

Uh oh!

chenBright left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rajvarun77 commented Jun 9, 2026

Uh oh!

chenBright left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajvarun77 commented Jun 9, 2026

Uh oh!

rajvarun77 commented Jun 11, 2026

Uh oh!

chenBright left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rajvarun77 commented May 25, 2026 •

edited

Loading

chenBright left a comment •

edited

Loading