Fix regression in mozIStorageStatement.bindUTF8StringParameter such that it accepts arbitrary octets by
authorDan Witte <dwitte@mozilla.com>
Mon, 22 Feb 2010 13:57:45 -0800
changeset 38397 252879cf77bd094467cba8fc22cbe41ec6bb9aa0
parent 38396 2c60006b6c1fedd09095b3153dd6415765f478ff
child 38399 29f9e42245333fc9cbd737b24dbedfaeb63a03f2
push idunknown
push userunknown
push dateunknown
bugs544496
milestone1.9.3a2pre
Fix regression in mozIStorageStatement.bindUTF8StringParameter such that it accepts arbitrary octets by not converting string types. b=544496, r=sdwilsh
storage/src/mozStorageBindingParams.cpp
storage/src/mozStorageConnection.cpp
storage/src/mozStoragePrivateHelpers.h
storage/src/variantToSQLiteT_impl.h
storage/test/Makefile.in
storage/test/storage_test_harness.h
storage/test/storage_test_harness_tail.h
storage/test/test_binding_params.cpp
--- a/storage/src/mozStorageBindingParams.cpp
+++ b/storage/src/mozStorageBindingParams.cpp
@@ -65,72 +65,79 @@ struct BindingColumnData
   }
   sqlite3_stmt *stmt;
   int column;
 };
 
 } // anonymous namespace
 
 ////////////////////////////////////////////////////////////////////////////////
-//// sqlite3_stmt Specialization Functions (varaintToSQLite3T)
+//// Variant Specialization Functions (variantToSQLiteT)
 
-template < >
-int
+static int
 sqlite3_T_int(BindingColumnData aData,
               int aValue)
 {
   return ::sqlite3_bind_int(aData.stmt, aData.column + 1, aValue);
 }
 
-template < >
-int
+static int
 sqlite3_T_int64(BindingColumnData aData,
                 sqlite3_int64 aValue)
 {
   return ::sqlite3_bind_int64(aData.stmt, aData.column + 1, aValue);
 }
 
-template < >
-int
+static int
 sqlite3_T_double(BindingColumnData aData,
                  double aValue)
 {
   return ::sqlite3_bind_double(aData.stmt, aData.column + 1, aValue);
 }
 
-template < >
-int
+static int
+sqlite3_T_text(BindingColumnData aData,
+               const nsCString& aValue)
+{
+  return ::sqlite3_bind_text(aData.stmt,
+                             aData.column + 1,
+                             aValue.get(),
+                             aValue.Length(),
+                             SQLITE_TRANSIENT);
+}
+
+static int
 sqlite3_T_text16(BindingColumnData aData,
-                 nsString aValue)
+                 const nsString& aValue)
 {
   return ::sqlite3_bind_text16(aData.stmt,
                                aData.column + 1,
-                               PromiseFlatString(aValue).get(),
+                               aValue.get(),
                                aValue.Length() * 2, // Length in bytes!
                                SQLITE_TRANSIENT);
 }
 
-template < >
-int
+static int
 sqlite3_T_null(BindingColumnData aData)
 {
   return ::sqlite3_bind_null(aData.stmt, aData.column + 1);
 }
 
-template < >
-int
+static int
 sqlite3_T_blob(BindingColumnData aData,
                const void *aBlob,
                int aSize)
 {
   return ::sqlite3_bind_blob(aData.stmt, aData.column + 1, aBlob, aSize,
                              NS_Free);
 
 }
 
+#include "variantToSQLiteT_impl.h"
+
 ////////////////////////////////////////////////////////////////////////////////
 //// BindingParams
 
 BindingParams::BindingParams(BindingParamsArray *aOwningArray,
                              Statement *aOwningStatement)
 : mOwningArray(aOwningArray)
 , mOwningStatement(aOwningStatement)
 , mLocked(false)
