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 id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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"