aherman
/
proxysql
mirror of https://github.com/sysown/proxysql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4921 from sysown/v3.0_unknown_param_crash_fix_4919

Browse Source 
ProxySQL crashes when client sends unknown connection parameter - v3.0
pull/4931/head
René Cannaò 1 year ago committed by GitHub
parent af2486c7b4 dd589508f4
commit bf73b083e0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 109 additions and 16 deletions
Show Stats Download Patch File Download Diff File

2
include/PgSQL_Backend.h
Unescape View File

@ -12,8 +12,6 @@ class PgSQL_Backend
void * operator new(size_t);
void operator delete(void *);
int hostgroup_id; //< The ID of the host group this connection belongs to. Set to -1 if uninitialized
char gtid_uuid[128]; //< An array to store a unique identifier for each transaction : for now unused
uint64_t gtid_trxid; //< The ID of the current transaction : for now unused
PgSQL_Data_Stream *server_myds;
// mysql_cp_entry_t *server_mycpe;
bytes_stats_t server_bytes_at_cmd; //< A structure storing the number of bytes received and sent

1
include/ProxySQL_Poll.h
Unescape View File

@ -44,6 +44,7 @@ class ProxySQL_Poll {
ProxySQL_Poll();
~ProxySQL_Poll();
void add(uint32_t _events, int _fd, T *_myds, unsigned long long sent_time);
void update_fd_at_index(unsigned int idx, int _fd);
void remove_index_fast(unsigned int i);
int find_index(int fd);
};

9
lib/Base_Thread.cpp
Unescape View File

