GNOME.org
 

[glom] Escape database connection details properly.

commit 6d313884cbd31d6f035ae71254c2858a9a82f02e
Author: Murray Cumming <murrayc murrayc com>
Date:   Thu Nov 10 10:22:55 2011 +0100

    Escape database connection details properly.
    
    * glom/libglom/db_utils.[h|cc]: Added gda_cnc_string_encode() for use
    with GdaConnection's cnc_string and auth_string, for instance to
    escape the database directory, name, username, and password.
    * glom/libglom/connectionpool_backends/postgres.cc:
    * glom/libglom/connectionpool_backends/postgres_self.cc:
    * glom/libglom/connectionpool_backends/sqlite.cc: Properly escape the
    cnc_string and auth_string key values.
    
    * tests/test_selfhosting_utils.cc: test_create_and_selfhost_from_example(),
    test_create_and_selfhost_from_uri(): Add an optional subdirectory parameter
    that we can use to force weird parts into the path.
    * Makefile_tests.am:
    * tests/test_selfhosting_new_from_example_strangepath.cc: Add a
    test case that uses a path with some weird characters. This now works
    thanks to the above changes.

 ChangeLog                                          |   20 +++++
 Makefile_tests.am                                  |    7 ++
 glom/libglom/connectionpool_backends/postgres.cc   |   10 ++-
 .../connectionpool_backends/postgres_self.cc       |    1 +
 glom/libglom/connectionpool_backends/sqlite.cc     |   10 ++-
 glom/libglom/db_utils.cc                           |    9 ++
 glom/libglom/db_utils.h                            |    5 +
 ...est_selfhosting_new_from_example_strangepath.cc |   83 ++++++++++++++++++++
 tests/test_selfhosting_utils.cc                    |   10 ++-
 tests/test_selfhosting_utils.h                     |    6 +-
 10 files changed, 148 insertions(+), 13 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index e9fddc3..85e9166 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2011-11-10  Murray Cumming  <murrayc murrayc com>
+
+	Escape database connection details properly.
+
+	* glom/libglom/db_utils.[h|cc]: Added gda_cnc_string_encode() for use 
+	with GdaConnection's cnc_string and auth_string, for instance to 
+	escape the database directory, name, username, and password.
+	* glom/libglom/connectionpool_backends/postgres.cc:
+	* glom/libglom/connectionpool_backends/postgres_self.cc:
+	* glom/libglom/connectionpool_backends/sqlite.cc: Properly escape the 
+	cnc_string and auth_string key values.
+
+	* tests/test_selfhosting_utils.cc: test_create_and_selfhost_from_example(),
+	test_create_and_selfhost_from_uri(): Add an optional subdirectory parameter
+	that we can use to force weird parts into the path.
+	* Makefile_tests.am:
+	* tests/test_selfhosting_new_from_example_strangepath.cc: Add a 
+	test case that uses a path with some weird characters. This now works
+	thanks to the above changes.
+
 2011-11-09  Murray Cumming  <murrayc murrayc com>
 
 	Use Glib::shell_quote() when spawning tar.
diff --git a/Makefile_tests.am b/Makefile_tests.am
index a554577..74f333f 100644
--- a/Makefile_tests.am
+++ b/Makefile_tests.am
@@ -27,6 +27,7 @@ check_PROGRAMS =						\
 	tests/python/test_python_module \
 	tests/test_selfhosting_new_empty \
 	tests/test_selfhosting_new_from_example \
+	tests/test_selfhosting_new_from_example_strangepath \
 	tests/test_selfhosting_new_then_report \
 	tests/test_selfhosting_new_then_image \
 	tests/test_selfhosting_new_then_backup_restore \
@@ -48,6 +49,7 @@ TESTS =	tests/test_document_load	\
 	tests/python/test_python_module \
 	tests/test_selfhosting_new_empty \
 	tests/test_selfhosting_new_from_example \
