[glom/glom-1-18] PostgreSQL backend: Use g_shell_quote().

[Murray Cumming <murrayc src gnome org>]

commit c56e16c67b5fa5174a09c205ba2b73471e415aa2
Author: Murray Cumming <murrayc murrayc com>
Date:   Wed Nov 9 21:51:39 2011 +0100

    PostgreSQL backend: Use g_shell_quote().
    
    * glom/libglom/connectionpool_backends/postgres.[h|cc]:
    get_path_to_postgres_executable(): Return the already-quoted path
    to avoid the need for manual quoting by the caller.
    However, make this optional (but the default) so that the check in
    * main.cc: can still work.
    * glom/libglom/connectionpool_backends/postgres_self.cc: Use
    Glib::shell_quote() instead of manually adding quote characters.
    Hopefully this also escapes special characters, though we should try to
    test this somehow.

 ChangeLog                                          |   14 ++++++++
 glom/libglom/connectionpool_backends/postgres.cc   |   35 +++++++++++++-------
 glom/libglom/connectionpool_backends/postgres.h    |    4 ++-
 .../connectionpool_backends/postgres_self.cc       |   25 ++++++++------
 glom/main.cc                                       |    2 +-
 5 files changed, 56 insertions(+), 24 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 1c32bd4..5b94fb4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2011-11-09  Murray Cumming  <murrayc murrayc com>
+
+	PostgreSQL backend: Use g_shell_quote().
+
+	* glom/libglom/connectionpool_backends/postgres.[h|cc]:
+	get_path_to_postgres_executable(): Return the already-quoted path
+	to avoid the need for manual quoting by the caller.
+	However, make this optional (but the default) so that the check in 
+	* main.cc: can still work.
+	* glom/libglom/connectionpool_backends/postgres_self.cc: Use 
+	Glib::shell_quote() instead of manually adding quote characters.
+	Hopefully this also escapes special characters, though we should try to 
+	test this somehow.
+
 2011-11-04  Murray Cumming  <murrayc murrayc com>>
 
 	Creating from examples: Do not create users and groups with SQLite.
diff --git a/glom/libglom/connectionpool_backends/postgres.cc b/glom/libglom/connectionpool_backends/postgres.cc
index bd5d787..0eb537a 100644
--- a/glom/libglom/connectionpool_backends/postgres.cc
+++ b/glom/libglom/connectionpool_backends/postgres.cc
@@ -28,6 +28,7 @@
 #include <giomm/file.h>
 #include <glibmm/convert.h>
 #include <glibmm/miscutils.h>
+#include <glibmm/shell.h>
 #include <glib/gstdio.h> /* For g_rename(). TODO: Wrap this in glibmm? */
 #include <glibmm/i18n.h>
 
@@ -496,7 +497,7 @@ bool Postgres::check_postgres_gda_client_is_available()
   return false;
 }
 
