Allow pg_upgrade to honor libpq environment variables. Add 'local'

checks for PGHOST and PGHOSTADDR.

Author: bmomjian

contrib/pg_upgrade/controldata.c

@@ -11,8 +11,6 @@
 
 #include <ctype.h>
 
-static void putenv2(const char *var, const char *val);
-
 /*
  * get_control_data()
  *
@@ -85,21 +83,21 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	if (getenv("LC_MESSAGES"))
 		lc_messages = pg_strdup(getenv("LC_MESSAGES"));
 
-	putenv2("LC_COLLATE", NULL);
-	putenv2("LC_CTYPE", NULL);
-	putenv2("LC_MONETARY", NULL);
-	putenv2("LC_NUMERIC", NULL);
-	putenv2("LC_TIME", NULL);
-	putenv2("LANG",
+	pg_putenv("LC_COLLATE", NULL);
+	pg_putenv("LC_CTYPE", NULL);
+	pg_putenv("LC_MONETARY", NULL);
+	pg_putenv("LC_NUMERIC", NULL);
+	pg_putenv("LC_TIME", NULL);
+	pg_putenv("LANG",
 #ifndef WIN32
 			NULL);
 #else
 	/* On Windows the default locale cannot be English, so force it */
 			"en");
 #endif
-	putenv2("LANGUAGE", NULL);
-	putenv2("LC_ALL", NULL);
-	putenv2("LC_MESSAGES", "C");
+	pg_putenv("LANGUAGE", NULL);
+	pg_putenv("LC_ALL", NULL);
+	pg_putenv("LC_MESSAGES", "C");
 
 	snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/%s \"%s\"" SYSTEMQUOTE,
 			 cluster->bindir,
@@ -374,15 +372,15 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	/*
 	 * Restore environment variables
 	 */
-	putenv2("LC_COLLATE", lc_collate);
-	putenv2("LC_CTYPE", lc_ctype);
-	putenv2("LC_MONETARY", lc_monetary);
-	putenv2("LC_NUMERIC", lc_numeric);
-	putenv2("LC_TIME", lc_time);
-	putenv2("LANG", lang);
-	putenv2("LANGUAGE", language);
-	putenv2("LC_ALL", lc_all);
-	putenv2("LC_MESSAGES", lc_messages);
+	pg_putenv("LC_COLLATE", lc_collate);
+	pg_putenv("LC_CTYPE", lc_ctype);
+	pg_putenv("LC_MONETARY", lc_monetary);
+	pg_putenv("LC_NUMERIC", lc_numeric);
+	pg_putenv("LC_TIME", lc_time);
+	pg_putenv("LANG", lang);
+	pg_putenv("LANGUAGE", language);
+	pg_putenv("LC_ALL", lc_all);
+	pg_putenv("LC_MESSAGES", lc_messages);
 
 	pg_free(lc_collate);
 	pg_free(lc_ctype);
@@ -529,40 +527,3 @@ rename_old_pg_control(void)
 		pg_log(PG_FATAL, "Unable to rename %s to %s.\n", old_path, new_path);
 	check_ok();
 }
-
-
-/*
- *	putenv2()
- *
- *	This is like putenv(), but takes two arguments.
- *	It also does unsetenv() if val is NULL.
- */
-static void
-putenv2(const char *var, const char *val)
-{
-	if (val)
-	{
-#ifndef WIN32
-		char	   *envstr = (char *) pg_malloc(strlen(var) +
-												strlen(val) + 2);
-
-		sprintf(envstr, "%s=%s", var, val);
-		putenv(envstr);
-
-		/*
-		 * Do not free envstr because it becomes part of the environment on
-		 * some operating systems.	See port/unsetenv.c::unsetenv.
-		 */
-#else
-		SetEnvironmentVariableA(var, val);
-#endif
-	}
-	else
-	{
-#ifndef WIN32
-		unsetenv(var);
-#else
-		SetEnvironmentVariableA(var, "");
-#endif
-	}
-}

contrib/pg_upgrade/option.c

@@ -53,23 +53,24 @@ parseCommandLine(int argc, char *argv[])
 	};
 	int			option;			/* Command line option */
 	int			optindex = 0;	/* used by getopt_long */
-	int			user_id;
+	int			os_user_effective_id;
 
-	if (getenv("PGUSER"))
-	{
-		pg_free(os_info.user);
-		os_info.user = pg_strdup(getenv("PGUSER"));
-	}
+	user_opts.transfer_mode = TRANSFER_MODE_COPY;
 
 	os_info.progname = get_progname(argv[0]);
