Bug 1543295 - Pass the text length when creating an `nsString` from a SQLite text result. r=mak
authorLina Cambridge <lina@yakshaving.ninja>
Thu, 11 Apr 2019 03:19:39 +0000
changeset 468935 b0a8a5c4533e067ea614da696b1ea622a92333f4
parent 468934 99bbdc8f991342463fe6e83f48945405216cb72a
child 468936 4d47e0abfaa821d2e49ec1a2e6828ebdde810ceb
push id82874
push userkcambridge@mozilla.com
push dateThu, 11 Apr 2019 03:20:28 +0000
treeherderautoland@b0a8a5c4533e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1543295
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1543295 - Pass the text length when creating an `nsString` from a SQLite text result. r=mak This commit updates mozStorage to always: * Pass the length, using sqlite3_{column, value}_bytes16, when creating an nsDependentString from a pointer. * Call sqlite3_{column, value}_bytes{16} after sqlite3_{column, value}_{text, blob, text16}, per the recommendation in https://www.sqlite.org/c3ref/column_blob.html. Some callers did this before, or in unclear order, since C++ doesn't specify one for evaluating function arguments. * Pass the byte length to sqlite3_result_text16. Differential Revision: https://phabricator.services.mozilla.com/D26848
storage/FileSystemModule.cpp
storage/mozIStorageStatement.idl
storage/mozIStorageValueArray.idl
storage/mozStorageArgValueArray.cpp
storage/mozStorageBindingParams.cpp
storage/mozStorageConnection.cpp
storage/mozStorageRow.cpp
storage/mozStorageSQLFunctions.cpp
storage/mozStorageStatement.cpp
storage/mozStorageStatementRow.cpp
--- a/storage/FileSystemModule.cpp
+++ b/storage/FileSystemModule.cpp
@@ -169,18 +169,21 @@ int Close(sqlite3_vtab_cursor* aCursor) 
 int Filter(sqlite3_vtab_cursor* aCursor, int aIdxNum, const char* aIdxStr,
            int aArgc, sqlite3_value** aArgv) {
   VirtualTableCursor* cursor = reinterpret_cast<VirtualTableCursor*>(aCursor);
 
   if (aArgc <= 0) {
     return SQLITE_OK;
   }
 
-  nsDependentString path(
-      reinterpret_cast<const char16_t*>(::sqlite3_value_text16(aArgv[0])));
+  const char16_t* value =
+      static_cast<const char16_t*>(::sqlite3_value_text16(aArgv[0]));
+
+  nsDependentString path(value,
+                         ::sqlite3_value_bytes16(aArgv[0]) / sizeof(char16_t));
 
   nsresult rv = cursor->Init(path);
   NS_ENSURE_SUCCESS(rv, SQLITE_ERROR);
 
   return SQLITE_OK;
 }
 
 int Next(sqlite3_vtab_cursor* aCursor) {
--- a/storage/mozIStorageStatement.idl
+++ b/storage/mozIStorageStatement.idl
@@ -235,21 +235,30 @@ interface mozIStorageStatement : mozISto
    *
    * @param aIndex
    *        0-based colummn index.
    * @return true if the value for the result column is null.
    */
   boolean getIsNull(in unsigned long aIndex);
 
   /**
-   * Returns a shared string pointer
+   * Returns a shared string pointer.
+   *
+   * @param aIndex
+   *        0-based colummn index.
+   * @param aByteLength
+   *        The number of bytes in the string or blob. This is the same as the
+   *        number of characters for UTF-8 strings, and twice the number of
+   *        characters for UTF-16 strings.
+   * @param aResult
+   *        A pointer to the string or blob.
    */
-  [noscript] void getSharedUTF8String(in unsigned long aIndex, out unsigned long aLength, [shared,retval] out string aResult);
-  [noscript] void getSharedString(in unsigned long aIndex, out unsigned long aLength, [shared,retval] out wstring aResult);
-  [noscript] void getSharedBlob(in unsigned long aIndex, out unsigned long aLength, [shared,retval] out octetPtr aResult);
+  [noscript] void getSharedUTF8String(in unsigned long aIndex, out unsigned long aByteLength, [shared,retval] out string aResult);
+  [noscript] void getSharedString(in unsigned long aIndex, out unsigned long aByteLength, [shared,retval] out wstring aResult);
+  [noscript] void getSharedBlob(in unsigned long aIndex, out unsigned long aByteLength, [shared,retval] out octetPtr aResult);
 
 %{C++
   /**
    * Getters for native code that return their values as
    * the return type, for convenience and sanity.
    *
    * Not virtual; no vtable bloat.
    */
--- a/storage/mozIStorageValueArray.idl
+++ b/storage/mozIStorageValueArray.idl
@@ -62,21 +62,30 @@ interface mozIStorageValueArray : nsISup
 
   // data will be NULL if dataSize = 0
   void getBlob(in unsigned long aIndex, out unsigned long aDataSize, [array,size_is(aDataSize)] out octet aData);
   AString getBlobAsString(in unsigned long aIndex);
   AUTF8String getBlobAsUTF8String(in unsigned long aIndex);
   boolean getIsNull(in unsigned long aIndex);
 
   /**
-   * Returns a shared string pointer
+   * Returns a shared string pointer.
+   *
+   * @param aIndex
+   *        0-based colummn index.
+   * @param aByteLength
+   *        The number of bytes in the string or blob. This is the same as the
+   *        number of characters for UTF-8 strings, and twice the number of
+   *        characters for UTF-16 strings.
+   * @param aResult
+   *        A pointer to the string or blob.
    */
-  [noscript] void getSharedUTF8String(in unsigned long aIndex, out unsigned long aLength, [shared,retval] out string aResult);
-  [noscript] void getSharedString(in unsigned long aIndex, out unsigned long aLength, [shared,retval] out wstring aResult);
-  [noscript] void getSharedBlob(in unsigned long aIndex, out unsigned long aLength, [shared,retval] out octetPtr aResult);
+  [noscript] void getSharedUTF8String(in unsigned long aIndex, out unsigned long aByteLength, [shared,retval] out string aResult);
+  [noscript] void getSharedString(in unsigned long aIndex, out unsigned long aByteLength, [shared,retval] out wstring aResult);
+  [noscript] void getSharedBlob(in unsigned long aIndex, out unsigned long aByteLength, [shared,retval] out octetPtr aResult);
 
 %{C++
   /**
    * Getters for native code that return their values as
    * the return type, for convenience and sanity.
    *
    * Not virtual; no vtable bloat.
    */
--- a/storage/mozStorageArgValueArray.cpp
+++ b/storage/mozStorageArgValueArray.cpp
@@ -103,19 +103,20 @@ NS_IMETHODIMP
 ArgValueArray::GetString(uint32_t aIndex, nsAString &_value) {
   ENSURE_INDEX_VALUE(aIndex, mArgc);
 
   if (::sqlite3_value_type(mArgv[aIndex]) == SQLITE_NULL) {
     // NULL columns should have IsVoid set to distinguish them from an empty
     // string.
     _value.SetIsVoid(true);
   } else {
-    _value.Assign(
-        static_cast<const char16_t *>(::sqlite3_value_text16(mArgv[aIndex])),
-        ::sqlite3_value_bytes16(mArgv[aIndex]) / 2);
+    const char16_t *string =
+        static_cast<const char16_t *>(::sqlite3_value_text16(mArgv[aIndex]));
+    _value.Assign(string,
+                  ::sqlite3_value_bytes16(mArgv[aIndex]) / sizeof(char16_t));
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ArgValueArray::GetBlob(uint32_t aIndex, uint32_t *_size, uint8_t **_blob) {
   ENSURE_INDEX_VALUE(aIndex, mArgc);
 
@@ -143,37 +144,41 @@ ArgValueArray::GetIsNull(uint32_t aIndex
   nsresult rv = GetTypeOfIndex(aIndex, &type);
   NS_ENSURE_SUCCESS(rv, rv);
 
   *_isNull = (type == VALUE_TYPE_NULL);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-ArgValueArray::GetSharedUTF8String(uint32_t aIndex, uint32_t *_length,
+ArgValueArray::GetSharedUTF8String(uint32_t aIndex, uint32_t *_byteLength,
                                    const char **_string) {
-  if (_length) *_length = ::sqlite3_value_bytes(mArgv[aIndex]);
-
   *_string =
       reinterpret_cast<const char *>(::sqlite3_value_text(mArgv[aIndex]));
+  if (_byteLength) {
+    *_byteLength = ::sqlite3_value_bytes(mArgv[aIndex]);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
-ArgValueArray::GetSharedString(uint32_t aIndex, uint32_t *_length,
+ArgValueArray::GetSharedString(uint32_t aIndex, uint32_t *_byteLength,
                                const char16_t **_string) {
-  if (_length) *_length = ::sqlite3_value_bytes(mArgv[aIndex]);
-
   *_string =
       static_cast<const char16_t *>(::sqlite3_value_text16(mArgv[aIndex]));
+  if (_byteLength) {
+    *_byteLength = ::sqlite3_value_bytes16(mArgv[aIndex]);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
-ArgValueArray::GetSharedBlob(uint32_t aIndex, uint32_t *_size,
+ArgValueArray::GetSharedBlob(uint32_t aIndex, uint32_t *_byteLength,
                              const uint8_t **_blob) {
-  *_size = ::sqlite3_value_bytes(mArgv[aIndex]);
   *_blob = static_cast<const uint8_t *>(::sqlite3_value_blob(mArgv[aIndex]));
+  if (_byteLength) {
+    *_byteLength = ::sqlite3_value_bytes(mArgv[aIndex]);
+  }
   return NS_OK;
 }
 
 }  // namespace storage
 }  // namespace mozilla
--- a/storage/mozStorageBindingParams.cpp
+++ b/storage/mozStorageBindingParams.cpp
@@ -46,19 +46,20 @@ int sqlite3_T_double(BindingColumnData a
 }
 
 int sqlite3_T_text(BindingColumnData aData, const nsCString &aValue) {
   return ::sqlite3_bind_text(aData.stmt, aData.column + 1, aValue.get(),
                              aValue.Length(), SQLITE_TRANSIENT);
 }
 
 int sqlite3_T_text16(BindingColumnData aData, const nsString &aValue) {
-  return ::sqlite3_bind_text16(aData.stmt, aData.column + 1, aValue.get(),
-                               aValue.Length() * 2,  // Length in bytes!
-                               SQLITE_TRANSIENT);
+  return ::sqlite3_bind_text16(
+      aData.stmt, aData.column + 1, aValue.get(),
+      aValue.Length() * sizeof(char16_t),  // Length in bytes!
+      SQLITE_TRANSIENT);
 }
 
 int sqlite3_T_null(BindingColumnData aData) {
   return ::sqlite3_bind_null(aData.stmt, aData.column + 1);
 }
 
 int sqlite3_T_blob(BindingColumnData aData, const void *aBlob, int aSize) {
   return ::sqlite3_bind_blob(aData.stmt, aData.column + 1, aBlob, aSize, free);
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -134,19 +134,20 @@ int sqlite3_T_double(sqlite3_context *aC
 }
 
 int sqlite3_T_text(sqlite3_context *aCtx, const nsCString &aValue) {
   ::sqlite3_result_text(aCtx, aValue.get(), aValue.Length(), SQLITE_TRANSIENT);
   return SQLITE_OK;
 }
 
 int sqlite3_T_text16(sqlite3_context *aCtx, const nsString &aValue) {
-  ::sqlite3_result_text16(aCtx, aValue.get(),
-                          aValue.Length() * 2,  // Number of bytes.
-                          SQLITE_TRANSIENT);
+  ::sqlite3_result_text16(
+      aCtx, aValue.get(),
+      aValue.Length() * sizeof(char16_t),  // Number of bytes.
+      SQLITE_TRANSIENT);
   return SQLITE_OK;
 }
 
 int sqlite3_T_null(sqlite3_context *aCtx) {
   ::sqlite3_result_null(aCtx);
   return SQLITE_OK;
 }
 
--- a/storage/mozStorageRow.cpp
+++ b/storage/mozStorageRow.cpp
@@ -29,27 +29,29 @@ nsresult Row::initialize(sqlite3_stmt *a
     switch (type) {
       case SQLITE_INTEGER:
         variant = new IntegerVariant(::sqlite3_column_int64(aStatement, i));
         break;
       case SQLITE_FLOAT:
         variant = new FloatVariant(::sqlite3_column_double(aStatement, i));
         break;
       case SQLITE_TEXT: {
-        nsDependentString str(static_cast<const char16_t *>(
-            ::sqlite3_column_text16(aStatement, i)));
+        const char16_t *value = static_cast<const char16_t *>(
+            ::sqlite3_column_text16(aStatement, i));
+        nsDependentString str(
+            value, ::sqlite3_column_bytes16(aStatement, i) / sizeof(char16_t));
         variant = new TextVariant(str);
         break;
       }
       case SQLITE_NULL:
         variant = new NullVariant();
         break;
       case SQLITE_BLOB: {
+        const void *data = ::sqlite3_column_blob(aStatement, i);
         int size = ::sqlite3_column_bytes(aStatement, i);
-        const void *data = ::sqlite3_column_blob(aStatement, i);
         variant = new BlobVariant(std::pair<const void *, int>(data, size));
         break;
       }
       default:
         return NS_ERROR_UNEXPECTED;
     }
     NS_ENSURE_TRUE(variant, NS_ERROR_OUT_OF_MEMORY);
 
--- a/storage/mozStorageSQLFunctions.cpp
+++ b/storage/mozStorageSQLFunctions.cpp
@@ -255,27 +255,30 @@ int registerFunctions(sqlite3 *aDB) {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// SQL Functions
 
 void caseFunction(sqlite3_context *aCtx, int aArgc, sqlite3_value **aArgv) {
   NS_ASSERTION(1 == aArgc, "Invalid number of arguments!");
 
-  nsAutoString data(
-      static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[0])));
+  const char16_t *value =
+      static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[0]));
+  nsAutoString data(value,
+                    ::sqlite3_value_bytes16(aArgv[0]) / sizeof(char16_t));
   bool toUpper = ::sqlite3_user_data(aCtx) ? true : false;
 
   if (toUpper)
     ::ToUpperCase(data);
   else
     ::ToLowerCase(data);
 
   // Set the result.
-  ::sqlite3_result_text16(aCtx, data.get(), -1, SQLITE_TRANSIENT);
+  ::sqlite3_result_text16(aCtx, data.get(), data.Length() * sizeof(char16_t),
+                          SQLITE_TRANSIENT);
 }
 
 /**
  * This implements the like() SQL function.  This is used by the LIKE operator.
  * The SQL statement 'A LIKE B' is implemented as 'like(B, A)', and if there is
  * an escape character, say E, it is implemented as 'like(B, A, E)'.
  */
 void likeFunction(sqlite3_context *aCtx, int aArgc, sqlite3_value **aArgv) {
@@ -285,20 +288,25 @@ void likeFunction(sqlite3_context *aCtx,
     ::sqlite3_result_error(aCtx, "LIKE or GLOB pattern too complex",
                            SQLITE_TOOBIG);
     return;
   }
 
   if (!::sqlite3_value_text16(aArgv[0]) || !::sqlite3_value_text16(aArgv[1]))
     return;
 
-  nsDependentString A(
-      static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[1])));
-  nsDependentString B(
-      static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[0])));
+  const char16_t *a =
+      static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[1]));
+  int aLen = ::sqlite3_value_bytes16(aArgv[1]) / sizeof(char16_t);
+  nsDependentString A(a, aLen);
+
+  const char16_t *b =
+      static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[0]));
+  int bLen = ::sqlite3_value_bytes16(aArgv[0]) / sizeof(char16_t);
+  nsDependentString B(b, bLen);
   NS_ASSERTION(!B.IsEmpty(), "LIKE string must not be null!");
 
   char16_t E = 0;
   if (3 == aArgc)
     E = static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[2]))[0];
 
   nsAString::const_iterator itrString, endString;
   A.BeginReading(itrString);
@@ -316,23 +324,23 @@ void levenshteinDistanceFunction(sqlite3
 
   // If either argument is a SQL NULL, then return SQL NULL.
   if (::sqlite3_value_type(aArgv[0]) == SQLITE_NULL ||
       ::sqlite3_value_type(aArgv[1]) == SQLITE_NULL) {
     ::sqlite3_result_null(aCtx);
     return;
   }
 
-  int aLen = ::sqlite3_value_bytes16(aArgv[0]) / sizeof(char16_t);
   const char16_t *a =
       static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[0]));
+  int aLen = ::sqlite3_value_bytes16(aArgv[0]) / sizeof(char16_t);
 
-  int bLen = ::sqlite3_value_bytes16(aArgv[1]) / sizeof(char16_t);
   const char16_t *b =
       static_cast<const char16_t *>(::sqlite3_value_text16(aArgv[1]));
+  int bLen = ::sqlite3_value_bytes16(aArgv[1]) / sizeof(char16_t);
 
   // Compute the Levenshtein Distance, and return the result (or error).
   int distance = -1;
   const nsDependentString A(a, aLen);
   const nsDependentString B(b, bLen);
   int status = levenshteinDistance(A, B, &distance);
   if (status == SQLITE_OK) {
     ::sqlite3_result_int(aCtx, distance);
--- a/storage/mozStorageStatement.cpp
+++ b/storage/mozStorageStatement.cpp
@@ -665,17 +665,18 @@ Statement::GetString(uint32_t aIndex, ns
   NS_ENSURE_SUCCESS(rv, rv);
   if (type == mozIStorageStatement::VALUE_TYPE_NULL) {
     // NULL columns should have IsVoid set to distinguish them from the empty
     // string.
     _value.SetIsVoid(true);
   } else {
     const char16_t *value = static_cast<const char16_t *>(
         ::sqlite3_column_text16(mDBStatement, aIndex));
-    _value.Assign(value, ::sqlite3_column_bytes16(mDBStatement, aIndex) / 2);
+    _value.Assign(value, ::sqlite3_column_bytes16(mDBStatement, aIndex) /
+                             sizeof(char16_t));
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 Statement::GetVariant(uint32_t aIndex, nsIVariant **_value) {
   if (!mDBStatement) {
     return NS_ERROR_NOT_INITIALIZED;
@@ -695,18 +696,19 @@ Statement::GetVariant(uint32_t aIndex, n
           new IntegerVariant(::sqlite3_column_int64(mDBStatement, aIndex));
       break;
     case SQLITE_FLOAT:
       variant = new FloatVariant(::sqlite3_column_double(mDBStatement, aIndex));
       break;
     case SQLITE_TEXT: {
       const char16_t *value = static_cast<const char16_t *>(
           ::sqlite3_column_text16(mDBStatement, aIndex));
-      nsDependentString str(value,
-                            ::sqlite3_column_bytes16(mDBStatement, aIndex) / 2);
+      nsDependentString str(
+          value,
+          ::sqlite3_column_bytes16(mDBStatement, aIndex) / sizeof(char16_t));
       variant = new TextVariant(str);
       break;
     }
     case SQLITE_NULL:
       variant = new NullVariant();
       break;
     case SQLITE_BLOB: {
       int size = ::sqlite3_column_bytes(mDBStatement, aIndex);
@@ -746,41 +748,45 @@ Statement::GetBlobAsString(uint32_t aInd
 }
 
 NS_IMETHODIMP
 Statement::GetBlobAsUTF8String(uint32_t aIndex, nsACString &aValue) {
   return DoGetBlobAsString(this, aIndex, aValue);
 }
 
 NS_IMETHODIMP
-Statement::GetSharedUTF8String(uint32_t aIndex, uint32_t *_length,
+Statement::GetSharedUTF8String(uint32_t aIndex, uint32_t *_byteLength,
                                const char **_value) {
-  if (_length) *_length = ::sqlite3_column_bytes(mDBStatement, aIndex);
-
   *_value = reinterpret_cast<const char *>(
       ::sqlite3_column_text(mDBStatement, aIndex));
+  if (_byteLength) {
+    *_byteLength = ::sqlite3_column_bytes(mDBStatement, aIndex);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
-Statement::GetSharedString(uint32_t aIndex, uint32_t *_length,
+Statement::GetSharedString(uint32_t aIndex, uint32_t *_byteLength,
                            const char16_t **_value) {
-  if (_length) *_length = ::sqlite3_column_bytes16(mDBStatement, aIndex);
-
   *_value = static_cast<const char16_t *>(
       ::sqlite3_column_text16(mDBStatement, aIndex));
+  if (_byteLength) {
+    *_byteLength = ::sqlite3_column_bytes16(mDBStatement, aIndex);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
-Statement::GetSharedBlob(uint32_t aIndex, uint32_t *_size,
+Statement::GetSharedBlob(uint32_t aIndex, uint32_t *_byteLength,
                          const uint8_t **_blob) {
-  *_size = ::sqlite3_column_bytes(mDBStatement, aIndex);
   *_blob =
       static_cast<const uint8_t *>(::sqlite3_column_blob(mDBStatement, aIndex));
+  if (_byteLength) {
+    *_byteLength = ::sqlite3_column_bytes(mDBStatement, aIndex);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 Statement::GetIsNull(uint32_t aIndex, bool *_isNull) {
   // Get type of Index will check aIndex for us, so we don't have to.
   int32_t type;
   nsresult rv = GetTypeOfIndex(aIndex, &type);
--- a/storage/mozStorageStatementRow.cpp
+++ b/storage/mozStorageStatementRow.cpp
@@ -80,17 +80,18 @@ void StatementRow::NamedGetter(JSContext
       aResult.set(::JS_NumberValue(dval));
       break;
     }
     case mozIStorageValueArray::VALUE_TYPE_TEXT: {
       uint32_t bytes;
       const char16_t* sval = reinterpret_cast<const char16_t*>(
           static_cast<mozIStorageStatement*>(mStatement)
               ->AsSharedWString(idx, &bytes));
-      JSString* str = ::JS_NewUCStringCopyN(aCx, sval, bytes / 2);
+      JSString* str =
+          ::JS_NewUCStringCopyN(aCx, sval, bytes / sizeof(char16_t));
       if (!str) {
         aRv.Throw(NS_ERROR_UNEXPECTED);
         return;
       }
       aResult.setString(str);
       break;
     }
     case mozIStorageValueArray::VALUE_TYPE_BLOB: {