gnucash unstable: Correct quoting for SQL backends.

John Ralls jralls at code.gnucash.org
Sat Nov 25 17:47:06 EST 2017


Updated	 via  https://github.com/Gnucash/gnucash/commit/d8c2f524 (commit)
	from  https://github.com/Gnucash/gnucash/commit/037c93fa (commit)



commit d8c2f5244755b8162268077335c78df1449e9a48
Author: John Ralls <jralls at ceridwen.us>
Date:   Fri Nov 24 16:53:58 2017 -0800

    Correct quoting for SQL backends.
    
    Only string values should be quoted in queries; in particular NULL
    isn't a string value and must not be quoted.
    Note that this is a less than perfect solution because it doesn't use
    the Database's quoting function and so doesn't escape quotes, linefeeds,
    or carriage returns inside the string. That's because the SQL generating
    logic is independent of the connection class and can't easily get to it.

diff --git a/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp b/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp
index 0498439..a4172f4 100644
--- a/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp
+++ b/libgnucash/backend/dbi/gnc-dbisqlconnection.cpp
@@ -73,8 +73,10 @@ GncDbiSqlStatement::add_where_cond(QofIdTypeConst type_name,
     {
         if (colpair != *col_values.begin())
             m_sql += " AND ";
-        m_sql += colpair.first + " = " +
-            m_conn->quote_string (colpair.second.c_str());
+        if (colpair.second == "NULL")
+            m_sql += colpair.first + " IS " + colpair.second;
+        else
+            m_sql += colpair.first + " = " + colpair.second;
     }
 }
 
diff --git a/libgnucash/backend/sql/gnc-address-sql.cpp b/libgnucash/backend/sql/gnc-address-sql.cpp
index 4ef7fc3..4445bef 100644
--- a/libgnucash/backend/sql/gnc-address-sql.cpp
+++ b/libgnucash/backend/sql/gnc-address-sql.cpp
@@ -138,7 +138,7 @@ GncSqlColumnTableEntryImpl<CT_ADDRESS>::add_to_query(QofIdTypeConst obj_name,
         if (s == nullptr)
             continue;
         auto buf = std::string{m_col_name} + "_" + subtable_row->m_col_name;
-        vec.emplace_back(make_pair(buf, std::string(s)));
+        vec.emplace_back(make_pair(buf, quote_string(s)));
     }
 }
 /* ========================== END OF FILE ===================== */
diff --git a/libgnucash/backend/sql/gnc-owner-sql.cpp b/libgnucash/backend/sql/gnc-owner-sql.cpp
index 2587689..2e862b2 100644
--- a/libgnucash/backend/sql/gnc-owner-sql.cpp
+++ b/libgnucash/backend/sql/gnc-owner-sql.cpp
@@ -224,12 +224,12 @@ GncSqlColumnTableEntryImpl<CT_OWNERREF>::add_to_query(QofIdTypeConst obj_name,
     std::ostringstream buf;
 
     buf << type;
-    vec.emplace_back(std::make_pair(type_hdr, buf.str()));
+    vec.emplace_back(std::make_pair(type_hdr, quote_string(buf.str())));
     buf.str("");
     auto guid = qof_instance_get_guid(inst);
     if (guid != nullptr)
         buf << guid_to_string(guid);
     else
         buf << "NULL";
-    vec.emplace_back(std::make_pair(guid_hdr, buf.str()));
+    vec.emplace_back(std::make_pair(guid_hdr, quote_string(buf.str())));
 }
diff --git a/libgnucash/backend/sql/gnc-sql-backend.cpp b/libgnucash/backend/sql/gnc-sql-backend.cpp
index 3af2d3e..0c7e6af 100644
--- a/libgnucash/backend/sql/gnc-sql-backend.cpp
+++ b/libgnucash/backend/sql/gnc-sql-backend.cpp
@@ -883,7 +883,7 @@ GncSqlBackend::build_insert_statement (const char* table_name,
     {
         if (col_value != *values.begin())
             sql << ",";
-        sql << quote_string(col_value.second);
+        sql << col_value.second;
     }
     sql << ")";
 
@@ -914,7 +914,7 @@ GncSqlBackend::build_update_statement(const gchar* table_name,
         if (col_value != *values.begin())
             sql << ",";
         sql << col_value.first << "=" <<
-            quote_string(col_value.second);
+            col_value.second;
     }
 
     stmt = create_statement_from_sql(sql.str());
diff --git a/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp b/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp
index 3f3bf52..7a0548c 100644
--- a/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp
+++ b/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp
@@ -50,6 +50,7 @@ set_autoinc_id (void* object, void* item)
     // Nowhere to put the ID
 }
 