@ -349,8 +349,13 @@ void Base_Thread::configure_pollout(DS * myds, unsigned int n) {
} else {
if (myds->DSS > STATE_MARIADB_BEGIN && myds->DSS < STATE_MARIADB_END) {
thr->mypolls.fds[n].events = POLLIN;
if (thr->mypolls.myds[n]->myconn->async_exit_status & MYSQL_WAIT_WRITE)
thr->mypolls.fds[n].events |= POLLOUT;
if constexpr (std::is_same_v<T, PgSQL_Thread>) {
if (thr->mypolls.myds[n]->myconn->async_exit_status & PG_EVENT_WRITE)
thr->mypolls.fds[n].events |= POLLOUT;
} else if constexpr (std::is_same_v<T, MySQL_Thread>) {
if (thr->mypolls.myds[n]->myconn->async_exit_status & MYSQL_WAIT_WRITE)
thr->mypolls.fds[n].events |= POLLOUT;
}
} else {
myds->set_pollout();
}

2
lib/PgSQL_Backend.cpp
Unescape View File

@ -15,8 +15,6 @@ PgSQL_Backend::PgSQL_Backend() {
server_myds=NULL;
server_bytes_at_cmd.bytes_recv=0;
server_bytes_at_cmd.bytes_sent=0;
memset(gtid_uuid,0,sizeof(gtid_uuid));
gtid_trxid=0;
}
PgSQL_Backend::~PgSQL_Backend() {

10
lib/PgSQL_Connection.cpp
Unescape View File

@ -884,7 +884,7 @@ void PgSQL_Connection::connect_cont(short event) {
case PGRES_POLLING_WRITING:
async_exit_status = PG_EVENT_WRITE;
break;
case PGRES_POLLING_ACTIVE:
case PGRES_POLLING_ACTIVE: // Not used
case PGRES_POLLING_READING:
async_exit_status = PG_EVENT_READ;
break;
@ -897,7 +897,13 @@ void PgSQL_Connection::connect_cont(short event) {
const PGresult* result = PQgetResultFromPGconn(pgsql_conn);
set_error_from_result(result);
proxy_error("Connect failed. %s\n", get_error_code_with_message().c_str());
return;
}
int current_fd = PQsocket(pgsql_conn);
if (current_fd != fd) {
proxy_warning("PgSQL Connection FD has been changed by PQconnectPoll(). oldFD:%d newFD:%d\n", fd, current_fd);
proxy_debug(PROXY_DEBUG_MYSQL_CONNECTION, 5, "PgSQL Connection FD has been changed by PQconnectPoll()"
"Session=%p, Conn=%p, myds=%p, oldFD=%d, newFD=%d\n", myds->sess, this, myds, fd, current_fd);
fd = current_fd;
}
}

13
lib/PgSQL_Session.cpp
Unescape View File

@ -1744,10 +1744,21 @@ bool PgSQL_Session::handler_again___status_CONNECTING_SERVER(int* _rc) {
if (myds->mypolls == NULL) {
// connection yet not in mypolls
myds->assign_fd_from_mysql_conn();
thread->mypolls.add(POLLIN | POLLOUT, mybe->server_myds->fd, mybe->server_myds, curtime);
thread->mypolls.add(POLLIN | POLLOUT, myds->fd, myds, curtime);
if (mirror) {
PROXY_TRACE();
}
} else {
// See Issue#4919 (https://github.com/sysown/proxysql/issues/4919)
// File descriptor was already set previously. Let's verify if it has changed
if (myds->fd != myconn->fd)
{
// PQconnectPoll has changed the file descriptor (FD) during the connection process.
// We need to update the new FD in mypolls, replacing the old one,
// Note: previous FD is closed by PQconnectPoll
myds->assign_fd_from_mysql_conn();
thread->mypolls.update_fd_at_index(myds->poll_fds_idx, myds->fd);
}
}
switch (rc) {
case 0:

15
lib/ProxySQL_Poll.cpp
Unescape View File

@ -126,6 +126,21 @@ void ProxySQL_Poll<T>::add(uint32_t _events, int _fd, T *_myds, unsigned long lo
len++;
}
/**
* @brief Updates the file descriptor (FD) at a specific index in the ProxySQL_Poll object.
*
* This function updates the file descriptor (FD) at a specific index in the ProxySQL_Poll object.
* It does not modify any other associated data or metadata.
*
* @param idx The index of the file descriptor (FD) to update.
* @param _fd The new file descriptor (FD) value.
*/
template<class T>
void ProxySQL_Poll<T>::update_fd_at_index(unsigned int idx, int _fd) {
if ((int)idx == -1 || idx >= len) return;
fds[idx].fd = _fd;
}
/**
* @brief Removes a file descriptor (FD) and its associated MySQL_Data_Stream from the ProxySQL_Poll object.
*

73
test/tap/tests/pgsql-connection_parameters_test-t.cpp
Unescape View File

@ -620,14 +620,14 @@ bool test_parameters(PGconn* admin_conn, const parameter_test& test) {
snprintf(buffer, sizeof(buffer), "SET %s='%s'", parameter.name.c_str(), parameter.value.c_str());
if (executeQueries(admin_conn, { buffer, "LOAD PGSQL VARIABLES TO RUNTIME" }) == false) {
BAIL_OUT("Error: failed to set admin variable in file %s, line %d", __FILE__, __LINE__);
diag("Error: failed to set admin variable in file %s, line %d", __FILE__, __LINE__);
return false;
}
}
int sock = connect_server(cl.pgsql_host, cl.pgsql_port);
if (sock == -1) {
BAIL_OUT("Error: failed to connect to the server in file %s, line %d", __FILE__, __LINE__);
diag("Error: failed to connect to the server in file %s, line %d", __FILE__, __LINE__);
return false;
}
@ -671,7 +671,7 @@ bool test_parameters(PGconn* admin_conn, const parameter_test& test) {
// Send StartupMessage
if (send_data(sock, startup_msg.data(), startup_msg.size()) == false) {
BAIL_OUT("Error: failed to send startup message in file %s, line %d", __FILE__, __LINE__);
diag("Error: failed to send startup message in file %s, line %d", __FILE__, __LINE__);
goto cleanup;
}
@ -692,7 +692,7 @@ bool test_parameters(PGconn* admin_conn, const parameter_test& test) {
auto result = execute_query(sock, buffer);
if (result == nullptr) {
BAIL_OUT("Error: failed to execute query in file %s, line %d", __FILE__, __LINE__);
diag("Error: failed to execute query in file %s, line %d", __FILE__, __LINE__);
goto cleanup;
}
@ -707,7 +707,7 @@ bool test_parameters(PGconn* admin_conn, const parameter_test& test) {
diag("Executing: %s\n", reset_cmd.c_str());
auto result = execute_query(sock, reset_cmd);
if (result == nullptr) {
BAIL_OUT("Error: failed to reset parameter in file %s, line %d", __FILE__, __LINE__);
diag("Error: failed to reset parameter in file %s, line %d", __FILE__, __LINE__);
goto cleanup;
}
if (result->error.empty() == false) {
@ -722,7 +722,7 @@ bool test_parameters(PGconn* admin_conn, const parameter_test& test) {
diag("Executing: %s\n", show_cmd.c_str());
auto result = execute_query(sock, show_cmd);
if (result == nullptr) {
BAIL_OUT("Error: failed to execute query in file %s, line %d", __FILE__, __LINE__);
diag("Error: failed to execute query in file %s, line %d", __FILE__, __LINE__);
goto cleanup;
}
if (test.expect_failure == false && result->error.empty()) {
@ -859,6 +859,26 @@ std::vector<parameter_test> test_cases = {
}
};
constexpr int MAX_REG_ITERATION_PER_THREAD = 5;
constexpr int MAX_REG_THREAD = 2;
void test_invalid_param_reg_4919_thread() {
auto admin_conn = createNewConnection(ConnType::ADMIN, "", false);
if (!admin_conn || PQstatus(admin_conn.get()) != CONNECTION_OK) {
diag("Error: failed to connect to the database in file %s, line %d", __FILE__, __LINE__);
return;
}
parameter_test invalid_param_test = test_cases.back();
for (int i = 0; i < MAX_REG_ITERATION_PER_THREAD; i++) {
if (test_parameters(admin_conn.get(), invalid_param_test) == false) {
diag("Error: failed to test parameters in file %s, line %d", __FILE__, __LINE__);
return;
}
}
}
int main(int argc, char** argv) {
@ -879,6 +899,29 @@ int main(int argc, char** argv) {
test_count += test_case.conn_params.size() * 2;
}
// Regression test for Issue#4919 (https://github.com/sysown/proxysql/issues/4919)
int test_count_regression = 0;
const auto& test_case = test_cases.back();
if (test_case.expect_failure) {
int case_count = 1;
if (test_case.set_commands.empty() == false)
case_count++;
if (test_case.reset_after)
case_count++;
test_count_regression += test_case.conn_params.size() * case_count;
}
else
test_count_regression += test_case.conn_params.size() * 2;
test_count_regression *= MAX_REG_ITERATION_PER_THREAD * MAX_REG_THREAD;
test_count_regression += 1; // execute "select 1" to check if proxysql is alive
// Regression test for Issue#4919
test_count += test_count_regression;
plan(test_count);
if (cl.getEnv())
@ -900,10 +943,26 @@ int main(int argc, char** argv) {
for (const auto& test_case : test_cases) {
if (test_parameters(admin_conn.get(), test_case) == false) {
BAIL_OUT("Error: failed to test parameters in file %s, line %d", __FILE__, __LINE__);
diag("Error: failed to test parameters in file %s, line %d", __FILE__, __LINE__);
return exit_status();
}
}
// Regression test for Issue#4919 (https://github.com/sysown/proxysql/issues/4919)
std::vector<std::thread> threads;
for (int i = 0; i < MAX_REG_THREAD; ++i) {
threads.emplace_back(test_invalid_param_reg_4919_thread);
}
for (auto& t : threads) {
t.join();
}
auto result = executeQueries(admin_conn.get(), {"SELECT 1"});
ok(result, "ProxySQL should be alive");
// Regression test for Issue#4919
return exit_status();
}

Write Preview
Loading…
Cancel
Save