You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by re...@apache.org on 2025/12/09 13:35:00 UTC

(cloudberry) branch cbdb-postgres-merge updated: Resolve rebase conflicts in `contrib/dblink`. (#1485)

This is an automated email from the ASF dual-hosted git repository.

reshke pushed a commit to branch cbdb-postgres-merge
in repository https://gitbox.apache.org/repos/asf/cloudberry.git


The following commit(s) were added to refs/heads/cbdb-postgres-merge by this push:
     new 256d8d879c4 Resolve rebase conflicts in `contrib/dblink`. (#1485)
256d8d879c4 is described below

commit 256d8d879c4095571679aaec47bea7650cf4f733
Author: reshke <re...@double.cloud>
AuthorDate: Tue Dec 9 18:34:57 2025 +0500

    Resolve rebase conflicts in `contrib/dblink`. (#1485)
    
    In MPP/Cloudberry we historically have dblink
    behaviour differencies with upstream.
    They were initially introduced 9b75844, as bug-fix.
    Namely, we do not allow non-superuser to create dblink connection
    without explicit password/username. We also force username in
    connection string to match session user, if not specified (see dblink_connstr_has_pw).
    
    Two function declarations changed:
    dblink_connstr_check
    dblink_connstr_has_pw
    
    they now return modified connection string.
---
 contrib/dblink/dblink.c            | 168 +++++++++++++++----------------------
 contrib/dblink/expected/dblink.out |   5 --
 2 files changed, 69 insertions(+), 104 deletions(-)

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index e640113aa22..c9f5ebdcd71 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -112,14 +112,9 @@ static int	get_attnum_pk_pos(int *pkattnums, int pknumatts, int key);
 static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pkattvals);
 static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode);
 static char *generate_relation_name(Relation rel);
-<<<<<<< HEAD
-static char *dblink_connstr_check(const char *connstr);
-static void dblink_security_check(PGconn *conn, remoteConn *rconn);
-=======
-static void dblink_connstr_check(const char *connstr);
-static bool dblink_connstr_has_pw(const char *connstr);
+static char *dblink_connstr_check(char *connstr);
+static char *dblink_connstr_has_pw(const char *connstr);
 static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr);
->>>>>>> REL_16_9
 static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
 							 bool fail, const char *fmt,...) pg_attribute_printf(5, 6);
 static char *get_connect_string(const char *servername);
@@ -201,12 +196,12 @@ dblink_get_conn(char *conname_or_str,
 	}
 	else
 	{
-		const char *connstr;
+		char *connstr;
 
 		connstr = get_connect_string(conname_or_str);
 		if (connstr == NULL)
 			connstr = conname_or_str;
-		dblink_connstr_check(connstr);
+		connstr = dblink_connstr_check(connstr);
 
 		/* OK to make connection */
 		conn = libpqsrv_connect(connstr, PG_WAIT_EXTENSION);
@@ -297,7 +292,6 @@ dblink_connect(PG_FUNCTION_ARGS)
 
 	/* check password in connection string if not superuser */
 	connstr = dblink_connstr_check(connstr);
-	dblink_connstr_check(connstr);
 
 	/* OK to make connection */
 	conn = libpqsrv_connect(connstr, PG_WAIT_EXTENSION);
@@ -2595,17 +2589,6 @@ deleteConnection(const char *name)
 }
 
 /*
-<<<<<<< HEAD
- * For non-superusers, insist that the connstr specify a password.  This
- * prevents a password from being picked up from .pgpass, a service file,
- * the environment, etc.  We don't want the postgres user's passwords
- * to be accessible to non-superusers.
- *
- * For Cloudberry, dblink uses built libpq to construct conninfo, whose user is
- * environment variable PGUSER, which is wrong, modifies this function to add
- * the session's username into connstr.
- *
-=======
  * We need to make sure that the connection made used credentials
  * which were provided by the user, so check what credentials were
  * used to connect and then make sure that they came from the user.
@@ -2617,8 +2600,13 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
 	if (superuser())
 		return;
 
+	/* Difference with upstream, return type is str not bool.
+	* dblink_connstr_has_pw also never returns of missing password
+	*/
+	(void) dblink_connstr_has_pw(connstr);
+
 	/* If password was used to connect, make sure it was one provided */
-	if (PQconnectionUsedPassword(conn) && dblink_connstr_has_pw(connstr))
+	if (PQconnectionUsedPassword(conn))
 		return;
 
 #ifdef ENABLE_GSS
@@ -2645,31 +2633,72 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
  * is using a provided password and not one picked up from the
  * environment.
  */
