Blink-compatible deserialization of "version 0" sparse arrays.

Version 0 dense arrays cannot be deserialized by current Chromium, which
suggests that this is not necessary.

BUG=chromium:148757

Review-Url: https://codereview.chromium.org/2256413002
Cr-Commit-Position: refs/heads/master@{#38754}

src/value-serializer.cc

@@ -717,10 +717,10 @@ void ValueDeserializer::AddObjectWithID(uint32_t id,
   }
 }
 
-static MaybeHandle<JSObject> CreateJSObjectFromKeyValuePairs(
-    Isolate* isolate, Handle<Object>* data, uint32_t num_properties) {
-  Handle<JSObject> object =
-      isolate->factory()->NewJSObject(isolate->object_function());
+static Maybe<bool> SetPropertiesFromKeyValuePairs(Isolate* isolate,
+                                                  Handle<JSObject> object,
+                                                  Handle<Object>* data,
+                                                  uint32_t num_properties) {
   for (unsigned i = 0; i < 2 * num_properties; i += 2) {
     Handle<Object> key = data[i];
     Handle<Object> value = data[i + 1];
@@ -730,10 +730,10 @@ static MaybeHandle<JSObject> CreateJSObjectFromKeyValuePairs(
     if (!success ||
         JSObject::DefineOwnPropertyIgnoreAttributes(&it, value, NONE)
             .is_null()) {
-      return MaybeHandle<JSObject>();
+      return Nothing<bool>();
     }
   }
-  return object;
+  return Just(true);
 }
 
 MaybeHandle<Object>
@@ -759,17 +759,51 @@ ValueDeserializer::ReadObjectUsingEntireBufferForLegacyFormat() {
           return MaybeHandle<Object>();
         }
 
-        size_t begin_properties = stack.size() - 2 * num_properties;
-        Handle<Object>* data =
-            num_properties ? &stack[begin_properties] : nullptr;
-        if (!CreateJSObjectFromKeyValuePairs(isolate_, data, num_properties)
-                 .ToHandle(&new_object)) {
+        size_t begin_properties =
+            stack.size() - 2 * static_cast<size_t>(num_properties);
+        Handle<JSObject> js_object =
+            isolate_->factory()->NewJSObject(isolate_->object_function());
+        if (num_properties &&
+            !SetPropertiesFromKeyValuePairs(
+                 isolate_, js_object, &stack[begin_properties], num_properties)
+                 .FromMaybe(false)) {
+          return MaybeHandle<Object>();
+        }
+
+        stack.resize(begin_properties);
+        new_object = js_object;
+        break;
+      }
+      case SerializationTag::kEndSparseJSArray: {
+        ConsumeTag(SerializationTag::kEndSparseJSArray);
+
+        // Sparse JS Array: Read the last 2*|num_properties| from the stack.
+        uint32_t num_properties;
+        uint32_t length;
+        if (!ReadVarint<uint32_t>().To(&num_properties) ||
+            !ReadVarint<uint32_t>().To(&length) ||
+            stack.size() / 2 < num_properties) {
+          return MaybeHandle<Object>();
+        }
+
+        Handle<JSArray> js_array = isolate_->factory()->NewJSArray(0);
+        JSArray::SetLength(js_array, length);
+        size_t begin_properties =
+            stack.size() - 2 * static_cast<size_t>(num_properties);
+        if (num_properties &&
+            !SetPropertiesFromKeyValuePairs(
+                 isolate_, js_array, &stack[begin_properties], num_properties)
+                 .FromMaybe(false)) {
           return MaybeHandle<Object>();
         }
 
         stack.resize(begin_properties);
+        new_object = js_array;
         break;
       }
+      case SerializationTag::kEndDenseJSArray:
+        // This was already broken in Chromium, and apparently wasn't missed.
+        return MaybeHandle<Object>();
       default:
         if (!ReadObject().ToHandle(&new_object)) return MaybeHandle<Object>();
         break;

test/unittests/value-serializer-unittest.cc

@@ -1039,5 +1039,39 @@ TEST_F(ValueSerializerTest, RoundTripArrayWithTrickyGetters) {
       });
 }
 
+TEST_F(ValueSerializerTest, DecodeSparseArrayVersion0) {
+  // Empty (sparse) array.
+  DecodeTestForVersion0({0x40, 0x00, 0x00, 0x00},
+                        [this](Local<Value> value) {
+                          ASSERT_TRUE(value->IsArray());
+                          ASSERT_EQ(0, Array::Cast(*value)->Length());
+                        });
+  // Sparse array with a mixture of elements and properties.
+  DecodeTestForVersion0(
+      {0x55, 0x00, 0x53, 0x01, 'a',  0x55, 0x02, 0x55, 0x05, 0x53,
+       0x03, 'f',  'o',  'o',  0x53, 0x03, 'b',  'a',  'r',  0x53,
+       0x03, 'b',  'a',  'z',  0x49, 0x0b, 0x40, 0x04, 0x03, 0x00},
+      [this](Local<Value> value) {
+        ASSERT_TRUE(value->IsArray());
+        EXPECT_EQ(3, Array::Cast(*value)->Length());
+        EXPECT_TRUE(
+            EvaluateScriptForResultBool("result.toString() === 'a,,5'"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("!(1 in result)"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.foo === 'bar'"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result.baz === -6"));
+      });
+  // Sparse array in a sparse array (sanity check of nesting).
+  DecodeTestForVersion0(
+      {0x55, 0x01, 0x55, 0x01, 0x54, 0x40, 0x01, 0x02, 0x40, 0x01, 0x02, 0x00},
+      [this](Local<Value> value) {
+        ASSERT_TRUE(value->IsArray());
+        EXPECT_EQ(2, Array::Cast(*value)->Length());
+        EXPECT_TRUE(EvaluateScriptForResultBool("!(0 in result)"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[1] instanceof Array"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("!(0 in result[1])"));
+        EXPECT_TRUE(EvaluateScriptForResultBool("result[1][1] === true"));
+      });
+}
+
 }  // namespace
 }  // namespace v8