--- a/storage/src/mozStorageConnection.cpp
+++ b/storage/src/mozStorageConnection.cpp
@@ -74,75 +74,82 @@ PRLogModuleInfo* gStorageLog = nsnull;
 #endif
 
 namespace mozilla {
 namespace storage {
 
 #define PREF_TS_SYNCHRONOUS "toolkit.storage.synchronous"
 
 ////////////////////////////////////////////////////////////////////////////////
-//// sqlite3_context Specialization Functions
+//// Variant Specialization Functions (variantToSQLiteT)
 
-template < >
-int
+static int
 sqlite3_T_int(sqlite3_context *aCtx,
               int aValue)
 {
   ::sqlite3_result_int(aCtx, aValue);
   return SQLITE_OK;
 }
 
-template < >
-int
+static int
 sqlite3_T_int64(sqlite3_context *aCtx,
                 sqlite3_int64 aValue)
 {
   ::sqlite3_result_int64(aCtx, aValue);
   return SQLITE_OK;
 }
 
-template < >
-int
+static int
 sqlite3_T_double(sqlite3_context *aCtx,
                  double aValue)
 {
   ::sqlite3_result_double(aCtx, aValue);
   return SQLITE_OK;
 }
 
-template < >
-int
+static int
+sqlite3_T_text(sqlite3_context *aCtx,
+               const nsCString &aValue)
+{
+  ::sqlite3_result_text(aCtx,
+                        aValue.get(),
+                        aValue.Length(),
+                        SQLITE_TRANSIENT);
+  return SQLITE_OK;
+}
+
+static int
 sqlite3_T_text16(sqlite3_context *aCtx,
-                 nsString aValue)
+                 const nsString &aValue)
 {
   ::sqlite3_result_text16(aCtx,
-                          PromiseFlatString(aValue).get(),
+                          aValue.get(),
                           aValue.Length() * 2, // Number of bytes.
                           SQLITE_TRANSIENT);
   return SQLITE_OK;
 }
 
-template < >
-int
+static int
 sqlite3_T_null(sqlite3_context *aCtx)
 {
   ::sqlite3_result_null(aCtx);
   return SQLITE_OK;
 }
 
-template < >
-int
+static int
 sqlite3_T_blob(sqlite3_context *aCtx,
                const void *aData,
                int aSize)
 {
   ::sqlite3_result_blob(aCtx, aData, aSize, NS_Free);
   return SQLITE_OK;
 }
 
+#include "variantToSQLiteT_impl.h"
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Local Functions
 
 namespace {
 #ifdef PR_LOGGING
 void tracefunc (void *aClosure, const char *aStmt)
 {
   PR_LOG(gStorageLog, PR_LOG_DEBUG, ("sqlite3_trace on %p for '%s'", aClosure,
--- a/storage/src/mozStoragePrivateHelpers.h
+++ b/storage/src/mozStoragePrivateHelpers.h
@@ -88,19 +88,12 @@ void checkAndLogStatementPerformance(sql
  *
  */
 bool
 bindJSValue(JSContext *aCtx,
             mozIStorageStatement *aStatement,
             int aIdx,
             jsval aValue);
 
-/**
- * Used to convert an nsIVariant to the proper SQLite type.
- */
-template <typename T>
-int variantToSQLiteT(T aObj, nsIVariant *aValue);
-#include "variantToSQLiteT_impl.h" // To keep this file easier to read.
-
 } // namespace storage
 } // namespace mozilla
 
 #endif // _mozStoragePrivateHelpers_h_
--- a/storage/src/variantToSQLiteT_impl.h
+++ b/storage/src/variantToSQLiteT_impl.h
@@ -35,31 +35,18 @@
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 // Note: we are already in the namepace mozilla::storage
 
-////////////////////////////////////////////////////////////////////////////////
-//// Specialization Declarations
-
-template <typename T>
-int sqlite3_T_int(T aObj, int aValue);
-template <typename T>
-int sqlite3_T_int64(T aObj, sqlite_int64 aValue);
-template <typename T>
-int sqlite3_T_double(T aObj, double aValue);
-template <typename T>
-int sqlite3_T_text16(T aObj, nsString aValue);
-template <typename T>
-int sqlite3_T_null(T aObj);
-template <typename T>
-int sqlite3_T_blob(T aObj, const void *aBlob, int aSize);
+// Note 2: whoever #includes this file must provide implementations of
+// sqlite3_T_* prior.
 
 ////////////////////////////////////////////////////////////////////////////////
 //// variantToSQLiteT Implementation
 
 template <typename T>
 int
 variantToSQLiteT(T aObj,
                  nsIVariant *aValue)
@@ -103,24 +90,33 @@ variantToSQLiteT(T aObj,
     case nsIDataType::VTYPE_BOOL:
     {
       PRBool value;
       nsresult rv = aValue->GetAsBool(&value);
       NS_ENSURE_SUCCESS(rv, SQLITE_MISMATCH);
       return sqlite3_T_int(aObj, value ? 1 : 0);
     }
     case nsIDataType::VTYPE_CHAR:
+    case nsIDataType::VTYPE_CHAR_STR:
+    case nsIDataType::VTYPE_STRING_SIZE_IS:
+    case nsIDataType::VTYPE_UTF8STRING:
+    case nsIDataType::VTYPE_CSTRING:
+    {
+      nsCAutoString value;
+      // GetAsAUTF8String should never perform conversion when coming from
+      // 8-bit string types, and thus can accept strings with arbitrary encoding
+      // (including UTF8 and ASCII).
+      nsresult rv = aValue->GetAsAUTF8String(value);
+      NS_ENSURE_SUCCESS(rv, SQLITE_MISMATCH);
+      return sqlite3_T_text(aObj, value);
+    }
     case nsIDataType::VTYPE_WCHAR:
     case nsIDataType::VTYPE_DOMSTRING:
-    case nsIDataType::VTYPE_CHAR_STR:
     case nsIDataType::VTYPE_WCHAR_STR:
-    case nsIDataType::VTYPE_STRING_SIZE_IS:
     case nsIDataType::VTYPE_WSTRING_SIZE_IS:
-    case nsIDataType::VTYPE_UTF8STRING:
-    case nsIDataType::VTYPE_CSTRING:
     case nsIDataType::VTYPE_ASTRING:
     {
       nsAutoString value;
       // GetAsAString does proper conversion to UCS2 from all string-like types.
       // It can be used universally without problems (unless someone implements
       // their own variant, but that's their problem).
       nsresult rv = aValue->GetAsAString(value);
       NS_ENSURE_SUCCESS(rv, SQLITE_MISMATCH);
--- a/storage/test/Makefile.in
+++ b/storage/test/Makefile.in
@@ -47,16 +47,17 @@ include $(DEPTH)/config/autoconf.mk
 MODULE = test_storage
 
 XPCSHELL_TESTS = unit
 
 CPP_UNIT_TESTS = \
   test_transaction_helper.cpp \
   test_statement_scoper.cpp \
   test_mutex.cpp \
+  test_binding_params.cpp \
   $(NULL)
 
 ifdef MOZ_DEBUG
 # FIXME bug 523392: test_deadlock_detector doesn't like Windows
 # FIXME bug 523378: also fails on OS X
 ifneq (,$(filter-out WINNT WINCE Darwin,$(OS_ARCH)))
 CPP_UNIT_TESTS += \
   test_deadlock_detector.cpp \
--- a/storage/test/storage_test_harness.h
+++ b/storage/test/storage_test_harness.h
@@ -37,18 +37,18 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "TestHarness.h"
 #include "nsMemory.h"
 #include "mozIStorageService.h"
 #include "mozIStorageConnection.h"
 
-static size_t gTotalTests = 0;
-static size_t gPassedTests = 0;
+static int gTotalTests = 0;
+static int gPassedTests = 0;
 
 #define do_check_true(aCondition) \
   PR_BEGIN_MACRO \
     gTotalTests++; \
     if (aCondition) \
       gPassedTests++; \
     else \
       fail("Expected true, got false on line %d!", __LINE__); \
--- a/storage/test/storage_test_harness_tail.h
+++ b/storage/test/storage_test_harness_tail.h
@@ -52,10 +52,10 @@ main(int aArgc,
   ScopedXPCOM xpcom(TEST_NAME);
 
   for (size_t i = 0; i < NS_ARRAY_LENGTH(gTests); i++)
     gTests[i]();
 
   if (gPassedTests == gTotalTests)
     passed(TEST_FILE);
 
-  (void)printf("%zu of %zu tests passed\n", gPassedTests, gTotalTests);
+  (void)printf("%i of %i tests passed\n", gPassedTests, gTotalTests);
 }
new file mode 100644
--- /dev/null
+++ b/storage/test/test_binding_params.cpp
@@ -0,0 +1,246 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ * vim: sw=2 ts=2 et lcs=trail\:.,tab\:>~ :
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is storage test code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Daniel Witte <dwitte@mozilla.com> (Original Author)
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "storage_test_harness.h"
+
+#include "mozStorageHelper.h"
+  
+/**
+ * This file tests binding and reading out string parameters through the
+ * mozIStorageStatement API.
+ */
+
+void
+test_ASCIIString()
+{
+  nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
+
+  // Create table with a single string column.
+  (void)db->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TABLE test (str STRING)"
+  ));
+
+  // Create statements to INSERT and SELECT the string.
+  nsCOMPtr<mozIStorageStatement> insert, select;
+  (void)db->CreateStatement(NS_LITERAL_CSTRING(
+    "INSERT INTO test (str) VALUES (?1)"
+  ), getter_AddRefs(insert));
+  (void)db->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT str FROM test"
+  ), getter_AddRefs(select));
+
+  // Roundtrip a string through the table, and ensure it comes out as expected.
+  nsCAutoString inserted("I'm an ASCII string");
+  {
+    mozStorageStatementScoper scoper(insert);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(insert->BindUTF8StringParameter(0, inserted)));
+    do_check_true(NS_SUCCEEDED(insert->ExecuteStep(&hasResult)));
+    do_check_false(hasResult);
+  }
+
+  nsCAutoString result;
+  {
+    mozStorageStatementScoper scoper(select);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(select->ExecuteStep(&hasResult)));
+    do_check_true(hasResult);
+    do_check_true(NS_SUCCEEDED(select->GetUTF8String(0, result)));
+  }
+
+  do_check_true(result == inserted);
+
+  (void)db->ExecuteSimpleSQL(NS_LITERAL_CSTRING("DELETE FROM test"));
+}
+
+void
+test_CString()
+{
+  nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
+
+  // Create table with a single string column.
+  (void)db->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TABLE test (str STRING)"
+  ));
+
+  // Create statements to INSERT and SELECT the string.
+  nsCOMPtr<mozIStorageStatement> insert, select;
+  (void)db->CreateStatement(NS_LITERAL_CSTRING(
+    "INSERT INTO test (str) VALUES (?1)"
+  ), getter_AddRefs(insert));
+  (void)db->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT str FROM test"
+  ), getter_AddRefs(select));
+
+  // Roundtrip a string through the table, and ensure it comes out as expected.
+  static const char sCharArray[] =
+    "I'm not a \xff\x00\xac\xde\xbb ASCII string!";
+  nsCAutoString inserted(sCharArray, NS_ARRAY_LENGTH(sCharArray) - 1);
+  do_check_true(inserted.Length() == NS_ARRAY_LENGTH(sCharArray) - 1);
+  {
+    mozStorageStatementScoper scoper(insert);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(insert->BindUTF8StringParameter(0, inserted)));
+    do_check_true(NS_SUCCEEDED(insert->ExecuteStep(&hasResult)));
+    do_check_false(hasResult);
+  }
+
+  {
+    nsCAutoString result;
+
+    mozStorageStatementScoper scoper(select);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(select->ExecuteStep(&hasResult)));
+    do_check_true(hasResult);
+    do_check_true(NS_SUCCEEDED(select->GetUTF8String(0, result)));
+
+    do_check_true(result == inserted);
+  }
+
+  (void)db->ExecuteSimpleSQL(NS_LITERAL_CSTRING("DELETE FROM test"));
+}
+
+void
+test_UTFStrings()
+{
+  nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
+
+  // Create table with a single string column.
+  (void)db->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TABLE test (str STRING)"
+  ));
+
+  // Create statements to INSERT and SELECT the string.
+  nsCOMPtr<mozIStorageStatement> insert, select;
+  (void)db->CreateStatement(NS_LITERAL_CSTRING(
+    "INSERT INTO test (str) VALUES (?1)"
+  ), getter_AddRefs(insert));
+  (void)db->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT str FROM test"
+  ), getter_AddRefs(select));
+
+  // Roundtrip a UTF8 string through the table, using UTF8 input and output.
+  static const char sCharArray[] =
+    "I'm a \xc3\xbb\xc3\xbc\xc3\xa2\xc3\xa4\xc3\xa7 UTF8 string!";
+  nsCAutoString insertedUTF8(sCharArray, NS_ARRAY_LENGTH(sCharArray) - 1);
+  do_check_true(insertedUTF8.Length() == NS_ARRAY_LENGTH(sCharArray) - 1);
+  NS_ConvertUTF8toUTF16 insertedUTF16(insertedUTF8);
+  do_check_true(insertedUTF8 == NS_ConvertUTF16toUTF8(insertedUTF16));
+  {
+    mozStorageStatementScoper scoper(insert);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(insert->BindUTF8StringParameter(0, insertedUTF8)));
+    do_check_true(NS_SUCCEEDED(insert->ExecuteStep(&hasResult)));
+    do_check_false(hasResult);
+  }
+
+  {
+    nsCAutoString result;
+
+    mozStorageStatementScoper scoper(select);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(select->ExecuteStep(&hasResult)));
+    do_check_true(hasResult);
+    do_check_true(NS_SUCCEEDED(select->GetUTF8String(0, result)));
+
+    do_check_true(result == insertedUTF8);
+  }
+
+  // Use UTF8 input and UTF16 output.
+  {
+    nsAutoString result;
+
+    mozStorageStatementScoper scoper(select);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(select->ExecuteStep(&hasResult)));
+    do_check_true(hasResult);
+    do_check_true(NS_SUCCEEDED(select->GetString(0, result)));
+
+    do_check_true(result == insertedUTF16);
+  }
+
+  (void)db->ExecuteSimpleSQL(NS_LITERAL_CSTRING("DELETE FROM test"));
+
+  // Roundtrip the same string using UTF16 input and UTF8 output.
+  {
+    mozStorageStatementScoper scoper(insert);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(insert->BindStringParameter(0, insertedUTF16)));
+    do_check_true(NS_SUCCEEDED(insert->ExecuteStep(&hasResult)));
+    do_check_false(hasResult);
+  }
+
+  {
+    nsCAutoString result;
+
+    mozStorageStatementScoper scoper(select);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(select->ExecuteStep(&hasResult)));
+    do_check_true(hasResult);
+    do_check_true(NS_SUCCEEDED(select->GetUTF8String(0, result)));
+
+    do_check_true(result == insertedUTF8);
+  }
+
+  // Use UTF16 input and UTF16 output.
+  {
+    nsAutoString result;
+
+    mozStorageStatementScoper scoper(select);
+    PRBool hasResult;
+    do_check_true(NS_SUCCEEDED(select->ExecuteStep(&hasResult)));
+    do_check_true(hasResult);
+    do_check_true(NS_SUCCEEDED(select->GetString(0, result)));
+
+    do_check_true(result == insertedUTF16);
+  }
+
+  (void)db->ExecuteSimpleSQL(NS_LITERAL_CSTRING("DELETE FROM test"));
+}
+
+void (*gTests[])(void) = {
+  test_ASCIIString,
+  test_CString,
+  test_UTFStrings,
+};
+
+const char *file = __FILE__;
+#define TEST_NAME "binding string params"
+#define TEST_FILE file
+#include "storage_test_harness_tail.h"