Merge pull request #4050 from sysown/v2.x-4023

Fix `java.lang.ArrayIndexOutOfBoundsException` when connecting with MySQL JDBC driver - Closes #4023

lib/MySQL_Protocol.cpp

@@ -533,6 +533,10 @@ bool MySQL_Protocol::generate_pkt_OK(bool send, void **ptr, unsigned int *len, u
 			internal_status |= SERVER_STATUS_NO_BACKSLASH_ESCAPES;
 		}
 	}
+	if (gtid_len == 0) {
+		// Remove 'SERVER_SESSION_STATE_CHANGED', since we don't track this info unless GTID related
+		internal_status &= ~SERVER_SESSION_STATE_CHANGED;
+	}
 	memcpy(_ptr+l, &internal_status, sizeof(uint16_t)); l+=sizeof(uint16_t);
 	memcpy(_ptr+l, &warnings, sizeof(uint16_t)); l+=sizeof(uint16_t);
 	if (msg && strlen(msg)) {
@@ -1249,17 +1253,24 @@ bool MySQL_Protocol::generate_pkt_initial_handshake(bool send, void **ptr, unsig
   uint8_t uint8_charset = ci->nr & 255;
   memcpy(_ptr+l,&uint8_charset, sizeof(uint8_charset)); l+=sizeof(uint8_charset);
   memcpy(_ptr+l,&server_status, sizeof(server_status)); l+=sizeof(server_status);
+	uint32_t extended_capabilities = CLIENT_MULTI_RESULTS | CLIENT_MULTI_STATEMENTS | CLIENT_PS_MULTI_RESULTS |
+		CLIENT_PLUGIN_AUTH | CLIENT_SESSION_TRACKING | CLIENT_REMEMBER_OPTIONS;
 	// we conditionally reply the client specifying in 'server_capabilities' that
 	// 'CLIENT_DEPRECATE_EOF' is available if explicitly enabled by 'mysql-enable_client_deprecate_eof'
 	// variable. This is the first step of ensuring that client connections doesn't
 	// enable 'CLIENT_DEPRECATE_EOF' unless explicitly stated by 'mysql-enable_client_deprecate_eof'.
 	// Second step occurs during client handshake response (process_pkt_handshake_response).
 	if (deprecate_eof_active && mysql_thread___enable_client_deprecate_eof) {
-		memcpy(_ptr+l,"\x8f\x81\x15",3); l+=3;
-	}
-	else {
-		memcpy(_ptr+l,"\x8f\x80\x15",3); l+=3;
+		extended_capabilities |= CLIENT_DEPRECATE_EOF;
 	}
+	// Copy the 'capability_flags_2'
+	uint16_t upper_word = static_cast<uint16_t>(extended_capabilities >> 16);
+	memcpy(_ptr+l, static_cast<void*>(&upper_word), sizeof(upper_word)); l += sizeof(upper_word);
+	// Copy the 'auth_plugin_data_len'. Hardcoded due to 'CLIENT_PLUGIN_AUTH' always enabled and reported
+	// as 'mysql_native_password'.
+	uint8_t auth_plugin_data_len = 21;
+	memcpy(_ptr+l, &auth_plugin_data_len, sizeof(auth_plugin_data_len)); l += sizeof(auth_plugin_data_len);
+
   for (i=0;i<10; i++) { _ptr[l]=0x00; l++; } //filler
   //create_random_string(mypkt->data+l,12,(struct my_rnd_struct *)&rand_st); l+=12;
 //#ifdef MARIADB_BASE_VERSION

test/tap/tests/test_greeting_capabilities-t.cpp

@@ -0,0 +1,134 @@
+/**
+ * @file test_greeting_capabilities-t.cpp
+ * @brief Checks that ProxySQL sends the correct capabilities during handshake.
+ * @details Thist test should also check conditional capabilities enabled by config variables. E.g:
+ *   'CLIENT_DEPRECATE_EOF' when enabled through 'mysql-enable_client_deprecate_eof'.
+ */
+
+#include <cstring>
+#include <string>
+#include <stdio.h>
+#include <utility>
+#include <vector>
+
+#include "mysql.h"
+
+#include "tap.h"
+#include "command_line.h"
+#include "utils.h"
+
+using std::pair;
+using std::string;
+using std::vector;
+
+// By default the following capabilities should be present
+std::vector<uint64_t> def_capabilities {
+	CLIENT_MULTI_RESULTS,
+	CLIENT_MULTI_STATEMENTS,
+	CLIENT_PS_MULTI_RESULTS,
+	CLIENT_PLUGIN_AUTH,
+	CLIENT_SESSION_TRACKING,
+	CLIENT_REMEMBER_OPTIONS
+};
+
+pair<bool,uint64_t> check_server_capabilities(
+	MYSQL* proxy, const vector<uint64_t>& exp_conn_caps, bool present
+) {
+	bool caps_match = true;
+	uint64_t exp_caps = 0;
+
+	for (const uint64_t cap : def_capabilities) {
+		caps_match = proxy->server_capabilities & cap;
+		exp_caps |= cap;
+
+		if (caps_match == false) {
+			diag("Missing expected DEFAULT capability: %ld", cap);
+		}
+	}
+
+	for (const uint64_t exp_cap : exp_conn_caps) {
+		if (present) {
+			caps_match = proxy->server_capabilities & exp_cap;
+			exp_caps |= exp_cap;
+		} else {
+			caps_match = !(proxy->server_capabilities & exp_cap);
+			exp_caps &= ~exp_cap;
+		}
+
+		if (caps_match == false) {
+			diag("Missing expected CONDITIONAL capability: %ld", exp_cap);
+		}
+	}
+
+	return { caps_match, exp_caps };
+}
+
+int test_proxy_capabilites(const CommandLine& cl, MYSQL* admin) {
+	MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=0");
+	MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME");
+
+	MYSQL* proxy = mysql_init(NULL);
+
+	if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy));
+		return EXIT_FAILURE;
+	}
+
+	pair<bool,uint64_t> caps_res { check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, false) };
+	uint64_t ext_caps = (proxy->server_capabilities >> 16) << 16;
+
+	mysql_close(proxy);
+
+	ok(
+		caps_res.first, "ProxySQL greeting should return the expected capabilities - Exp: '%ld', Act: '%ld'",
+		caps_res.second, ext_caps
+	);
+
+	MYSQL_QUERY(admin, "SET mysql-enable_client_deprecate_eof=1");
+	MYSQL_QUERY(admin, "LOAD MYSQL VARIABLES TO RUNTIME");
+
+	proxy = mysql_init(NULL);
+	proxy->options.client_flag |= CLIENT_DEPRECATE_EOF;
+
+	if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy));
+		return EXIT_FAILURE;
+	}
+
+	caps_res = check_server_capabilities(proxy, { CLIENT_DEPRECATE_EOF }, true);
+	ext_caps = (proxy->server_capabilities >> 16) << 16;
+
+	ok(
+		caps_res.first, "ProxySQL greeting should return the expected capabilities - Exp: '%ld', Act: '%ld'",
+		caps_res.second, ext_caps
+	);
+
+	mysql_close(proxy);
+
+	return EXIT_SUCCESS;
+}
+
+int main(int argc, char** argv) {
+	CommandLine cl;
+
+	// TODO: Harcoded for now, this is an initial version of the test.
+	plan(2);
+
+	if (cl.getEnv()) {
+		diag("Failed to get the required environmental variables.");
+		return EXIT_FAILURE;
+	}
+
+	MYSQL* admin = mysql_init(NULL);
+
+	if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin));
+		return EXIT_FAILURE;
+	}
+
+	test_proxy_capabilites(cl, admin);
+
+	mysql_close(admin);
+
+	return exit_status();
+}