+	tests/test_selfhosting_new_from_example_strangepath \
 	tests/test_selfhosting_new_then_report \
 	tests/test_selfhosting_new_then_backup_restore \
 	tests/test_selfhosting_new_then_image \
@@ -148,6 +150,11 @@ tests_test_selfhosting_new_from_example_SOURCES = tests/test_selfhosting_new_fro
 tests_test_selfhosting_new_from_example_LDADD = $(tests_ldadd)
 tests_test_selfhosting_new_from_example_CPPFLAGS = $(tests_cppflags)
 
+
+tests_test_selfhosting_new_from_example_strangepath_SOURCES = tests/test_selfhosting_new_from_example_strangepath.cc $(sources_test_selfhosting_utils)
+tests_test_selfhosting_new_from_example_strangepath_LDADD = $(tests_ldadd)
+tests_test_selfhosting_new_from_example_strangepath_CPPFLAGS = $(tests_cppflags)
+
 tests_test_selfhosting_new_then_report_SOURCES = tests/test_selfhosting_new_then_report.cc $(sources_test_selfhosting_utils)
 tests_test_selfhosting_new_then_report_LDADD = $(tests_ldadd)
 tests_test_selfhosting_new_then_report_CPPFLAGS = $(tests_cppflags)
diff --git a/glom/libglom/connectionpool_backends/postgres.cc b/glom/libglom/connectionpool_backends/postgres.cc
index 3b6546f..df2e582 100644
--- a/glom/libglom/connectionpool_backends/postgres.cc
+++ b/glom/libglom/connectionpool_backends/postgres.cc
@@ -46,7 +46,8 @@ static Glib::ustring create_auth_string(const Glib::ustring& username, const Gli
   if(username.empty() and password.empty())
     return Glib::ustring();
   else
-    return "USERNAME=" + username + ";PASSWORD=" + password; //TODO: How should we quote and escape these?
+    return "USERNAME=" + Glom::DbUtils::gda_cnc_string_encode(username) + 
+      ";PASSWORD=" + Glom::DbUtils::gda_cnc_string_encode(password);
 }
 
 } //anonymous namespace
