Fixes possible api misuse when connecting with server

Started by Ranier Vilela7 months ago1 messages
#1Ranier Vilela
ranier.vf@gmail.com
5 attachment(s)

Hi.

IMO, I think that some sources can have api misuse.

The functions * PQconnectdbParams* and * PQconnectdb*,
can return NULL and need a handler too.
More, if a connection fail with CONNECTION_BAD, some
sources do not handle correctly when failing.
Must call PQfinish, to release OS resources.

Patches attached.

best regards,
Ranier Vilela

Attachments:

fix-connection-api-miuse-connect_utils.patchapplication/octet-stream; name=fix-connection-api-miuse-connect_utils.patchDownload
diff --git a/src/fe_utils/connect_utils.c b/src/fe_utils/connect_utils.c
index cda1c11099..22bd48c925 100644
--- a/src/fe_utils/connect_utils.c
+++ b/src/fe_utils/connect_utils.c
@@ -108,12 +108,13 @@ connectDatabase(const ConnParams *cparams, const char *progname,
 	/* check to see that the backend connection was successfully made */
 	if (PQstatus(conn) == CONNECTION_BAD)
 	{
+		pg_log_error("%s", PQerrorMessage(conn));
+		PQfinish(conn);
+
 		if (fail_ok)
-		{
-			PQfinish(conn);
 			return NULL;
-		}
-		pg_fatal("%s", PQerrorMessage(conn));
+
+		pg_fatal("could not connect to server");
 	}
 
 	/* Start strict; callers may override this. */
fix-connection-api-miuse-ecpg.patchapplication/octet-stream; name=fix-connection-api-miuse-ecpg.patchDownload
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 2bbb70333d..45c85ff2e3 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -642,6 +642,12 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 	ecpg_free(conn_values);
 	ecpg_free(conn_keywords);
 
+	if (!this->connection)
+	{
+		ecpg_log("ECPGconnect: could not connect to server");
+		return false;
+	}
+
 	if (PQstatus(this->connection) == CONNECTION_BAD)
 	{
 		const char *errmsg = PQerrorMessage(this->connection);
fix-connection-api-miuse-pg_rewind.patchapplication/octet-stream; name=fix-connection-api-miuse-pg_rewind.patchDownload
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 9d16c1e6b4..d77a46b3ff 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -307,8 +307,15 @@ main(int argc, char **argv)
 	{
 		conn = PQconnectdb(connstr_source);
 
+		if (conn == NULL)
+			pg_fatal("could not connect to server");
+
 		if (PQstatus(conn) == CONNECTION_BAD)
-			pg_fatal("%s", PQerrorMessage(conn));
+		{
+			pg_log_error("%s", PQerrorMessage(conn));
+			PQfinish(conn);
+			pg_fatal("could not connect to server");
+		}
 
 		if (showprogress)
 			pg_log_info("connected to server");
fix-connection-api-miuse-psql.patchapplication/octet-stream; name=fix-connection-api-miuse-psql.patchDownload
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 249b6aa516..043972a37e 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -278,6 +278,9 @@ main(int argc, char *argv[])
 		free(keywords);
 		free(values);
 
+		if (!pset.db)
+			pg_fatal("could not connect to server");
+
 		if (PQstatus(pset.db) == CONNECTION_BAD &&
 			PQconnectionNeedsPassword(pset.db) &&
 			!password &&
fix-connection-api-miuse-pg_createsubscriber.patchapplication/octet-stream; name=fix-connection-api-miuse-pg_createsubscriber.patchDownload

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index c43c0cbbba..20c571d52a 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -539,6 +539,14 @@ connect_database(const char *conninfo, bool exit_on_error)
 	PGresult   *res;
 
 	conn = PQconnectdb(conninfo);
+	if (conn == NULL)
+	{
+		pg_log_error("could not connect to server");
+
+		if (exit_on_error)
+			exit(1);
+		return NULL;
+	}
 	if (PQstatus(conn) != CONNECTION_OK)
 	{
 		pg_log_error("connection to database failed: %s",