+
+	/* Process libpq env. variables; load values here for usage() output */
 	old_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT;
 	new_cluster.port = getenv("PGPORT") ? atoi(getenv("PGPORT")) : DEF_PGPORT;
-	/* must save value, getenv()'s pointer is not stable */
 
-	user_opts.transfer_mode = TRANSFER_MODE_COPY;
-
-	/* user lookup and 'root' test must be split because of usage() */
-	user_id = get_user_info(&os_info.user);
+	os_user_effective_id = get_user_info(&os_info.user);
+	/* we override just the database user name;  we got the OS id above */
+	if (getenv("PGUSER"))
+	{
+		pg_free(os_info.user);
+		/* must save value, getenv()'s pointer is not stable */
+		os_info.user = pg_strdup(getenv("PGUSER"));
+	}
 
 	if (argc > 1)
 	{
@@ -86,7 +87,8 @@ parseCommandLine(int argc, char *argv[])
 		}
 	}
 
-	if (user_id == 0)
+	/* Allow help and version to be run as root, so do the test here. */
+	if (os_user_effective_id == 0)
 		pg_log(PG_FATAL, "%s: cannot be run as root\n", os_info.progname);
 
 	getcwd(os_info.cwd, MAXPGPATH);
@@ -96,14 +98,6 @@ parseCommandLine(int argc, char *argv[])
 	{
 		switch (option)
 		{
-			case 'd':
-				old_cluster.pgdata = pg_strdup(optarg);
-				break;
-
-			case 'D':
-				new_cluster.pgdata = pg_strdup(optarg);
-				break;
-
 			case 'b':
 				old_cluster.bindir = pg_strdup(optarg);
 				break;
@@ -116,6 +110,14 @@ parseCommandLine(int argc, char *argv[])
 				user_opts.check = true;
 				break;
 
+			case 'd':
+				old_cluster.pgdata = pg_strdup(optarg);
+				break;
+
+			case 'D':
+				new_cluster.pgdata = pg_strdup(optarg);
+				break;
+
 			case 'g':
 				pg_log(PG_REPORT, "Running in debug mode\n");
 				log_opts.debug = true;
@@ -156,6 +158,11 @@ parseCommandLine(int argc, char *argv[])
 			case 'u':
 				pg_free(os_info.user);
 				os_info.user = pg_strdup(optarg);
+				/*
+				 * Push the user name into the environment so pre-9.1
+				 * pg_ctl/libpq uses it.
+				 */
+				pg_putenv("PGUSER", os_info.user);
 				break;
 
 			case 'v':
@@ -197,14 +204,14 @@ parseCommandLine(int argc, char *argv[])
 	}
 
 	/* Get values from env if not already set */
-	validateDirectoryOption(&old_cluster.pgdata, "OLDDATADIR", "-d",
-							"old cluster data resides");
-	validateDirectoryOption(&new_cluster.pgdata, "NEWDATADIR", "-D",
-							"new cluster data resides");
 	validateDirectoryOption(&old_cluster.bindir, "OLDBINDIR", "-b",
 							"old cluster binaries reside");
 	validateDirectoryOption(&new_cluster.bindir, "NEWBINDIR", "-B",
 							"new cluster binaries reside");
+	validateDirectoryOption(&old_cluster.pgdata, "OLDDATADIR", "-d",
+							"old cluster data resides");
+	validateDirectoryOption(&new_cluster.pgdata, "NEWDATADIR", "-D",
+							"new cluster data resides");
 
 	get_pkglibdirs();
 }

contrib/pg_upgrade/pg_upgrade.c

@@ -149,7 +149,7 @@ setup(char *argv0, bool live_check)
 	 * make sure the user has a clean environment, otherwise, we may confuse
 	 * libpq when we connect to one (or both) of the servers.
 	 */
-	check_for_libpq_envvars();
+	check_pghost_envvar();
 
 	verify_directories();
 

contrib/pg_upgrade/pg_upgrade.h

@@ -361,7 +361,7 @@ PGresult   *executeQueryOrDie(PGconn *conn, const char *fmt,...);
 void		start_postmaster(ClusterInfo *cluster);
 void		stop_postmaster(bool fast);
 uint32		get_major_server_version(ClusterInfo *cluster);
-void		check_for_libpq_envvars(void);
+void		check_pghost_envvar(void);
 
 
 /* util.c */
@@ -378,6 +378,7 @@ void	   *pg_malloc(int size);
 void		pg_free(void *ptr);
 const char *getErrorText(int errNum);
 unsigned int str2uint(const char *str);
+void 		pg_putenv(const char *var, const char *val);
 
 
 /* version.c */

