diff --git a/deps/Makefile b/deps/Makefile index d83913798..48a812eec 100644 --- a/deps/Makefile +++ b/deps/Makefile @@ -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 diff --git a/deps/postgresql/bind_fmt_text.patch b/deps/postgresql/bind_fmt_text.patch new file mode 100644 index 000000000..de0d9947c --- /dev/null +++ b/deps/postgresql/bind_fmt_text.patch @@ -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) diff --git a/include/PgSQL_Extended_Query_Message.h b/include/PgSQL_Extended_Query_Message.h index 92ddcd5f5..4ab3ae3f6 100644 --- a/include/PgSQL_Extended_Query_Message.h +++ b/include/PgSQL_Extended_Query_Message.h @@ -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 @@ -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(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) { - // 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(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(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(""); // 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--; diff --git a/lib/PgSQL_Connection.cpp b/lib/PgSQL_Connection.cpp index 494d300cc..02c507b4c 100644 --- a/lib/PgSQL_Connection.cpp +++ b/lib/PgSQL_Connection.cpp @@ -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; } diff --git a/lib/PgSQL_Extended_Query_Message.cpp b/lib/PgSQL_Extended_Query_Message.cpp index 6f4a34d6e..375a6a1b0 100644 --- a/lib/PgSQL_Extended_Query_Message.cpp +++ b/lib/PgSQL_Extended_Query_Message.cpp @@ -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; diff --git a/test/tap/tests/pgsql-reg_test_5140_bind_param_fmt_mix-t.cpp b/test/tap/tests/pgsql-reg_test_5140_bind_param_fmt_mix-t.cpp new file mode 100644 index 000000000..98e61a7e9 --- /dev/null +++ b/test/tap/tests/pgsql-reg_test_5140_bind_param_fmt_mix-t.cpp @@ -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 +#include +#include +#include +#include +#include +#include +#include "libpq-fe.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +CommandLine cl; + +using PGConnPtr = std::unique_ptr; + +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(); +}