-static bool
+static char *
 dblink_connstr_has_pw(const char *connstr)
 {
 	PQconninfoOption *options;
 	PQconninfoOption *option;
 	bool		connstr_gives_password = false;
+	bool		username_is_set = false;
+	bool		host_is_set = false;
+	char		*connstr_modified = (char *) connstr;
 
 	options = PQconninfoParse(connstr, NULL);
 	if (options)
 	{
 		for (option = options; option->keyword != NULL; option++)
 		{
+
+			if (strcmp(option->keyword, "host") == 0)
+			{
+				if (option->val != NULL && option->val[0] != '\0')
+				{
+					host_is_set = true;
+				}
+			}
+
+			if (strcmp(option->keyword, "user") == 0)
+			{
+				if (option->val == NULL || option->val[0] == '\0')
+				{
+					char *username = GetUserNameFromId(GetUserId(), false);
+
+					/* 7 is strlen("user= ") + length of '\0' */
+					connstr_modified = palloc0(7 + strlen(username) + strlen(connstr));
+					sprintf(connstr_modified, "user=%s %s", username, connstr);
+				}
+
+				username_is_set = true;
+			}
+
+
 			if (strcmp(option->keyword, "password") == 0)
 			{
 				if (option->val != NULL && option->val[0] != '\0')
 				{
 					connstr_gives_password = true;
-					break;
 				}
 			}
+
+			if (host_is_set && username_is_set && connstr_gives_password)
+				break;
 		}
 		PQconninfoFree(options);
 	}
 
-	return connstr_gives_password;
+	if (!host_is_set)
+		ereport(ERROR,
+				(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+					errmsg("host is required"),
+					errdetail("Non-superusers must provide a host in the connection string.")));
+
+	if (!connstr_gives_password)
+		ereport(ERROR,
+				(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+					errmsg("password is required"),
+					errdetail("Non-superusers must provide a password in the connection string.")));
+
+	return connstr_modified;
 }
 
 /*
@@ -2679,94 +2708,35 @@ dblink_connstr_has_pw(const char *connstr)
  * password or GSSAPI credentials from being picked up from .pgpass, a
  * service file, the environment, etc.  We don't want the postgres user's
  * passwords or Kerberos credentials to be accessible to non-superusers.
->>>>>>> REL_16_9
+ *
+ * For Cloudberry, dblink uses built libpq to construct conninfo, whose user is
+ * environment variable PGUSER, which is wrong, modifies this function to add
+ * the session's username into connstr.
  */
 static char *
-dblink_connstr_check(const char *connstr)
+dblink_connstr_check(char *connstr)
 {
-<<<<<<< HEAD
-	char	*connstr_modified = (char *) connstr;
-
-	if (!superuser())
-	{
-		PQconninfoOption *options;
-		PQconninfoOption *option;
-		bool		connstr_gives_password = false;
-		bool				username_is_set = false;
-		bool				host_is_set = false;
-
-		options = PQconninfoParse(connstr, NULL);
-		if (options)
-		{
-			for (option = options; option->keyword != NULL; option++)
-			{
-				if (strcmp(option->keyword, "host") == 0)
-				{
-					if (option->val != NULL && option->val[0] != '\0')
-					{
-						host_is_set = true;
-					}
-				}
-
-				if (strcmp(option->keyword, "user") == 0)
-				{
-					if (option->val == NULL || option->val[0] == '\0')
-					{
-						char *username = GetUserNameFromId(GetUserId(), false);
-
-						/* 7 is strlen("user= ") + length of '\0' */
-						connstr_modified = palloc0(7 + strlen(username) + strlen(connstr));
-						sprintf(connstr_modified, "user=%s %s", username, connstr);
-					}
-
-					username_is_set = true;
-				}
-
-				if (strcmp(option->keyword, "password") == 0)
-				{
-					if (option->val != NULL && option->val[0] != '\0')
-					{
-						connstr_gives_password = true;
-					}
-				}
-
-				if (host_is_set && username_is_set && connstr_gives_password)
-					break;
-			}
-			PQconninfoFree(options);
-		}
-
-		if (!host_is_set)
-			ereport(ERROR,
-					(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-					 errmsg("host is required"),
-					 errdetail("Non-superusers must provide a host in the connection string.")));
-
-		if (!connstr_gives_password)
-			ereport(ERROR,
-					(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-					 errmsg("password is required"),
-					 errdetail("Non-superusers must provide a password in the connection string.")));
-	}
-
-	return connstr_modified;
-=======
 	if (superuser())
-		return;
+		return connstr;
 
-	if (dblink_connstr_has_pw(connstr))
-		return;
+	/* Difference with upstream, return type is str not bool.
+	* dblink_connstr_has_pw also never returns of missing password
+	*/
+	char *res = dblink_connstr_has_pw(connstr);
 
 #ifdef ENABLE_GSS
 	if (be_gssapi_get_delegation(MyProcPort))
-		return;
+		return res;
 #endif
 
+	/* see `dblink_connstr_has_pw` */
+	return res;
+#if 0
 	ereport(ERROR,
 			(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
 			 errmsg("password or GSSAPI delegated credentials required"),
 			 errdetail("Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials.")));
->>>>>>> REL_16_9
+#endif
 }
 
 /*
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
index 1948c7227d3..115910591f7 100644
--- a/contrib/dblink/expected/dblink.out
+++ b/contrib/dblink/expected/dblink.out
@@ -967,13 +967,8 @@ SET SESSION AUTHORIZATION regress_dblink_user;
 -- should fail
 -- GPDB: We also check for hostname in connection string which is checked first
 SELECT dblink_connect('myconn', 'fdtest');
-<<<<<<< HEAD
 DETAIL:  Non-superusers must provide a host in the connection string.
 ERROR:  host is required
-=======
-ERROR:  password or GSSAPI delegated credentials required
-DETAIL:  Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials.
->>>>>>> REL_16_9
 -- should succeed
 SELECT dblink_connect_u('myconn', 'fdtest');
  dblink_connect_u 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org