Graceful handling of new versions of IndexedDB serialized data.

third_party/WebKit/Source/bindings/modules/v8/V8BindingForModulesTest.cpp

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *     notice, this list of conditions and the following disclaimer in the
  *     documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
  * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
  * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
  * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
  * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
  * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "bindings/modules/v8/V8BindingForModules.h"
 
+#include "bindings/core/v8/SerializationTag.h"
+#include "bindings/core/v8/SerializedScriptValue.h"
 #include "bindings/core/v8/ToV8.h"
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8BindingForTesting.h"
 #include "bindings/core/v8/V8PerIsolateData.h"
 #include "bindings/modules/v8/ToV8ForModules.h"
+#include "modules/indexeddb/IDBAny.h"
 #include "modules/indexeddb/IDBKey.h"
 #include "modules/indexeddb/IDBKeyPath.h"
+#include "modules/indexeddb/IDBValue.h"
+#include "platform/SharedBuffer.h"
+#include "public/platform/WebBlobInfo.h"
+#include "public/platform/WebData.h"
+#include "public/platform/WebString.h"
+#include "public/platform/modules/indexeddb/WebIDBKey.h"
+#include "public/platform/modules/indexeddb/WebIDBKeyPath.h"
+#include "public/platform/modules/indexeddb/WebIDBValue.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace blink {
 
 namespace {
 
 IDBKey* checkKeyFromValueAndKeyPathInternal(v8::Isolate* isolate,
                                             const ScriptValue& value,
                                             const String& keyPath) {
   IDBKeyPath idbKeyPath(keyPath);
@@ skipping 64 matching lines @@
 void checkKeyPathNumberValue(v8::Isolate* isolate,
                              const ScriptValue& value,
                              const String& keyPath,
                              int expected) {
   IDBKey* idbKey = checkKeyFromValueAndKeyPathInternal(isolate, value, keyPath);
   ASSERT_TRUE(idbKey);
   ASSERT_EQ(IDBKey::NumberType, idbKey->getType());
   ASSERT_TRUE(expected == idbKey->number());
 }
 
+void serializeEmptyObject(v8::Isolate* isolate, Vector<char>* wireBytes) {
+  v8::Local<v8::Object> object = v8::Object::New(isolate);
+  ASSERT_TRUE(object->IsObject());
+  NonThrowableExceptionState nonThrowableExceptionState;
+
+  SerializedScriptValue::SerializeOptions options;
+  RefPtr<SerializedScriptValue> serializedValue =
+      SerializedScriptValue::serialize(isolate, object, options,
+                                       nonThrowableExceptionState);
+  serializedValue->toWireBytes(*wireBytes);
+
+  // Sanity check that the serialization header has not changed, as the tests
+  // that use this method rely on the header format.
+  //
+  // The cast from char* to unsigned char* is necessary to avoid VS2015 warning
+  // C4309 (truncation of constant value). This happens because VersionTag is
+  // 0xFF.
+  const unsigned char* wireData =
+      reinterpret_cast<unsigned char*>(wireBytes->data());
+  ASSERT_EQ(
+      static_cast<unsigned char>(SerializedScriptValue::wireFormatVersion),
+      wireData[0]);

[jsbell, 2017/04/03 19:31:52]

Can you make the offsets (0...3) consts somewhere?

[pwnall, 2017/04/04 00:55:12]

Done.

+  ASSERT_EQ(static_cast<unsigned char>(VersionTag), wireData[1]);
+
+  // 13 is v8::internal::kLatestVersion in v8/src/value-serializer.cc at the

[jsbell, 2017/04/03 19:31:52]

Define this as a const somewhere?

[pwnall, 2017/04/04 00:55:12]

Done.

+  // time when this test was written.
+  ASSERT_GE(static_cast<unsigned char>(13), wireData[2]);
+  ASSERT_EQ(static_cast<unsigned char>(VersionTag), wireData[3]);
+}
+
+PassRefPtr<IDBValue> createIDBValue(v8::Isolate* isolate,
+                                    Vector<char>& wireBytes,
+                                    double primaryKey,
+                                    const WebString& keyPath) {
+  DCHECK_GE(wireBytes.size(), 5UL);
+  WebData webData(SharedBuffer::adoptVector(wireBytes));
+  DCHECK_GE(webData.size(), 5UL);
+  Vector<WebBlobInfo> webBlobInfo;
+  WebIDBKey webIdbKey = WebIDBKey::createNumber(42.0);
+  WebIDBKeyPath webIdbKeyPath(WebString("foo"));
+  WebIDBValue webIdbValue(webData, webBlobInfo, webIdbKey, webIdbKeyPath);
+  DCHECK_GE(webIdbValue.data.size(), 5UL);
+  return IDBValue::create(webIdbValue, isolate);
+}
+
 TEST(IDBKeyFromValueAndKeyPathTest, TopLevelPropertyStringValue) {
   V8TestingScope scope;
   v8::Isolate* isolate = scope.isolate();
 
   // object = { foo: "zoo" }
   v8::Local<v8::Object> object = v8::Object::New(isolate);
   ASSERT_TRUE(
       v8CallBoolean(object->Set(scope.context(), v8AtomicString(isolate, "foo"),
                                 v8AtomicString(isolate, "zoo"))));
 
@@ skipping 102 matching lines @@
                  IDBKey::createArray(IDBKey::KeyArray()), scriptObject,
                  "foo.baz");
   checkInjection(scope.getScriptState(),
                  IDBKey::createArray(IDBKey::KeyArray()), scriptObject, "bar");
 
   checkInjectionDisallowed(scope.getScriptState(), scriptObject, "foo.bar.baz");
   checkInjection(scope.getScriptState(), IDBKey::createString("zoo"),
                  scriptObject, "foo.xyz.foo");
 }
 
+TEST(DeserializeIDBValueTest, CurrentVersions) {
+  V8TestingScope scope;
+  v8::Isolate* isolate = scope.isolate();
+
+  Vector<char> objectBytes;
+  serializeEmptyObject(isolate, &objectBytes);
+  RefPtr<IDBValue> idbValue = createIDBValue(isolate, objectBytes, 42.0, "foo");
+
+  v8::Local<v8::Value> v8Value =
+      deserializeIDBValue(isolate, scope.context()->Global(), idbValue.get());
+  EXPECT_TRUE(!scope.getExceptionState().hadException());
+
+  ASSERT_TRUE(v8Value->IsObject());
+  v8::Local<v8::Object> v8ValueObject = v8Value.As<v8::Object>();
+  v8::Local<v8::Value> v8NumberValue =
+      v8ValueObject->Get(scope.context(), v8AtomicString(isolate, "foo"))
+          .ToLocalChecked();
+  ASSERT_TRUE(v8NumberValue->IsNumber());
+  v8::Local<v8::Number> v8Number = v8NumberValue.As<v8::Number>();
+  EXPECT_EQ(v8Number->Value(), 42.0);
+}
+
+TEST(DeserializeIDBValueTest, FutureV8Version) {
+  V8TestingScope scope;
+  v8::Isolate* isolate = scope.isolate();
+
+  // These steps reproduce the circumstances of http://crbug.com/703704

[jsbell, 2017/04/03 19:31:52]

nit: prefer https

Also, I'd put the explanation inline rather than just linking to the bug.

[pwnall, 2017/04/04 00:55:12]

Done. (the https part)

The explanation was getting rather long, so I distributed it across comments for
the functions in V8BindingForModules.cpp. Does this look reasonable?

+  Vector<char> objectBytes;
+  serializeEmptyObject(isolate, &objectBytes);
+  objectBytes[2] += 1;

[jsbell, 2017/04/03 19:31:52]

Explain what this is doing. (Having the offset be a const will help)

[pwnall, 2017/04/04 00:55:12]

Done.

+  RefPtr<IDBValue> idbValue = createIDBValue(isolate, objectBytes, 42.0, "foo");
+
+  v8::Local<v8::Value> v8Value =
+      deserializeIDBValue(isolate, scope.context()->Global(), idbValue.get());
+  // The exception check below will need to be changed if we throw exceptions

[jsbell, 2017/04/03 19:31:52]

I don't think you need this comment - it's implicit in the EXPECT (and we'll
notice c/o test failures)

[pwnall, 2017/04/04 00:55:12]

Done.

+  // instead of returning null on deserialization errors.
+  EXPECT_TRUE(!scope.getExceptionState().hadException());
+  EXPECT_TRUE(v8Value->IsNull());
+}
+
 }  // namespace blink