Use unsigned types where possible

venilnoronha · venilnoronha · commit 9968c6aedad1 · 2019-05-22T16:23:47.000-07:00
This updates variable types from int to uint32_t for the length field
and uint8_t for the sequence ID field in the MySQL proxy filter.

Signed-off-by: Venil Noronha &lt;veniln@vmware.com&gt;
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec.h b/source/extensions/filters/network/mysql_proxy/mysql_codec.h
@@ -99,7 +99,7 @@ class MySQLCodec : public Logger::Loggable<Logger::Id::filter> {
 
   PACKED_STRUCT(struct header_fields {
     uint32_t length_ : 24;
-    uint32_t seq_ : 8;
+    uint8_t seq_ : 8;
   });
 
   union MySQLHeader {
@@ -109,17 +109,17 @@ class MySQLCodec : public Logger::Loggable<Logger::Id::filter> {
 
   virtual ~MySQLCodec() {}
 
-  int decode(Buffer::Instance& data, int seq, int len) {
+  int decode(Buffer::Instance& data, uint8_t seq, uint32_t len) {
     seq_ = seq;
     return parseMessage(data, len);
   }
 
   virtual std::string encode() PURE;
 
 protected:
-  virtual int parseMessage(Buffer::Instance& data, int len) PURE;
+  virtual int parseMessage(Buffer::Instance& data, uint32_t len) PURE;
 
-  int seq_;
+  uint8_t seq_;
 };
 
 } // namespace MySQLProxy
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin.cc b/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin.cc
@@ -44,7 +44,7 @@ bool ClientLogin::isClientSecureConnection() const {
   return extended_client_cap_ & MYSQL_EXT_CL_SECURE_CONNECTION;
 }
 
-int ClientLogin::parseMessage(Buffer::Instance& buffer, int) {
+int ClientLogin::parseMessage(Buffer::Instance& buffer, uint32_t) {
   uint16_t client_cap = 0;
   if (BufferHelper::readUint16(buffer, client_cap) != MYSQL_SUCCESS) {
     ENVOY_LOG(info, "error parsing client_cap in mysql ClientLogin msg");
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin.h b/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin.h
@@ -12,7 +12,7 @@ constexpr int UNSET_BYTES = 23;
 class ClientLogin : public MySQLCodec {
 public:
   // MySQLCodec
-  int parseMessage(Buffer::Instance& buffer, int len) override;
+  int parseMessage(Buffer::Instance& buffer, uint32_t len) override;
   std::string encode() override;
 
   int getClientCap() const { return client_cap_; }
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin_resp.cc b/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin_resp.cc
@@ -20,7 +20,7 @@ void ClientLoginResponse::setServerStatus(uint16_t status) { server_status_ = st
 
 void ClientLoginResponse::setWarnings(uint16_t warnings) { warnings_ = warnings; }
 
-int ClientLoginResponse::parseMessage(Buffer::Instance& buffer, int) {
+int ClientLoginResponse::parseMessage(Buffer::Instance& buffer, uint32_t) {
   uint8_t resp_code = 0;
   if (BufferHelper::readUint8(buffer, resp_code) != MYSQL_SUCCESS) {
     ENVOY_LOG(info, "error parsing response code in mysql Login Ok msg");
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin_resp.h b/source/extensions/filters/network/mysql_proxy/mysql_codec_clogin_resp.h
@@ -13,7 +13,7 @@ namespace MySQLProxy {
 class ClientLoginResponse : public MySQLCodec {
 public:
   // MySQLCodec
-  int parseMessage(Buffer::Instance& buffer, int len) override;
+  int parseMessage(Buffer::Instance& buffer, uint32_t len) override;
   std::string encode() override;
 
   uint8_t getRespCode() const { return resp_code_; }
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_command.cc b/source/extensions/filters/network/mysql_proxy/mysql_codec_command.cc
@@ -20,7 +20,7 @@ void Command::setCmd(Command::Cmd cmd) { cmd_ = cmd; }
 
 void Command::setDb(std::string db) { db_ = db; }
 
-int Command::parseMessage(Buffer::Instance& buffer, int len) {
+int Command::parseMessage(Buffer::Instance& buffer, uint32_t len) {
   Command::Cmd cmd = parseCmd(buffer);
   setCmd(cmd);
   if (cmd == Command::Cmd::COM_NULL) {
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_command.h b/source/extensions/filters/network/mysql_proxy/mysql_codec_command.h
@@ -35,7 +35,7 @@ class Command : public MySQLCodec {
   };
 
   // MySQLCodec
-  int parseMessage(Buffer::Instance&, int len) override;
+  int parseMessage(Buffer::Instance&, uint32_t len) override;
   std::string encode() override;
 
   Cmd parseCmd(Buffer::Instance& data);
@@ -57,7 +57,7 @@ class Command : public MySQLCodec {
 class CommandResponse : public MySQLCodec {
 public:
   // MySQLCodec
-  int parseMessage(Buffer::Instance&, int) override { return MYSQL_SUCCESS; }
+  int parseMessage(Buffer::Instance&, uint32_t) override { return MYSQL_SUCCESS; }
   std::string encode() override { return ""; }
 
   uint16_t getServerStatus() const { return server_status_; }
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_greeting.cc b/source/extensions/filters/network/mysql_proxy/mysql_codec_greeting.cc
@@ -24,7 +24,7 @@ void ServerGreeting::setServerStatus(int server_status) { server_status_ = serve
 
 void ServerGreeting::setExtServerCap(int ext_server_cap) { ext_server_cap_ = ext_server_cap; }
 
-int ServerGreeting::parseMessage(Buffer::Instance& buffer, int) {
+int ServerGreeting::parseMessage(Buffer::Instance& buffer, uint32_t) {
   uint8_t protocol = 0;
   if (BufferHelper::readUint8(buffer, protocol) != MYSQL_SUCCESS) {
     ENVOY_LOG(info, "error parsing protocol in mysql Greeting msg");
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_greeting.h b/source/extensions/filters/network/mysql_proxy/mysql_codec_greeting.h
@@ -11,7 +11,7 @@ namespace MySQLProxy {
 class ServerGreeting : public MySQLCodec {
 public:
   // MySQLCodec
-  int parseMessage(Buffer::Instance& buffer, int len) override;
+  int parseMessage(Buffer::Instance& buffer, uint32_t len) override;
   std::string encode() override;
 
   int getProtocol() const { return protocol_; }
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_switch_resp.cc b/source/extensions/filters/network/mysql_proxy/mysql_codec_switch_resp.cc
@@ -12,7 +12,7 @@ void ClientSwitchResponse::setAuthPluginResp(std::string& auth_plugin_resp) {
   auth_plugin_resp_.assign(auth_plugin_resp);
 }
 
-int ClientSwitchResponse::parseMessage(Buffer::Instance&, int) { return MYSQL_SUCCESS; }
+int ClientSwitchResponse::parseMessage(Buffer::Instance&, uint32_t) { return MYSQL_SUCCESS; }
 
 std::string ClientSwitchResponse::encode() {
   Buffer::InstancePtr buffer(new Buffer::OwnedImpl());
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_codec_switch_resp.h b/source/extensions/filters/network/mysql_proxy/mysql_codec_switch_resp.h
@@ -11,7 +11,7 @@ namespace MySQLProxy {
 class ClientSwitchResponse : public MySQLCodec {
 public:
   // MySQLCodec
-  int parseMessage(Buffer::Instance& buffer, int len) override;
+  int parseMessage(Buffer::Instance& buffer, uint32_t len) override;
   std::string encode() override;
 
   const std::string& getAuthPluginResp() const { return auth_plugin_resp_; }
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_decoder.cc b/source/extensions/filters/network/mysql_proxy/mysql_decoder.cc
@@ -9,7 +9,7 @@ namespace Extensions {
 namespace NetworkFilters {
 namespace MySQLProxy {
 
-void DecoderImpl::parseMessage(Buffer::Instance& message, int seq, int len) {
+void DecoderImpl::parseMessage(Buffer::Instance& message, uint8_t seq, uint32_t len) {
   ENVOY_LOG(trace, "mysql_proxy: parsing message, seq {}, len {}", seq, len);
 
   // Run the MySQL state machine
@@ -135,8 +135,8 @@ void DecoderImpl::parseMessage(Buffer::Instance& message, int seq, int len) {
 bool DecoderImpl::decode(Buffer::Instance& data) {
   ENVOY_LOG(trace, "mysql_proxy: decoding {} bytes", data.length());
 
-  int len = 0;
-  int seq = 0;
+  uint32_t len = 0;
+  uint8_t seq = 0;
   if (BufferHelper::peekHdr(data, len, seq) != MYSQL_SUCCESS) {
     throw EnvoyException("error parsing mysql packet header");
   }
@@ -160,9 +160,9 @@ bool DecoderImpl::decode(Buffer::Instance& data) {
 
   session_.setExpectedSeq(seq + 1);
 
-  const int data_len = data.length();
+  const ssize_t data_len = data.length();
   parseMessage(data, seq, len);
-  const int consumed_len = data_len - data.length();
+  const ssize_t consumed_len = data_len - data.length();
   data.drain(len - consumed_len); // Ensure that the whole message was consumed
 
   ENVOY_LOG(trace, "mysql_proxy: {} bytes remaining in buffer", data.length());
@@ -171,7 +171,7 @@ bool DecoderImpl::decode(Buffer::Instance& data) {
 
 void DecoderImpl::onData(Buffer::Instance& data) {
   // TODO(venilnoronha): handle messages over 16 mb. See
-  // https://dev.mysql.com/doc/dev/mysql-server/8.0.2/page_protocol_basic_packets.html#sect_protocol_basic_packets_sending_mt_16mb.
+  // https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_packets.html#sect_protocol_basic_packets_sending_mt_16mb.
   while (!BufferHelper::endOfBuffer(data) && decode(data)) {
   }
 }
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_decoder.h b/source/extensions/filters/network/mysql_proxy/mysql_decoder.h
@@ -59,7 +59,7 @@ class DecoderImpl : public Decoder, Logger::Loggable<Logger::Id::filter> {
 
 private:
   bool decode(Buffer::Instance& data);
-  void parseMessage(Buffer::Instance& message, int seq, int len);
+  void parseMessage(Buffer::Instance& message, uint8_t seq, uint32_t len);
 
   DecoderCallbacks& callbacks_;
   MySQLSession session_;
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_session.h b/source/extensions/filters/network/mysql_proxy/mysql_session.h
@@ -29,12 +29,12 @@ class MySQLSession : Logger::Loggable<Logger::Id::filter> {
 
   void setState(MySQLSession::State state) { state_ = state; }
   MySQLSession::State getState() { return state_; }
-  int getExpectedSeq() { return expected_seq_; }
-  void setExpectedSeq(int seq) { expected_seq_ = seq; }
+  uint8_t getExpectedSeq() { return expected_seq_; }
+  void setExpectedSeq(uint8_t seq) { expected_seq_ = seq; }
 
 private:
   MySQLSession::State state_{State::MYSQL_INIT};
-  int expected_seq_{0};
+  uint8_t expected_seq_{0};
 };
 
 } // namespace MySQLProxy
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_utils.cc b/source/extensions/filters/network/mysql_proxy/mysql_utils.cc
@@ -21,7 +21,7 @@ void BufferHelper::addUint32(Buffer::Instance& buffer, uint32_t val) {
 
 void BufferHelper::addString(Buffer::Instance& buffer, const std::string& str) { buffer.add(str); }
 
-std::string BufferHelper::encodeHdr(const std::string& cmd_str, int seq) {
+std::string BufferHelper::encodeHdr(const std::string& cmd_str, uint8_t seq) {
   MySQLCodec::MySQLHeader mysqlhdr;
   mysqlhdr.fields_.length_ = cmd_str.length();
   mysqlhdr.fields_.seq_ = seq;
@@ -147,7 +147,7 @@ int BufferHelper::peekUint32(Buffer::Instance& buffer, uint32_t& val) {
 
 void BufferHelper::consumeHdr(Buffer::Instance& buffer) { buffer.drain(sizeof(uint32_t)); }
 
-int BufferHelper::peekHdr(Buffer::Instance& buffer, int& len, int& seq) {
+int BufferHelper::peekHdr(Buffer::Instance& buffer, uint32_t& len, uint8_t& seq) {
   uint32_t val = 0;
   if (peekUint32(buffer, val) != MYSQL_SUCCESS) {
     return MYSQL_FAILURE;
diff --git a/source/extensions/filters/network/mysql_proxy/mysql_utils.h b/source/extensions/filters/network/mysql_proxy/mysql_utils.h
@@ -29,7 +29,7 @@ class BufferHelper : public Logger::Loggable<Logger::Id::filter> {
   static void addUint16(Buffer::Instance& buffer, uint16_t val);
   static void addUint32(Buffer::Instance& buffer, uint32_t val);
   static void addString(Buffer::Instance& buffer, const std::string& str);
-  static std::string encodeHdr(const std::string& cmd_str, int seq);
+  static std::string encodeHdr(const std::string& cmd_str, uint8_t seq);
   static bool endOfBuffer(Buffer::Instance& buffer);
   static int readUint8(Buffer::Instance& buffer, uint8_t& val);
   static int readUint16(Buffer::Instance& buffer, uint16_t& val);
@@ -40,7 +40,7 @@ class BufferHelper : public Logger::Loggable<Logger::Id::filter> {
   static int readStringBySize(Buffer::Instance& buffer, size_t len, std::string& str);
   static int peekUint32(Buffer::Instance& buffer, uint32_t& val);
   static void consumeHdr(Buffer::Instance& buffer);
-  static int peekHdr(Buffer::Instance& buffer, int& len, int& seq);
+  static int peekHdr(Buffer::Instance& buffer, uint32_t& len, uint8_t& seq);
 };
 
 } // namespace MySQLProxy
diff --git a/test/extensions/filters/network/mysql_proxy/mysql_command_test.cc b/test/extensions/filters/network/mysql_proxy/mysql_command_test.cc
@@ -1,3 +1,5 @@
+#include <cstdint>
+
 #include "extensions/filters/network/mysql_proxy/mysql_codec.h"
 #include "extensions/filters/network/mysql_proxy/mysql_codec_clogin.h"
 #include "extensions/filters/network/mysql_proxy/mysql_codec_clogin_resp.h"
@@ -20,8 +22,8 @@ class MySQLCommandTest : public testing::Test, public MySQLTestUtils {
   int encodeQuery(std::string query, hsql::SQLParserResult& result) {
     Command mysql_cmd_encode{};
     Command mysql_cmd_decode{};
-    int seq = 0;
-    int len = 0;
+    uint8_t seq = 0u;
+    uint32_t len = 0u;
     mysql_cmd_encode.setCmd(Command::Cmd::COM_QUERY);
     mysql_cmd_encode.setData(query);
     std::string data = mysql_cmd_encode.encode();
diff --git a/test/extensions/filters/network/mysql_proxy/mysql_test_utils.cc b/test/extensions/filters/network/mysql_proxy/mysql_test_utils.cc
@@ -43,15 +43,15 @@ std::string MySQLTestUtils::encodeClientLogin(uint16_t client_cap, std::string u
   return mysql_msg;
 }
 
-std::string MySQLTestUtils::encodeClientLoginResp(uint8_t srv_resp, int it, int seq_force) {
+std::string MySQLTestUtils::encodeClientLoginResp(uint8_t srv_resp, uint8_t it, uint8_t seq_force) {
   ClientLoginResponse mysql_loginok_encode{};
   mysql_loginok_encode.setRespCode(srv_resp);
   mysql_loginok_encode.setAffectedRows(MYSQL_SM_AFFECTED_ROWS);
   mysql_loginok_encode.setLastInsertId(MYSQL_SM_LAST_ID);
   mysql_loginok_encode.setServerStatus(MYSQL_SM_SERVER_OK);
   mysql_loginok_encode.setWarnings(MYSQL_SM_SERVER_WARNINGS);
   std::string data = mysql_loginok_encode.encode();
-  int seq = CHALLENGE_RESP_SEQ_NUM + 2 * it;
+  uint8_t seq = CHALLENGE_RESP_SEQ_NUM + 2 * it;
   if (seq_force > 0) {
     seq = seq_force;
   }
diff --git a/test/extensions/filters/network/mysql_proxy/mysql_test_utils.h b/test/extensions/filters/network/mysql_proxy/mysql_test_utils.h
@@ -27,7 +27,7 @@ class MySQLTestUtils {
 
   std::string encodeServerGreeting(int protocol);
   std::string encodeClientLogin(uint16_t client_cap, std::string user, uint8_t seq);
-  std::string encodeClientLoginResp(uint8_t srv_resp, int it = 0, int seq_force = 0);
+  std::string encodeClientLoginResp(uint8_t srv_resp, uint8_t it = 0, uint8_t seq_force = 0);
   std::string encodeAuthSwitchResp();
 };
 

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ bool ClientLogin::isClientSecureConnection() const {`
`44`	`44`	`return extended_client_cap_ & MYSQL_EXT_CL_SECURE_CONNECTION;`
`45`	`45`	`}`
`46`	`46`
`47`		`-int ClientLogin::parseMessage(Buffer::Instance& buffer, int) {`
	`47`	`+int ClientLogin::parseMessage(Buffer::Instance& buffer, uint32_t) {`
`48`	`48`	`uint16_t client_cap = 0;`
`49`	`49`	`if (BufferHelper::readUint16(buffer, client_cap) != MYSQL_SUCCESS) {`
`50`	`50`	`ENVOY_LOG(info, "error parsing client_cap in mysql ClientLogin msg");`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ void ClientSwitchResponse::setAuthPluginResp(std::string& auth_plugin_resp) {`
`12`	`12`	`auth_plugin_resp_.assign(auth_plugin_resp);`
`13`	`13`	`}`
`14`	`14`
`15`		`-int ClientSwitchResponse::parseMessage(Buffer::Instance&, int) { return MYSQL_SUCCESS; }`
	`15`	`+int ClientSwitchResponse::parseMessage(Buffer::Instance&, uint32_t) { return MYSQL_SUCCESS; }`
`16`	`16`
`17`	`17`	`std::string ClientSwitchResponse::encode() {`
`18`	`18`	`Buffer::InstancePtr buffer(new Buffer::OwnedImpl());`