+
 QofAccessFunc
 GncSqlColumnTableEntry::get_getter (QofIdTypeConst obj_name) const noexcept
 {
@@ -103,7 +104,7 @@ GncSqlColumnTableEntry::add_objectref_guid_to_query (QofIdTypeConst obj_name,
     auto guid = qof_instance_get_guid (inst);
     if (guid != nullptr)
         vec.emplace_back (std::make_pair (std::string{m_col_name},
-                                          std::string{guid_to_string(guid)}));
+                                          quote_string(guid_to_string(guid))));
 }
 
 void
@@ -153,7 +154,8 @@ GncSqlColumnTableEntryImpl<CT_STRING>::add_to_query(QofIdTypeConst obj_name,
     {
         std::ostringstream stream;
         stream << s;
-        vec.emplace_back (std::make_pair (std::string{m_col_name}, stream.str()));
+        vec.emplace_back (std::make_pair (std::string{m_col_name},
+                                          quote_string(stream.str())));
         return;
     }
 }
@@ -360,7 +362,7 @@ GncSqlColumnTableEntryImpl<CT_GUID>::add_to_query(QofIdTypeConst obj_name,
     {
 
         vec.emplace_back (std::make_pair (std::string{m_col_name},
-                                          std::string{guid_to_string(s)}));
+                                          quote_string(guid_to_string(s))));
         return;
     }
 }
@@ -467,7 +469,7 @@ GncSqlColumnTableEntryImpl<CT_TIMESPEC>::add_to_query(QofIdTypeConst obj_name,
 
     GncDateTime time(ts.tv_sec);
     vec.emplace_back (std::make_pair (std::string{m_col_name},
-                                      time.format_zulu ("%Y-%m-%d %H:%M:%S")));
+                                      time.format_zulu ("'%Y-%m-%d %H:%M:%S'")));
 }
 
 /* ----------------------------------------------------------------- */
@@ -539,7 +541,8 @@ GncSqlColumnTableEntryImpl<CT_GDATE>::add_to_query(QofIdTypeConst obj_name,
         buf << std::setfill ('0') << std::setw (4) << g_date_get_year (date) <<
             std::setw (2) << g_date_get_month (date) <<
             std::setw (2) << static_cast<int>(g_date_get_day (date));
-        vec.emplace_back (std::make_pair (std::string{m_col_name}, buf.str()));
+        vec.emplace_back (std::make_pair (std::string{m_col_name},
+                                          quote_string(buf.str())));
         return;
     }
 }
diff --git a/libgnucash/backend/sql/gnc-sql-column-table-entry.hpp b/libgnucash/backend/sql/gnc-sql-column-table-entry.hpp
index d86fbfc..28fbb64 100644
--- a/libgnucash/backend/sql/gnc-sql-column-table-entry.hpp
+++ b/libgnucash/backend/sql/gnc-sql-column-table-entry.hpp
@@ -30,6 +30,7 @@ extern "C"
 }
 #include <memory>
 #include <vector>
+#include <iostream>
 #include "gnc-sql-result.hpp"
 
 struct GncSqlColumnInfo;
@@ -86,6 +87,27 @@ enum GncSqlObjectType
     CT_TAXTABLEREF
 };
 
+static inline std::string
+quote_string(const std::string& str)
+{
+    if (str == "NULL" || str == "null") return "NULL";
+    /* FIXME: This is here because transactions.num has a NULL
+     * constraint, which is dumb; it's often empty.
+     */
+    if (str.empty()) return "''";
+    std::string retval;
+    retval.reserve(str.length() + 2);
+    retval.insert(0, 1, '\'');
+    for (auto c = str.begin(); c != str.end(); ++c)
+    {
+        if (*c == '\'')
+            retval += *c;
+        retval += *c;
+    }
+    retval += '\'';
+    return retval;
+}
+
 /**
  * Contains all of the information required to copy information between an
  * object and the database for a specific object property.



Summary of changes:
 libgnucash/backend/dbi/gnc-dbisqlconnection.cpp    |  6 ++++--
 libgnucash/backend/sql/gnc-address-sql.cpp         |  2 +-
 libgnucash/backend/sql/gnc-owner-sql.cpp           |  4 ++--
 libgnucash/backend/sql/gnc-sql-backend.cpp         |  4 ++--
 .../backend/sql/gnc-sql-column-table-entry.cpp     | 13 ++++++++-----
 .../backend/sql/gnc-sql-column-table-entry.hpp     | 22 ++++++++++++++++++++++
 6 files changed, 39 insertions(+), 12 deletions(-)



More information about the gnucash-changes mailing list