Merge pull request #5142 from sysown/v3.0_bind_fmt_bug_fix_5140

Fixed: Invalid UTF-8 byte sequence error (0xff) when relaying text parameter followed by null parameter [Extended Query]
pull/5161/head
Keith Brings 6 months ago committed by GitHub
commit aaefac0ca4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

3
deps/Makefile vendored

@ -311,7 +311,8 @@ postgresql/postgresql/src/interfaces/libpq/libpq.a:
cd postgresql/postgresql && patch -p0 < ../get_result_from_pgconn.patch
cd postgresql/postgresql && patch -p0 < ../handle_row_data.patch
cd postgresql/postgresql && patch -p0 < ../fmt_err_msg.patch
#cd postgresql/postgresql && LD_LIBRARY_PATH="$(shell pwd)/libssl/openssl" ./configure --with-ssl=openssl --with-includes="$(shell pwd)/libssl/openssl/include/" --with-libraries="$(shell pwd)/libssl/openssl/" --without-readline --enable-debug CFLAGS="-ggdb -O0 -fno-omit-frame-pointer" CPPFLAGS="-g -O0"
cd postgresql/postgresql && patch -p0 < ../bind_fmt_text.patch
#cd postgresql/postgresql && LD_LIBRARY_PATH="$(SSL_LDIR)" ./configure --with-ssl=openssl --with-includes="$(SSL_IDIR)" --with-libraries="$(SSL_LDIR)" --without-readline --enable-debug CFLAGS="-ggdb -O0 -fno-omit-frame-pointer" CPPFLAGS="-g -O0"
cd postgresql/postgresql && LD_LIBRARY_PATH="$(SSL_LDIR)" ./configure --with-ssl=openssl --with-includes="$(SSL_IDIR)" --with-libraries="$(SSL_LDIR)" --without-readline
cd postgresql/postgresql/src/interfaces/libpq && CC=${CC} CXX=${CXX} ${MAKE} MAKELEVEL=0
#cd postgresql/postgresql && CC=${CC} CXX=${CXX} ${MAKE} -f src/interfaces/libpq/Makefile all

@ -0,0 +1,22 @@
diff --git src/interfaces/libpq/fe-exec.c src/interfaces/libpq/fe-exec.c
index d572bde..99bab2c 100644
--- src/interfaces/libpq/fe-exec.c
+++ src/interfaces/libpq/fe-exec.c
@@ -1855,8 +1855,15 @@ PQsendQueryGuts(PGconn *conn,
}
else
{
- /* text parameter, do not use paramLengths */
- nbytes = strlen(paramValues[i]);
+ /* text parameter */
+ if (paramLengths && paramLengths[i] >= 0)
+ {
+ nbytes = paramLengths[i]; // use supplied length
+ }
+ else
+ {
+ nbytes = strlen(paramValues[i]); // fallback to strlen
+ }
}
if (pqPutInt(nbytes, 4, conn) < 0 ||
pqPutnchar(paramValues[i], nbytes, conn) < 0)

@ -4,6 +4,7 @@
#include "proxysql.h"
#include "cpp.h"
constexpr uint32_t PGSQL_PARAM_NULL = 0xFFFFFFFFu;
/**
* @brief Base class for handling PostgreSQL extended query messages.
@ -105,6 +106,10 @@ struct PgSQL_Param_Value {
* If count is non-zero but the buffer is invalid (malformed packet), this is detected and handled
* during parsing before constructing the reader.
*
* Ownership/lifetime: The reader only keeps raw pointers into the caller-provided buffer and
* does not allocate. Any T that exposes pointers (e.g., PgSQL_Param_Value) references the original
* buffer; ensure it outlives the reader and any consumer that accesses those pointers.
*
* @tparam T The type of field to read (e.g., uint32_t, uint16_t, PgSQL_Param_Value).
*/
template<class T>
@ -136,6 +141,16 @@ public:
*
* For uint32_t and uint16_t, reads the value in big-endian order.
* For PgSQL_Param_Value, reads the length and value pointer, handling NULL values.
*
* PgSQL_Param_Value decoding details:
* - Reads a 4-byte big-endian length (uint32_t).
* * 0xFFFFFFFF => SQL NULL: out->len = -1 and out->value = nullptr.
* * 0x00000000 => empty, non-NULL value: out->len = 0 and out->value is set
* to an empty string.
* * Otherwise => non-empty: out->len = static_cast<int32_t>(len) and out->value points
* to the next len bytes starting at the current parse position.
* - The internal cursor advances by out->len bytes only when out->len > 0.
* - The pointer returned is non-owning and valid only as long as the source buffer is alive.
*/
bool next(T* out) {
if (remaining == 0) return false;
@ -151,19 +166,34 @@ public:
}
current += sizeof(uint16_t);
} else if constexpr (std::is_same_v<T, PgSQL_Param_Value>) {
// Read length (big-endian)
// Read length (big-endian) from the wire
uint32_t len;
if (!get_uint32be(current, &len)) {
return false;
}
current += sizeof(uint32_t);
out->len = (len == 0xFFFFFFFF) ? -1 : static_cast<int32_t>(len);
out->value = (len == 0xFFFFFFFF) ? nullptr : current;
if (len != PGSQL_PARAM_NULL && len > INT32_MAX) {
return false; // Malformed: length does not fit into int32_t
}
out->len = (len == PGSQL_PARAM_NULL) ? -1 : static_cast<int32_t>(len);
// Value pointer semantics:
// - NULL (len == -1): pointer is nullptr.
// - Empty (len == 0): pointer is set to empty string.
// - Non-empty (len > 0): pointer to the next 'len' bytes.
if (out->len == -1) {
out->value = nullptr; // NULL
} else if (out->len == 0) {
out->value = reinterpret_cast<const unsigned char*>(""); // empty string
} else {
out->value = current;
}
// Advance pointer if not NULL
// Advance pointer only for non-empty values
if (out->len > 0) {
current += len;
current += out->len;
}
}
remaining--;