@@ -69,8 +70,9 @@ Glib::RefPtr<Gnome::Gda::Connection> Postgres::attempt_connect(const Glib::ustri
   //This _might_ be different on some systems. I hope not. murrayc
   const Glib::ustring default_database = "template1";
   //const Glib::ustring& actual_database = (!database.empty()) ? database : default_database;;
-  const Glib::ustring cnc_string_main = "HOST=" + m_host + ";PORT=" + port;
-  Glib::ustring cnc_string = cnc_string_main + ";DB_NAME=" + database;
+  const Glib::ustring cnc_string_main = "HOST=" + DbUtils::gda_cnc_string_encode(m_host)
+   + ";PORT=" + DbUtils::gda_cnc_string_encode(port);
+  const Glib::ustring cnc_string = cnc_string_main + ";DB_NAME=" + DbUtils::gda_cnc_string_encode(database);
 
   Glib::RefPtr<Gnome::Gda::Connection> connection;
   Glib::RefPtr<Gnome::Gda::DataModel> data_model;
@@ -99,7 +101,7 @@ Glib::RefPtr<Gnome::Gda::Connection> Postgres::attempt_connect(const Glib::ustri
     std::cout << "debug: " << G_STRFUNC << ": Attempting to connect without specifying the database." << std::endl;
 #endif
 
-    const Glib::ustring cnc_string = cnc_string_main + ";DB_NAME=" + default_database;
+    const Glib::ustring cnc_string = cnc_string_main + ";DB_NAME=" + DbUtils::gda_cnc_string_encode(default_database);
     Glib::RefPtr<Gnome::Gda::Connection> temp_conn;
     Glib::ustring auth_string = create_auth_string(username, password);
     try
diff --git a/glom/libglom/connectionpool_backends/postgres_self.cc b/glom/libglom/connectionpool_backends/postgres_self.cc
index c77e12d..1ba19ba 100644
--- a/glom/libglom/connectionpool_backends/postgres_self.cc
+++ b/glom/libglom/connectionpool_backends/postgres_self.cc
@@ -435,6 +435,7 @@ Backend::StartupErrors PostgresSelfHosted::startup(const SlotProgress& slot_prog
                                   + " -c ident_file=" + Glib::shell_quote(dbdir_ident)
                                   + " -k " + Glib::shell_quote(dbdir)
                                   + " --external_pid_file=" + Glib::shell_quote(dbdir_pid);
+  //std::cout << G_STRFUNC << ": debug: " << command_postgres_start << std::endl;
 
   // Make sure to use double quotes for the executable path, because the
   // CreateProcess() API used on Windows does not support single quotes.
diff --git a/glom/libglom/connectionpool_backends/sqlite.cc b/glom/libglom/connectionpool_backends/sqlite.cc
index fa4eff1..1af915f 100644
--- a/glom/libglom/connectionpool_backends/sqlite.cc
+++ b/glom/libglom/connectionpool_backends/sqlite.cc
@@ -53,6 +53,7 @@ Glib::RefPtr<Gnome::Gda::Connection> Sqlite::connect(const Glib::ustring& databa
   if(!file_exists)
   {
     std::cerr << G_STRFUNC << ": The db file does not exist at path: " << db_file->get_uri() << std::endl;
+    //std::cerr << "  as filepath: " << db_file->get_path() << std::endl;
   }
   else
   {
@@ -65,8 +66,10 @@ Glib::RefPtr<Gnome::Gda::Connection> Sqlite::connect(const Glib::ustring& databa
       // Convert URI to path, for GDA connection string
       const std::string database_directory = db_dir->get_path();
 
-      const Glib::ustring cnc_string = "DB_DIR=" + database_directory + ";DB_NAME=" + database;
-      const Glib::ustring auth_string = Glib::ustring::compose("USERNAME=%1;PASSWORD=%2", username, password);
+      const Glib::ustring cnc_string = "DB_DIR=" + DbUtils::gda_cnc_string_encode(database_directory) + 
+        ";DB_NAME=" + DbUtils::gda_cnc_string_encode(database);
+      const Glib::ustring auth_string = Glib::ustring::compose("USERNAME=%1;PASSWORD=%2", 
+        DbUtils::gda_cnc_string_encode(username), DbUtils::gda_cnc_string_encode(password));
 
       connection = Gnome::Gda::Connection::open_from_string("SQLite",
         cnc_string, auth_string,
@@ -100,7 +103,8 @@ bool Sqlite::create_database(const Glib::ustring& database_name, const Glib::ust
   
   Glib::RefPtr<Gio::File> file = Gio::File::create_for_uri(m_database_directory_uri);
   const std::string database_directory = file->get_path();
-  const Glib::ustring cnc_string = Glib::ustring::compose("DB_DIR=%1;DB_NAME=%2", database_directory, database_name);
+  const Glib::ustring cnc_string = Glib::ustring::compose("DB_DIR=%1;DB_NAME=%2", 
+    DbUtils::gda_cnc_string_encode(database_directory), DbUtils::gda_cnc_string_encode(database_name));
 
   Glib::RefPtr<Gnome::Gda::Connection> cnc =
     Gnome::Gda::Connection::open_from_string("SQLite",
diff --git a/glom/libglom/db_utils.cc b/glom/libglom/db_utils.cc
index 0765fdf..704f768 100644
--- a/glom/libglom/db_utils.cc
+++ b/glom/libglom/db_utils.cc
@@ -1871,6 +1871,15 @@ Glib::ustring escape_sql_id(const Glib::ustring& id)
   return gda_connection->quote_sql_identifier(id);
 }
 
+Glib::ustring gda_cnc_string_encode(const Glib::ustring& str)
+{
+  char* pch = gda_rfc1738_encode(str.c_str());
+  if(!pch)
+    return Glib::ustring();
+  else
+    return Glib::ustring(pch);
+}
+
 Glib::ustring build_query_create_group(const Glib::ustring& group, bool superuser)
 {
   if(group.empty())
diff --git a/glom/libglom/db_utils.h b/glom/libglom/db_utils.h
index 513cd89..a50df37 100644
--- a/glom/libglom/db_utils.h
+++ b/glom/libglom/db_utils.h
@@ -151,6 +151,11 @@ bool drop_table(const Glib::ustring& table_name);
  */
 Glib::ustring escape_sql_id(const Glib::ustring& id);
 
+/** Just a wrapper around gda_rfc1738_encode(),
+ * for use when building libgda connection strings or authentication strings.
+ */ 
+Glib::ustring gda_cnc_string_encode(const Glib::ustring& str);
+
 Glib::ustring build_query_create_group(const Glib::ustring& group, bool superuser = false);
 
 Glib::ustring build_query_add_user_to_group(const Glib::ustring& group, const Glib::ustring& user);
diff --git a/tests/test_selfhosting_new_from_example_strangepath.cc b/tests/test_selfhosting_new_from_example_strangepath.cc
new file mode 100644
index 0000000..c074cbd
--- /dev/null
+++ b/tests/test_selfhosting_new_from_example_strangepath.cc
@@ -0,0 +1,83 @@
+/* Glom
+ *
+ * Copyright (C) 2010 Openismus GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+71 * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#include "tests/test_selfhosting_utils.h"
+#include <libglom/init.h>
+#include <libglom/utils.h>
+#include <libglom/db_utils.h>
+#include <glib.h> //For g_assert()
+#include <iostream>
+#include <cstdlib> //For EXIT_SUCCESS and EXIT_FAILURE
+
+static bool test(Glom::Document::HostingMode hosting_mode)
+{
+  Glom::Document document;
+  const bool recreated = 
+    test_create_and_selfhost_from_example("example_music_collection.glom", document, 
+      hosting_mode, "path with space and \" quote and single ' quote and $ dollar sign and / forward slash and \backwards slash ");
+  if(!recreated)
+  {
+    std::cerr << "Recreation failed." << std::endl;
+    return false;
+  }
+  
+  if(!test_example_musiccollection_data(&document))
+  {
+    std::cerr << "test_example_musiccollection_data() failed." << std::endl;
+    return false;
+  }
+
+  if(!test_table_exists("songs", document))
+  {
+    return false;
+  }
+
+  if(!test_table_exists("publishers", document))
+  {
+    return false;
+  }
+
+  test_selfhosting_cleanup();
+ 
+  return true; 
+}
+
+int main()
+{
+  Glom::libglom_init();
+  
+  if(!test(Glom::Document::HOSTING_MODE_POSTGRES_SELF))
+  {
+    std::cerr << "Failed with PostgreSQL" << std::endl;
+    test_selfhosting_cleanup();
+    return EXIT_FAILURE;
+  }
+  
+  if(!test(Glom::Document::HOSTING_MODE_SQLITE))
+  {
+    std::cerr << "Failed with SQLite" << std::endl;
+    test_selfhosting_cleanup();
+    return EXIT_FAILURE;
+  }
+
+  Glom::libglom_deinit();
+
+  return EXIT_SUCCESS;
+}
diff --git a/tests/test_selfhosting_utils.cc b/tests/test_selfhosting_utils.cc
index f65d550..16493c8 100644
--- a/tests/test_selfhosting_utils.cc
+++ b/tests/test_selfhosting_utils.cc
@@ -138,7 +138,7 @@ void test_selfhosting_cleanup()
   temp_filepath_dir.clear();
 }
 
-bool test_create_and_selfhost_from_example(const std::string& example_filename, Glom::Document& document, Glom::Document::HostingMode hosting_mode)
+bool test_create_and_selfhost_from_example(const std::string& example_filename, Glom::Document& document, Glom::Document::HostingMode hosting_mode, const std::string& subdirectory_path)
 {
   Glib::ustring uri;
   
@@ -156,10 +156,10 @@ bool test_create_and_selfhost_from_example(const std::string& example_filename,
     return false;
   }
   
-  return test_create_and_selfhost_from_uri(uri, document, hosting_mode);
+  return test_create_and_selfhost_from_uri(uri, document, hosting_mode, subdirectory_path);
 }
 
-bool test_create_and_selfhost_from_uri(const Glib::ustring& example_file_uri, Glom::Document& document, Glom::Document::HostingMode hosting_mode)
+bool test_create_and_selfhost_from_uri(const Glib::ustring& example_file_uri, Glom::Document& document, Glom::Document::HostingMode hosting_mode, const std::string& subdirectory_path)
 {
   if( (hosting_mode != Glom::Document::HOSTING_MODE_POSTGRES_SELF) &&
     (hosting_mode != Glom::Document::HOSTING_MODE_SQLITE) )
@@ -189,10 +189,12 @@ bool test_create_and_selfhost_from_uri(const Glib::ustring& example_file_uri, Gl
   Glom::ConnectionPool* connection_pool = Glom::ConnectionPool::get_instance();
 
   //Save a copy, specifying the path to file in a directory:
-  //For instance, /tmp/testfileglom/testfile.glom");
+  //For instance, /tmp/testglom/testglom.glom");
   const std::string temp_filename = "testglom";
   temp_filepath_dir = 
     Glom::Utils::get_temp_directory_path(temp_filename);
+  if(!subdirectory_path.empty())
+    temp_filepath_dir = Glib::build_filename(temp_filepath_dir, subdirectory_path);
   const std::string temp_filepath = Glib::build_filename(temp_filepath_dir, temp_filename);
 
   //Make sure that the file does not exist yet:
diff --git a/tests/test_selfhosting_utils.h b/tests/test_selfhosting_utils.h
index d442cbf..355054f 100644
--- a/tests/test_selfhosting_utils.h
+++ b/tests/test_selfhosting_utils.h
@@ -28,14 +28,16 @@
 /** Create a .glom file from an example, with database data, and start a PostgreSQL server if necessary.
  *
  * @param hosting_mode Either HOSTING_MODE_POSTGRES_SELF or HOSTING_MODE_SQLITE
+ * @param subdirectory_path: An additional directory path to use under the temporary directory that will be used to save the file.
  */
-bool test_create_and_selfhost_from_example(const std::string& example_filename, Glom::Document& document, Glom::Document::HostingMode hosting_mode);
+bool test_create_and_selfhost_from_example(const std::string& example_filename, Glom::Document& document, Glom::Document::HostingMode hosting_mode, const std::string& subdirectory_path = std::string());
 
 /** Create a .glom file from an existing .glom example file with database data, and start a PostgreSQL server if necessary.
  *
  * @param hosting_mode Either HOSTING_MODE_POSTGRES_SELF or HOSTING_MODE_SQLITE
+ * @param subdirectory_path: An additional directory path to use under the temporary directory that will be used to save the file.
  */
-bool test_create_and_selfhost_from_uri(const Glib::ustring& file_uri, Glom::Document& document, Glom::Document::HostingMode hosting_mode);
+bool test_create_and_selfhost_from_uri(const Glib::ustring& file_uri, Glom::Document& document, Glom::Document::HostingMode hosting_mode, const std::string& subdirectory_path = std::string());
 
 bool test_model_expected_size(const Glib::RefPtr<Gnome::Gda::DataModel>& data_model, guint columns_count, guint rows_count);
 bool test_table_exists(const Glib::ustring& table_name, const Glom::Document& document);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]