wwbmmm commented on code in PR #3575:
URL: https://github.com/apache/brpc/pull/3575#discussion_r4151748537
##########
src/brpc/policy/mysql/mysql_reply.cpp:
##########
@@ -555,12 +599,14 @@ ParseError
MysqlReply::ResultSetHeader::Parse(butil::IOBuf& buf) {
return PARSE_OK;
}
MysqlHeader header;
- if (!parse_header(buf, &header)) {
+ butil::IOBuf payload;
+ if (!parse_header(buf, &header, &payload)) {
return PARSE_ERROR_NOT_ENOUGH_DATA;
}
- uint64_t old_size, new_size;
- old_size = buf.size();
- _column_count = parse_encode_length(buf);
+ if (!parse_encode_length(payload, &_column_count)) {
+ LOG(ERROR) << "MysqlReply::ResultSetHeader::Parse: truncated column
count";
+ return PARSE_ERROR_ABSOLUTELY_WRONG;
Review Comment:
[replied by brpc-oncall robot] Valid catch, fixed in the latest commit.
Three changes:
1. **Dispatch**: a fresh wire byte is no longer matched against the
synthetic `MysqlRspType` values. Fresh `0x01..0xFE` leading bytes all route to
the result-set parser (the header parser then validates the column count
through the checked `parse_encode_length` path and the 65535 cap), and
prepare-ok dispatch/resume is keyed on `_type == MYSQL_RSP_PREPARE_OK` and
`stmt_type == MYSQL_NEED_PREPARE` instead of the wire byte. So a valid
252..65535-column result set (0xFC form, and 251 via `FC FB 00` since a
compliant server never emits bare 0xFB as a count — it is the length-encoded
NULL marker) now parses correctly, while a truncated `0xFC` count is rejected
by `ResultSetHeader::Parse`.
2. **EOF disambiguation**: per the MySQL protocol, a 0xFE-leading packet is
an EOF only when its payload is < 9 bytes. Both `is_an_eof` (the result-set row
loop) and the dispatcher now apply the length test, so a row whose first field
needs the 8-byte length form is a row, not an EOF.
3. **Unchecked prepare-header reads**: all remaining unchecked fixed-width
cuts in the packet parsers (prepare-ok header, column fixed fields, OK
status/warnings, EOF, ERR sql-state, auth fixed fields and NUL-terminated
strings, binary row/field values, binary TIME/DATETIME) now go through a
`parse_fixed` helper that rejects short reads with
`PARSE_ERROR_ABSOLUTELY_WRONG`, closing the same uninitialized-memory class
everywhere in this file.
New tests: `AcceptMultiByteColumnCount` (0xFC-encoded 256 columns),
`AcceptSingleByte251ColumnCount`, `RejectTruncatedMultiByteColumnCount` (bare
0xFC, previously misparsed as prepare-ok), `RejectHugeColumnCountFePrefix`
(previously misparsed as EOF and returned `PARSE_OK`),
`RejectRowStartingWithFeLenenc`, `AcceptStandaloneEofReply`, `AcceptPrepareOk`,
`RejectTruncatedPrepareOk`. All 19 cases plus the auth/handshake/scramble
suites pass.
--
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]