@ -1864,7 +1864,6 @@ void PgSQL_Connection::stmt_execute_start() {
set_error(PGSQL_ERROR_CODES::ERRCODE_INVALID_PARAMETER_VALUE,
"Failed to read param format", false);
return;
return;
}
param_formats[i] = format;
}

@ -379,7 +379,7 @@ bool PgSQL_Bind_Message::parse(PtrSize_t& pkt) {
return false;
}
offset += sizeof(uint32_t);
if (param_value_len != 0xFFFFFFFF) {
if (param_value_len != PGSQL_PARAM_NULL) {
// Ensure the parameter value size is valid
if (offset + param_value_len > pkt_len)
return false;

@ -0,0 +1,143 @@
/**
* @file pgsql-reg_test_5140_bind_param_fmt_mix-t.cpp
* @brief Regression test to check libpq's handling of mixed text and binary parameter.
*
*/
#include <unistd.h>
#include <string>
#include <sstream>
#include <chrono>
#include <thread>
#include <mutex>
#include <condition_variable>
#include "libpq-fe.h"
#include "command_line.h"
#include "tap.h"
#include "utils.h"
CommandLine cl;
using PGConnPtr = std::unique_ptr<PGconn, decltype(&PQfinish)>;
enum ConnType {
ADMIN,
BACKEND
};
PGConnPtr createNewConnection(ConnType conn_type, const std::string& options = "", bool with_ssl = false) {
const char* host = (conn_type == BACKEND) ? cl.pgsql_host : cl.pgsql_admin_host;
int port = (conn_type == BACKEND) ? cl.pgsql_port : cl.pgsql_admin_port;
const char* username = (conn_type == BACKEND) ? cl.pgsql_username : cl.admin_username;
const char* password = (conn_type == BACKEND) ? cl.pgsql_password : cl.admin_password;
std::stringstream ss;
ss << "host=" << host << " port=" << port;
ss << " user=" << username << " password=" << password;
ss << (with_ssl ? " sslmode=require" : " sslmode=disable");
if (options.empty() == false) {
ss << " options='" << options << "'";
}
PGconn* conn = PQconnectdb(ss.str().c_str());
if (PQstatus(conn) != CONNECTION_OK) {
fprintf(stderr, "Connection failed to '%s': %s", (conn_type == BACKEND ? "Backend" : "Admin"), PQerrorMessage(conn));
PQfinish(conn);
return PGConnPtr(nullptr, &PQfinish);
}
return PGConnPtr(conn, &PQfinish);
}
int main(int argc, char** argv) {
if (cl.getEnv())
return exit_status();
plan(4);
auto conn = createNewConnection(BACKEND);
if (!conn) {
diag("connection failed");
return 1;
}
// Ensure we have a test table
PGresult* res = PQexec(conn.get(),
"CREATE TEMP TABLE reg_test_5140 (col1 text,col2 int, col3 text, col4 text)"
);
if (PQresultStatus(res) != PGRES_COMMAND_OK) {
diag("Error: %s", PQerrorMessage(conn.get()));
PQclear(res);
return 1;
}
PQclear(res);
// Prepare a statement with 4 parameters
res = PQprepare(conn.get(), "stmt_test_bind_5140",
"INSERT INTO reg_test_5140 VALUES ($1, $2, $3, $4)", 4, nullptr);
if (PQresultStatus(res) != PGRES_COMMAND_OK) {
diag("Error: %s", PQerrorMessage(conn.get()));
PQclear(res);
return 1;
}
PQclear(res);
// Parameter values:
// col1 = "ABCDEFGHIJKLMN" (length 14)
// col2 = NULL (-1)
// col3 = empty string (length 0)
// col4 = binary data (length 0)
const char* values[4];
int lengths[4];
int formats[4];
values[0] = "ABCDEFGHIJKLMN"; // 14 chars
lengths[0] = 14;
formats[0] = 0; // Text Format
values[1] = nullptr; // NULL
lengths[1] = -1;
formats[1] = 1; // Binary Format
values[2] = ""; // empty string
lengths[2] = 0;
formats[2] = 0; // Text Format
values[3] = ""; // empty binary
lengths[3] = 0;
formats[3] = 1; // Binary Format
res = PQexecPrepared(conn.get(), "stmt_test_bind_5140", 4, values, lengths, formats, 0);
if (PQresultStatus(res) != PGRES_COMMAND_OK) {
diag("Error: %s", PQerrorMessage(conn.get()));
PQclear(res);
return 1;
}
PQclear(res);
// Verify row inserted as expected
res = PQexec(conn.get(), "SELECT col1, col2, col3, col4 FROM reg_test_5140");
if (PQresultStatus(res) != PGRES_TUPLES_OK) {
diag("Error: %s", PQerrorMessage(conn.get()));
PQclear(res);
return 1;
}
char const* c1 = PQgetvalue(res, 0, 0);
bool isnull_c2 = PQgetisnull(res, 0, 1);
char const* c3 = PQgetvalue(res, 0, 2);
bool isnull_c4 = PQgetisnull(res, 0, 3);
int len_c4 = PQgetlength(res, 0, 3);
ok(std::string(c1) == "ABCDEFGHIJKLMN", "col1 length 14 parsed correctly");
ok(isnull_c2, "col2 NULL parsed correctly");
ok(std::string(c3) == "", "col3 empty string parsed correctly");
ok(!isnull_c4 && len_c4 == 0, "col4 empty binary parsed correctly");
PQclear(res);
return exit_status();
}
Loading…
Cancel
Save