test/tap/tests/test_server_sess_status-t.cpp

@@ -0,0 +1,162 @@
+/**
+ * @file test_server_sess_status-t.cpp
+ * @brief Test checking that ProxySQL 'server_status' value is properly updated for different operations.
+ * @details Test should also check that unsupported status like 'SERVER_SESSION_STATE_CHANGED' are never
+ *   reported by ProxySQL.
+ */
+
+#include <cstring>
+#include <string>
+#include <stdio.h>
+#include <utility>
+#include <vector>
+
+#include "mysql.h"
+
+#include "tap.h"
+#include "command_line.h"
+#include "utils.h"
+
+using std::pair;
+using std::string;
+
+int get_user_def_hg(MYSQL* admin, const string& user) {
+	const string sel_q { "SELECT default_hostgroup FROM mysql_users WHERE username='" + user + "'" };
+	if (mysql_query(admin, sel_q.c_str())) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); \
+		return -1;
+	}
+
+	MYSQL_RES* myres = mysql_store_result(admin);
+	MYSQL_ROW myrow = mysql_fetch_row(myres);
+
+	if (myrow && myrow[0]) {
+		int def_hg = std::atoi(myrow[0]);
+		mysql_free_result(myres);
+
+		return def_hg;
+	} else {
+		const string err_msg { "Unexpected empty result received for query: " + sel_q };
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, err_msg.c_str());
+		return -1;
+	}
+}
+
+pair<string,int> get_def_srv_host_port(MYSQL* admin, int hg) {
+	const string sel_q { "SELECT hostname,port FROM mysql_servers WHERE hostgroup_id=" + std::to_string(hg) };
+	int myrc = mysql_query(admin, sel_q.c_str());
+
+	if (myrc) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin));
+		return { "", -1 };
+	} else {
+		MYSQL_RES* myres = mysql_store_result(admin);
+		MYSQL_ROW myrow = mysql_fetch_row(myres);
+
+		if (myrow && myrow[0] && myrow[1]) {
+			string host { myrow[0] };
+			int port { std::atoi(myrow[1]) };
+			mysql_free_result(myres);
+
+			return { host, port };
+		} else {
+			const string err_msg { "Unexpected empty result received for query: '" + sel_q + "'"};
+			fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, err_msg.c_str());
+			return { "", -1 };
+		}
+	}
+}
+
+pair<string,int> get_def_srv_host(MYSQL* admin, const string user) {
+	// Get the server from the default hostgroup
+	int def_hg = get_user_def_hg(admin, user);
+	if (def_hg == -1) {
+		return { "", -1 };
+	}
+
+	return get_def_srv_host_port(admin, def_hg);
+}
+
+int main(int argc, char** argv) {
+	CommandLine cl;
+
+	// TODO: Harcoded for now, this is an initial version of the test.
+	plan(4);
+
+	if (cl.getEnv()) {
+		diag("Failed to get the required environmental variables.");
+		return EXIT_FAILURE;
+	}
+
+	MYSQL* proxy = mysql_init(NULL);
+	MYSQL* mysql = mysql_init(NULL);
+	MYSQL* admin = mysql_init(NULL);
+
+	if (!mysql_real_connect(proxy, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy));
+		return EXIT_FAILURE;
+	}
+
+	if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin));
+		return EXIT_FAILURE;
+	}
+
+	const pair<string,int> srv_host { get_def_srv_host(admin, cl.username) };
+	if (srv_host.first.empty()) {
+		diag("Failed to obtain the target server hostname/port. Aborting further testing");
+		goto cleanup;
+	}
+
+	{
+		if (!mysql_real_connect(mysql, srv_host.first.c_str(), cl.username, cl.password, NULL, srv_host.second, NULL, 0)) {
+			fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxy));
+			goto cleanup;
+		}
+
+		int exp_mysql_srv_st = SERVER_STATUS_AUTOCOMMIT;
+
+		ok(
+			exp_mysql_srv_st == mysql->server_status,
+			"MySQL init server status should match expected - exp: '%d', act:'%d'",
+			exp_mysql_srv_st, mysql->server_status
+		);
+
+		mysql_query(mysql, "SET SESSION session_track_transaction_info=\"CHARACTERISTICS\"");
+		mysql_query(mysql, "START TRANSACTION");
+
+		exp_mysql_srv_st = SERVER_STATUS_AUTOCOMMIT | SERVER_STATUS_IN_TRANS | SERVER_SESSION_STATE_CHANGED;
+
+		ok(
+			exp_mysql_srv_st == mysql->server_status,
+			"MySQL new server status should match expected - exp: '%d', act:'%d'",
+			exp_mysql_srv_st, mysql->server_status
+		);
+
+		// TODO-FIXME: We are setting here '0' as expecting to see 'SERVER_STATUS_AUTOCOMMIT' to be false.
+		// This is a bug that should be addressed, and this test revisited.
+		ok(
+			proxy->server_status == 0,
+			"ProxySQL init server status should match expected - exp: '%d', act:'%d'",
+			0, proxy->server_status
+		);
+
+		mysql_query(proxy, "SET SESSION session_track_transaction_info=\"CHARACTERISTICS\"");
+		mysql_query(proxy, "START TRANSACTION");
+
+		uint32_t exp_proxy_srv_st = SERVER_STATUS_AUTOCOMMIT | SERVER_STATUS_IN_TRANS;
+
+		ok(
+			exp_proxy_srv_st == proxy->server_status,
+			"ProxySQL new server status should match expected - exp: '%d', act:'%d'",
+			exp_proxy_srv_st, proxy->server_status
+		);
+	}
+
+cleanup:
+	mysql_close(proxy);
+	mysql_close(mysql);
+	mysql_close(admin);
+
+	return exit_status();
+}