Copilot commented on code in PR #3575:
URL: https://github.com/apache/brpc/pull/3575#discussion_r4163354116
##########
src/brpc/policy/mysql/mysql_reply.cpp:
##########
@@ -745,34 +791,43 @@ ParseError MysqlReply::Error::Parse(butil::IOBuf& buf,
butil::Arena* arena) {
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;
}
- buf.pop_front(1); // 0xFF
+ // error message, Null-Terminated string.
+ // payload layout: 0xFF(1) + errcode(2) + '#'(1) + sql_state(5) = 9 bytes;
+ // guard against a malformed short packet to avoid reading past the
+ // packet boundary.
+ if (header.payload_size < 9) {
+ LOG(WARNING) << "MysqlReply::Error::Parse: truncated ERR packet,
payload_size "
+ << header.payload_size << " < 9
(0xFF+errcode+'#'+sql_state)";
+ return PARSE_ERROR_ABSOLUTELY_WRONG;
+ }
+ payload.pop_front(1); // 0xFF
{
uint8_t tmp[2];
- buf.cutn(tmp, sizeof(tmp));
+ MY_PARSE_CHECK(parse_fixed(payload, tmp, sizeof(tmp)));
_errcode = mysql_uint2korr(tmp);
}
- buf.pop_front(1); // '#'
+ {
+ uint8_t sharp = 0;
+ MY_PARSE_CHECK(parse_fixed(payload, &sharp, 1)); // '#'
+ if (sharp != '#') {
+ LOG(WARNING) << "MysqlReply::Error::Parse: expected '#' before
sql_state, got "
+ << sharp;
+ return PARSE_ERROR_ABSOLUTELY_WRONG;
Review Comment:
An initial-handshake ERR can omit '#' and SQLSTATE because
CLIENT_PROTOCOL_41 has not yet been negotiated. For example, `FF 10 04`
followed by `Too many connections` is a valid error 1040 packet, but this new
check rejects its first message byte ('T'). `ConsumePartialIOBuf` routes
authentication ERR packets here too, so the server error is treated as
malformed instead of producing a parsed error reply. Make the layout
conditional on the connection phase, retain the SQLSTATE checks for
protocol-4.1 errors, and add an initial-handshake ERR regression test.
--
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]
