wwbmmm commented on code in PR #3575:
URL: https://github.com/apache/brpc/pull/3575#discussion_r4153120075
##########
src/brpc/policy/mysql/mysql_reply.cpp:
##########
@@ -267,25 +319,37 @@ ParseError MysqlReply::ConsumePartialIOBuf(butil::IOBuf&
buf,
MY_PARSE_CHECK(_data.auth->Parse(buf, arena));
return PARSE_OK;
}
+ // A 0xFE-leading packet is an EOF only when it is short (payload < 9
+ // bytes, per the MySQL protocol); a longer 0xFE-leading packet starts a
+ // length-encoded column count and belongs to the result-set branch below.
+ const bool is_eof_packet =
+ (type == 0xFE) && (_type == MYSQL_RSP_EOF || mysql_uint3korr(p) < 9);
if (type == 0x00 && (is_auth || stmt_type != MYSQL_NEED_PREPARE)) {
_type = MYSQL_RSP_OK;
MY_ALLOC_CHECK(my_alloc_check(arena, 1, _data.ok));
MY_PARSE_CHECK(_data.ok->Parse(buf, arena));
*more_results = _data.ok->status() & MYSQL_SERVER_MORE_RESULTS_EXISTS;
- } else if ((type == 0x00 && stmt_type == MYSQL_NEED_PREPARE) || type ==
MYSQL_RSP_PREPARE_OK) {
+ } else if ((type == 0x00 && stmt_type == MYSQL_NEED_PREPARE) ||
+ _type == MYSQL_RSP_PREPARE_OK) {
_type = MYSQL_RSP_PREPARE_OK;
MY_ALLOC_CHECK(my_alloc_check(arena, 1, _data.prepare_ok));
MY_PARSE_CHECK(_data.prepare_ok->Parse(buf, arena));
} else if (type == 0xFF) {
_type = MYSQL_RSP_ERROR;
MY_ALLOC_CHECK(my_alloc_check(arena, 1, _data.error));
MY_PARSE_CHECK(_data.error->Parse(buf, arena));
- } else if (type == 0xFE) {
+ } else if (is_eof_packet) {
_type = MYSQL_RSP_EOF;
MY_ALLOC_CHECK(my_alloc_check(arena, 1, _data.eof));
MY_PARSE_CHECK(_data.eof->Parse(buf));
*more_results = _data.eof->status() & MYSQL_SERVER_MORE_RESULTS_EXISTS;
- } else if (type >= 0x01 && type <= 0xFA) {
+ } else if (type >= 0x01 && type <= 0xFE) {
+ // Any other leading byte is the length-encoded column count of a
+ // result set, including the multi-byte prefixes 0xFB (251) and
+ // 0xFC (252..65535) and a long 0xFE-leading count. These bytes must
Review Comment:
[replied by brpc-oncall robot] Correct, fixed in e3b4cae8. Two changes:
1. The fresh-dispatch result-set range is now `0x01..0xFE` excluding `0xFB`,
so a bare `0xFB` header falls through to the unknown-type rejection
(`PARSE_ERROR_ABSOLUTELY_WRONG`) instead of reaching `ResultSetHeader::Parse`.
This also matches the pre-PR behavior where `0xFB` was outside the result-set
range.
2. `ResultSetHeader::Parse` now additionally rejects an explicitly encoded
zero column count (`FC 00 00` followed by two EOF packets was likewise accepted
as a "valid" zero-column result set) — a MySQL result set always carries at
least one column.
Added `RejectBareFbHeader` (your exact scenario: `0xFB` + two well-formed
EOF packets) and `RejectZeroColumnCount`. All 23 cases in
`brpc_mysql_reply_parse_unittest` plus the auth packet/handshake/scramble
suites pass; the existing `AcceptSingleByte251ColumnCount` test (encoded `FC FB
00`, 251 columns) confirms the compliant encoding still parses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
