Bug 1576260 - Make LSValue initialization fallible; r=asuth

Differential Revision: https://phabricator.services.mozilla.com/D45849 --HG-- extra : moz-landing-system : lando
2019-09-14 05:33:01 +00:00 · 2019-09-14 05:33:01 +00:00 · 4f4188c7ab
--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@ -7913,26 +7913,12 @@ nsresult PrepareDatastoreOp::LoadDataOp::DoDatastoreWork() {
      return rv;
    }

-    nsCString buffer;
-    rv = stmt->GetUTF8String(1, buffer);
+    LSValue value;
+    rv = value.InitFromStatement(stmt, 1);
    if (NS_WARN_IF(NS_FAILED(rv))) {
      return rv;
    }

-    int32_t utf16Length;
-    rv = stmt->GetInt32(2, &utf16Length);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-
-    int32_t compressed;
-    rv = stmt->GetInt32(3, &compressed);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-
-    LSValue value(buffer, utf16Length, compressed);
-
    mPrepareDatastoreOp->mValues.Put(key, value);
    auto item = mPrepareDatastoreOp->mOrderedItems.AppendElement();
    item->key() = key;
@ -8013,11 +7999,15 @@ PrepareDatastoreOp::CompressFunction::OnFunctionCall(
  }

  nsCString compressed;
-  if (!SnappyCompress(value, compressed)) {
-    compressed = value;
+  if (NS_WARN_IF(!SnappyCompress(value, compressed))) {
+    return NS_ERROR_FAILURE;
  }

-  nsCOMPtr<nsIVariant> result = new storage::UTF8TextVariant(compressed);
+  if (!compressed.IsVoid()) {
+    value = compressed;
+  }
+
+  nsCOMPtr<nsIVariant> result = new storage::UTF8TextVariant(value);

  result.forget(aResult);
  return NS_OK;
@ -8051,7 +8041,11 @@ PrepareDatastoreOp::CompressibleFunction::OnFunctionCall(
  }

  nsCString compressed;
-  bool compressible = SnappyCompress(value, compressed);
+  if (NS_WARN_IF(!SnappyCompress(value, compressed))) {
+    return NS_ERROR_FAILURE;
+  }
+
+  bool compressible = !compressed.IsVoid();

  nsCOMPtr<nsIVariant> result = new storage::IntegerVariant(compressible);

--- a/dom/localstorage/LSSnapshot.cpp
+++ b/dom/localstorage/LSSnapshot.cpp
@ -24,7 +24,7 @@ const uint32_t kSnapshotTimeoutMs = 20000;
 * observers for other content processes.)
 */
 class SnapshotWriteOptimizer final