contrib/pg_upgrade/server.c

@@ -145,6 +145,7 @@ start_postmaster(ClusterInfo *cluster)
 	char		cmd[MAXPGPATH];
 	PGconn	   *conn;
 	bool		exit_hook_registered = false;
+	int			pg_ctl_return = 0;
 #ifndef WIN32
 	char		*output_filename = log_opts.filename;
 #else
@@ -183,7 +184,11 @@ start_postmaster(ClusterInfo *cluster)
 				"-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
 			 log_opts.filename);
 
-	exec_prog(true, "%s", cmd);
+	/*
+	 * Don't throw an error right away, let connecting throw the error
+	 * because it might supply a reason for the failure.
+	 */
+	pg_ctl_return = exec_prog(false, "%s", cmd);
 
 	/* Check to see if we can connect to the server; if not, report it. */
 	if ((conn = get_db_conn(cluster, "template1")) == NULL ||
@@ -198,6 +203,11 @@ start_postmaster(ClusterInfo *cluster)
 	}
 	PQfinish(conn);
 
+	/* If the connection didn't fail, fail now */
+	if (pg_ctl_return != 0)
+		pg_log(PG_FATAL, "pg_ctl failed to start the %s server\n",
+				CLUSTER_NAME(cluster));
+	
 	os_info.running_cluster = cluster;
 }
 
@@ -241,50 +251,37 @@ stop_postmaster(bool fast)
 
 
 /*
- * check_for_libpq_envvars()
+ * check_pghost_envvar()
  *
- * tests whether any libpq environment variables are set.
- * Since pg_upgrade connects to both the old and the new server,
- * it is potentially dangerous to have any of these set.
- *
- * If any are found, will log them and cancel.
+ * Tests that PGHOST does not point to a non-local server
  */
 void
-check_for_libpq_envvars(void)
+check_pghost_envvar(void)
 {
 	PQconninfoOption *option;
 	PQconninfoOption *start;
-	bool		found = false;
 
 	/* Get valid libpq env vars from the PQconndefaults function */
 
 	start = PQconndefaults();
 
 	for (option = start; option->keyword != NULL; option++)
 	{
-		if (option->envvar)
+		if (option->envvar && (strcmp(option->envvar, "PGHOST") == 0 ||
+			strcmp(option->envvar, "PGHOSTADDR") == 0))
 		{
-			const char *value;
-
-			/* This allows us to see error messages in the local encoding */
-			if (strcmp(option->envvar, "PGCLIENTENCODING") == 0)
-				continue;
-
-			value = getenv(option->envvar);
-			if (value && strlen(value) > 0)
-			{
-				found = true;
-
-				pg_log(PG_WARNING,
-					   "libpq env var %-20s is currently set to: %s\n", option->envvar, value);
-			}
+			const char *value = getenv(option->envvar);
+
+			if (value && strlen(value) > 0 &&
+				/* check for 'local' host values */
+				(strcmp(value, "localhost") != 0 && strcmp(value, "127.0.0.1") != 0 &&
+				 strcmp(value, "::1") != 0 && value[0] != '/'))
+				pg_log(PG_FATAL,
+					"libpq environment variable %s has a non-local server value: %s\n",
+					option->envvar, value);
 		}
 	}
 
 	/* Free the memory that libpq allocated on our behalf */
 	PQconninfoFree(start);
-
-	if (found)
-		pg_log(PG_FATAL,
-			   "libpq env vars have been found and listed above, please unset them for pg_upgrade\n");
 }

contrib/pg_upgrade/util.c

@@ -244,3 +244,41 @@ str2uint(const char *str)
 {
 	return strtoul(str, NULL, 10);
 }
+
+
+/*
+ *	pg_putenv()
+ *
+ *	This is like putenv(), but takes two arguments.
+ *	It also does unsetenv() if val is NULL.
+ */
+void
+pg_putenv(const char *var, const char *val)
+{
+	if (val)
+	{
+#ifndef WIN32
+		char	   *envstr = (char *) pg_malloc(strlen(var) +
+												strlen(val) + 2);
+
+		sprintf(envstr, "%s=%s", var, val);
+		putenv(envstr);
+
+		/*
+		 * Do not free envstr because it becomes part of the environment on
+		 * some operating systems.	See port/unsetenv.c::unsetenv.
+		 */
+#else
+		SetEnvironmentVariableA(var, val);
+#endif
+	}
+	else
+	{
+#ifndef WIN32
+		unsetenv(var);
+#else
+		SetEnvironmentVariableA(var, "");
+#endif
+	}
+}
+