-std::string Postgres::get_path_to_postgres_executable(const std::string& program)
+std::string Postgres::get_path_to_postgres_executable(const std::string& program, bool quoted)
 {
 #ifdef G_OS_WIN32
   // Add the .exe extension on Windows:
@@ -525,15 +526,25 @@ std::string Postgres::get_path_to_postgres_executable(const std::string& program
   g_free(installation_directory);
 
   if(Glib::file_test(test, Glib::FILE_TEST_IS_EXECUTABLE))
+  {
+    if(quoted)
+      test = Glib::shell_quote(path);
     return test;
+  }
 
   // Look in PATH otherwise
-  return Glib::find_program_in_path(real_program);
+  std::string path = Glib::find_program_in_path(real_program);
+  if(quoted)
+    path = Glib::shell_quote(path);
+  return path;
 #else // G_OS_WIN32
   // POSTGRES_UTILS_PATH is defined in config.h, based on the configure.
   try
   {
-    return Glib::build_filename(POSTGRES_UTILS_PATH, program + EXEEXT);
+    std::string path = Glib::build_filename(POSTGRES_UTILS_PATH, program + EXEEXT);
+    if(quoted)
+      path = Glib::shell_quote(path);
+    return path;
   }
   catch(const Glib::Error& ex)
   {
@@ -662,13 +673,13 @@ bool Postgres::save_backup(const SlotProgress& slot_progress, const Glib::ustrin
 
   // Make sure to use double quotes for the executable path, because the
   // CreateProcess() API used on Windows does not support single quotes.
-  const std::string command_dump = "\"" + get_path_to_postgres_executable("pg_dump") + "\"" +
+  const std::string command_dump = get_path_to_postgres_executable("pg_dump") +
     " --format=c " + // The default (plain) format cannot be used with pg_restore.
-    " --create --file=\"" + path_backup + "\"" +
-    " --host=\"" + m_host + "\"" +
+    " --create --file=" + Glib::shell_quote(path_backup) +
+    " --host=" + Glib::shell_quote(m_host) +
     " --port=" + port_as_string(m_port) +
-    " --username=\"" + username + "\"" +
-    " " + database_name;
+    " --username=" + Glib::shell_quote(username) +
+    " " + database_name; //TODO: Quote database_name?
 
 
   //std::cout << "DEBUG: command_dump=" << command_dump << std::endl;
@@ -746,11 +757,11 @@ bool Postgres::convert_backup(const SlotProgress& slot_progress, const std::stri
 
   // Make sure to use double quotes for the executable path, because the
   // CreateProcess() API used on Windows does not support single quotes.
-  const std::string command_restore = "\"" + get_path_to_postgres_executable("pg_restore") + "\"" +
-    " -d " + database_name +
-    " --host=\"" + m_host + "\"" +
+  const std::string command_restore = get_path_to_postgres_executable("pg_restore") +
+    " -d " + database_name + //TODO: Quote database name?
+    " --host=" + Glib::shell_quote(m_host) +
     " --port=" + port_as_string(m_port) +
-    " --username=\"" + username + "\"" +
+    " --username=" + Glib::shell_quote(username) +
     " " + path_backup;
 
   std::cout << "DEBUG: command_restore=" << command_restore << std::endl;
diff --git a/glom/libglom/connectionpool_backends/postgres.h b/glom/libglom/connectionpool_backends/postgres.h
index 3b4c106..90ddab9 100644
--- a/glom/libglom/connectionpool_backends/postgres.h
+++ b/glom/libglom/connectionpool_backends/postgres.h
@@ -58,7 +58,9 @@ public:
 
   virtual bool convert_backup(const SlotProgress& slot_progress, const std::string& base_directory, const Glib::ustring& username, const Glib::ustring& password, const Glib::ustring& database_name);
 
-  static std::string get_path_to_postgres_executable(const std::string& program);
+  /** Return the quoted path to the specified PostgreSQL utility.
+   */
+  static std::string get_path_to_postgres_executable(const std::string& program, bool quoted = true);
 
 
 
diff --git a/glom/libglom/connectionpool_backends/postgres_self.cc b/glom/libglom/connectionpool_backends/postgres_self.cc
index a23d352..9e12ebc 100644
--- a/glom/libglom/connectionpool_backends/postgres_self.cc
+++ b/glom/libglom/connectionpool_backends/postgres_self.cc
@@ -29,6 +29,7 @@
 #include <glibmm/stringutils.h>
 #include <glibmm/regex.h>
 #include <glibmm/main.h>
+#include <glibmm/shell.h>
 #include <glibmm/i18n.h>
 
 #include <libglom/gst-package.h>
@@ -236,8 +237,8 @@ Backend::InitErrors PostgresSelfHosted::initialize(const SlotProgress& slot_prog
 
   // Make sure to use double quotes for the executable path, because the
   // CreateProcess() API used on Windows does not support single quotes.
-  const std::string command_initdb = "\"" + get_path_to_postgres_executable("initdb") + "\" -D \"" + dbdir_data + "\"" +
-                                        " -U " + initial_username + " --pwfile=\"" + temp_pwfile + "\"";
+  const std::string command_initdb = get_path_to_postgres_executable("initdb") + " -D " + Glib::shell_quote(dbdir_data) +
+                                        " -U " + initial_username + " --pwfile=" + Glib::shell_quote(temp_pwfile);
 
   //Note that --pwfile takes the password from the first line of a file. It's an alternative to supplying it when prompted on stdin.
   const bool result = Glom::Spawn::execute_command_line_and_wait(command_initdb, slot_progress);
@@ -256,7 +257,7 @@ Glib::ustring PostgresSelfHosted::get_postgresql_utils_version(const SlotProgres
 {
   Glib::ustring result;
 
-  const std::string command = "\"" + get_path_to_postgres_executable("pg_ctl") + "\" --version";
+  const std::string command = get_path_to_postgres_executable("pg_ctl") + " --version";
 
   //The first command does not return, but the second command can check whether it succeeded:
   std::string output;
@@ -445,17 +446,21 @@ Backend::StartupErrors PostgresSelfHosted::startup(const SlotProgress& slot_prog
   // -k specifies a directory to use for the socket. This must be writable by us.
   // Make sure to use double quotes for the executable path, because the
   // CreateProcess() API used on Windows does not support single quotes.
-  const std::string command_postgres_start = "\"" + get_path_to_postgres_executable("postgres") + "\" -D \"" + dbdir_data + "\" "
+  const std::string dbdir_config = Glib::build_filename(dbdir, "config");
+  const std::string dbdir_hba = Glib::build_filename(dbdir_config, "pg_hba.conf");
+  const std::string dbdir_ident = Glib::build_filename(dbdir_config, "pg_ident.conf");
+  const std::string dbdir_pid = Glib::build_filename(dbdir, "pid");
+  const std::string command_postgres_start = get_path_to_postgres_executable("postgres") + " -D " + Glib::shell_quote(dbdir_data)
                                   + " -p " + port_as_text
                                   + " -i " //Equivalent to -h "*", which in turn is equivalent to listen_addresses in postgresql.conf. Listen to all IP addresses, so any client can connect (with a username+password)
-                                  + " -c hba_file=\"" + dbdir + "/config/pg_hba.conf\""
-                                  + " -c ident_file=\"" + dbdir + "/config/pg_ident.conf\""
-                                  + " -k \"" + dbdir + "\""
-                                  + " --external_pid_file=\"" + dbdir + "/pid\"";
+                                  + " -c hba_file=" + Glib::shell_quote(dbdir_hba)
+                                  + " -c ident_file=" + Glib::shell_quote(dbdir_ident)
+                                  + " -k " + Glib::shell_quote(dbdir)
+                                  + " --external_pid_file=" + Glib::shell_quote(dbdir_pid);
 
   // Make sure to use double quotes for the executable path, because the
   // CreateProcess() API used on Windows does not support single quotes.
-  const std::string command_check_postgres_has_started = "\"" + get_path_to_postgres_executable("pg_ctl") + "\" status -D \"" + dbdir_data + "\"";
+  const std::string command_check_postgres_has_started = get_path_to_postgres_executable("pg_ctl") + " status -D " + Glib::shell_quote(dbdir_data);
 
   //For postgres 8.1, this is "postmaster is running".
   //For postgres 8.2, this is "server is running".
@@ -505,7 +510,7 @@ bool PostgresSelfHosted::cleanup(const SlotProgress& slot_progress)
   // TODO: Warn about connected clients on other computers? Warn those other users?
   // Make sure to use double quotes for the executable path, because the
   // CreateProcess() API used on Windows does not support single quotes.
-  const std::string command_postgres_stop = "\"" + get_path_to_postgres_executable("pg_ctl") + "\" -D \"" + dbdir_data + "\" stop -m fast";
+  const std::string command_postgres_stop = get_path_to_postgres_executable("pg_ctl") + " -D " + Glib::shell_quote(dbdir_data) + " stop -m fast";
   const bool result = Glom::Spawn::execute_command_line_and_wait(command_postgres_stop, slot_progress);
   if(!result)
   {
diff --git a/glom/main.cc b/glom/main.cc
index c4cc15a..98dfa00 100644
--- a/glom/main.cc
+++ b/glom/main.cc
@@ -292,7 +292,7 @@ bool check_user_is_not_root_with_warning()
  */
 bool check_postgres_is_available_with_warning()
 {
-  const std::string binpath = Glom::ConnectionPoolBackends::PostgresSelfHosted::get_path_to_postgres_executable("postgres");
+  const std::string binpath = Glom::ConnectionPoolBackends::PostgresSelfHosted::get_path_to_postgres_executable("postgres", false /* not quoted */);
 
   // TODO: At least on Windows we should probably also check for initdb and
   // pg_ctl. Perhaps it would also be a good idea to access these files as