-    : public LSWriteOptimizer<nsAString, nsString> {
+    : public LSWriteOptimizer<LSValue> {
 public:
  void Enumerate(nsTArray<LSWriteInfo>& aWriteInfos);
 };
@ -46,7 +46,7 @@ void SnapshotWriteOptimizer::Enumerate(nsTArray<LSWriteInfo>& aWriteInfos) {

        LSSetItemInfo setItemInfo;
        setItemInfo.key() = insertItemInfo->GetKey();
-        setItemInfo.value() = LSValue(insertItemInfo->GetValue());
+        setItemInfo.value() = insertItemInfo->GetValue();

        aWriteInfos.AppendElement(std::move(setItemInfo));

@ -68,7 +68,7 @@ void SnapshotWriteOptimizer::Enumerate(nsTArray<LSWriteInfo>& aWriteInfos) {

        LSSetItemInfo setItemInfo;
        setItemInfo.key() = updateItemInfo->GetKey();
-        setItemInfo.value() = LSValue(updateItemInfo->GetValue());
+        setItemInfo.value() = updateItemInfo->GetValue();

        aWriteInfos.AppendElement(std::move(setItemInfo));

@ -312,6 +312,30 @@ nsresult LSSnapshot::SetItem(const nsAString& aKey, const nsAString& aValue,
  } else {
    changed = true;

+    auto autoRevertValue = MakeScopeExit([&] {
+      if (oldValue.IsVoid()) {
+        mValues.Remove(aKey);
+      } else {
+        mValues.Put(aKey, oldValue);
+      }
+    });
+
+    // Anything that can fail must be done early before we start modifying the
+    // state.
+
+    Maybe<LSValue> oldValueFromString;
+    if (mHasOtherProcessObservers) {
+      oldValueFromString.emplace();
+      if (NS_WARN_IF(!oldValueFromString->InitFromString(oldValue))) {
+        return NS_ERROR_FAILURE;
+      }
+    }
+
+    LSValue valueFromString;
+    if (NS_WARN_IF(!valueFromString.InitFromString(aValue))) {
+      return NS_ERROR_FAILURE;
+    }
+
    int64_t delta = static_cast<int64_t>(aValue.Length()) -
                    static_cast<int64_t>(oldValue.Length());

@ -321,11 +345,6 @@ nsresult LSSnapshot::SetItem(const nsAString& aKey, const nsAString& aValue,

    rv = UpdateUsage(delta);
    if (NS_WARN_IF(NS_FAILED(rv))) {
-      if (oldValue.IsVoid()) {
-        mValues.Remove(aKey);
-      } else {
-        mValues.Put(aKey, oldValue);
-      }
      return rv;
    }

@ -335,22 +354,25 @@ nsresult LSSnapshot::SetItem(const nsAString& aKey, const nsAString& aValue,

    if (mHasOtherProcessObservers) {
      MOZ_ASSERT(mWriteAndNotifyInfos);
+      MOZ_ASSERT(oldValueFromString.isSome());

      LSSetItemAndNotifyInfo setItemAndNotifyInfo;
      setItemAndNotifyInfo.key() = aKey;
-      setItemAndNotifyInfo.oldValue() = LSValue(oldValue);
-      setItemAndNotifyInfo.value() = LSValue(aValue);
+      setItemAndNotifyInfo.oldValue() = oldValueFromString.value();
+      setItemAndNotifyInfo.value() = valueFromString;

      mWriteAndNotifyInfos->AppendElement(std::move(setItemAndNotifyInfo));
    } else {
      MOZ_ASSERT(mWriteOptimizer);

      if (oldValue.IsVoid()) {
-        mWriteOptimizer->InsertItem(aKey, aValue);
+        mWriteOptimizer->InsertItem(aKey, valueFromString);
      } else {
-        mWriteOptimizer->UpdateItem(aKey, aValue);
+        mWriteOptimizer->UpdateItem(aKey, valueFromString);
      }
    }
+
+    autoRevertValue.release();
  }

  aNotifyInfo.changed() = changed;
@ -381,6 +403,22 @@ nsresult LSSnapshot::RemoveItem(const nsAString& aKey,
  } else {
    changed = true;

+    auto autoRevertValue = MakeScopeExit([&] {
+      MOZ_ASSERT(!oldValue.IsVoid());
+      mValues.Put(aKey, oldValue);
+    });
+
+    // Anything that can fail must be done early before we start modifying the
+    // state.
+
+    Maybe<LSValue> oldValueFromString;
+    if (mHasOtherProcessObservers) {
+      oldValueFromString.emplace();
+      if (NS_WARN_IF(!oldValueFromString->InitFromString(oldValue))) {
+        return NS_ERROR_FAILURE;
+      }
+    }
+
    int64_t delta = -(static_cast<int64_t>(aKey.Length()) +
                      static_cast<int64_t>(oldValue.Length()));

@ -393,10 +431,11 @@ nsresult LSSnapshot::RemoveItem(const nsAString& aKey,

    if (mHasOtherProcessObservers) {
      MOZ_ASSERT(mWriteAndNotifyInfos);
+      MOZ_ASSERT(oldValueFromString.isSome());

      LSRemoveItemAndNotifyInfo removeItemAndNotifyInfo;
      removeItemAndNotifyInfo.key() = aKey;
-      removeItemAndNotifyInfo.oldValue() = LSValue(oldValue);
+      removeItemAndNotifyInfo.oldValue() = oldValueFromString.value();

      mWriteAndNotifyInfos->AppendElement(std::move(removeItemAndNotifyInfo));
    } else {
@ -404,6 +443,8 @@ nsresult LSSnapshot::RemoveItem(const nsAString& aKey,

      mWriteOptimizer->DeleteItem(aKey);
    }
+
+    autoRevertValue.release();
  }

  aNotifyInfo.changed() = changed;
--- a/dom/localstorage/LSValue.cpp
+++ b/dom/localstorage/LSValue.cpp
@ -9,8 +9,71 @@
 namespace mozilla {
 namespace dom {

+bool LSValue::InitFromString(const nsAString& aBuffer) {
+  MOZ_ASSERT(mBuffer.IsVoid());
+  MOZ_ASSERT(!mUTF16Length);
+  MOZ_ASSERT(!mCompressed);
+
+  if (aBuffer.IsVoid()) {
+    return true;
+  }
+
+  nsCString converted;
+  if (NS_WARN_IF(!CopyUTF16toUTF8(aBuffer, converted, fallible))) {
+    return false;
+  }
+
+  nsCString convertedAndCompressed;
+  if (NS_WARN_IF(!SnappyCompress(converted, convertedAndCompressed))) {
+    return false;
+  }
+
+  if (convertedAndCompressed.IsVoid()) {
+    mBuffer = converted;
+    mUTF16Length = aBuffer.Length();
+  } else {
+    mBuffer = convertedAndCompressed;
+    mUTF16Length = aBuffer.Length();
+    mCompressed = true;
+  }
+
+  return true;
+}
+
+nsresult LSValue::InitFromStatement(mozIStorageStatement* aStatement,
+                                    uint32_t aIndex) {
+  MOZ_ASSERT(aStatement);
+  MOZ_ASSERT(mBuffer.IsVoid());
+  MOZ_ASSERT(!mUTF16Length);
+  MOZ_ASSERT(!mCompressed);
+
+  nsCString buffer;
+  nsresult rv = aStatement->GetUTF8String(aIndex, buffer);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  int32_t utf16Length;
+  rv = aStatement->GetInt32(aIndex + 1, &utf16Length);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  int32_t compressed;
+  rv = aStatement->GetInt32(aIndex + 2, &compressed);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  mBuffer = buffer;
+  mUTF16Length = utf16Length;
+  mCompressed = compressed;
+
+  return NS_OK;
+}
+
 const LSValue& VoidLSValue() {
-  static const LSValue sVoidLSValue(VoidCString(), 0, false);
+  static const LSValue sVoidLSValue;

  return sVoidLSValue;
 }
--- a/dom/localstorage/LSValue.h
+++ b/dom/localstorage/LSValue.h
@ -9,6 +9,8 @@

 #include "SnappyUtils.h"

+class mozIStorageStatement;
+
 namespace mozilla {
 namespace dom {

@ -29,26 +31,11 @@ class LSValue final {
  bool mCompressed;

 public:
-  LSValue() : mUTF16Length(0), mCompressed(false) {}
+  LSValue() : mUTF16Length(0), mCompressed(false) { SetIsVoid(true); }

-  explicit LSValue(const nsACString& aBuffer, uint32_t aUTF16Length,
-                   bool aCompressed)
-      : mBuffer(aBuffer),
-        mUTF16Length(aUTF16Length),
-        mCompressed(aCompressed) {}
+  bool InitFromString(const nsAString& aBuffer);

-  explicit LSValue(const nsAString& aBuffer) : mUTF16Length(aBuffer.Length()) {
-    if (aBuffer.IsVoid()) {
-      mBuffer.SetIsVoid(true);
-      mCompressed = false;
-    } else {
-      CopyUTF16toUTF8(aBuffer, mBuffer);
-      nsCString buffer;
-      if ((mCompressed = SnappyCompress(mBuffer, buffer))) {
-        mBuffer = buffer;
-      }
-    }
-  }
+  nsresult InitFromStatement(mozIStorageStatement* aStatement, uint32_t aIndex);

  bool IsVoid() const { return mBuffer.IsVoid(); }

--- a/dom/localstorage/SnappyUtils.cpp
+++ b/dom/localstorage/SnappyUtils.cpp
@ -17,21 +17,27 @@ bool SnappyCompress(const nsACString& aSource, nsACString& aDest) {
  size_t uncompressedLength = aSource.Length();

  if (uncompressedLength <= 16) {
-    return false;
+    aDest.SetIsVoid(true);
+    return true;
  }

  size_t compressedLength = snappy::MaxCompressedLength(uncompressedLength);

-  aDest.SetLength(compressedLength);
+  if (NS_WARN_IF(!aDest.SetLength(compressedLength, fallible))) {
+    return false;
+  }

  snappy::RawCompress(aSource.BeginReading(), uncompressedLength,
                      aDest.BeginWriting(), &compressedLength);

  if (compressedLength >= uncompressedLength) {
-    return false;
+    aDest.SetIsVoid(true);
+    return true;
  }

-  aDest.SetLength(compressedLength);
+  if (NS_WARN_IF(!aDest.SetLength(compressedLength, fallible))) {
+    return false;
+  }

  return true;
 }