Handle errors in v8::ValueDeserializer by throwing exceptions.

This restores the contract that all API methods that return Maybe<T> or
MaybeLocal<T> always throw an exception when they return nothing.

Since v8::ValueDeserializer::ReadHeader can now throw exceptions, it
needs a Local<Context> parameter so that it can set up execution state
(entering the context, etc.). The old method has been marked for
deprecation, but since this API is experimental I intend to remove it
as soon as I've removed the use from Blink.

value-serializer-unittest has been updated to expect an exception in
all decode failure cases.

BUG=chromium:148757,chromium:641964

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

include/v8.h

@@ -1743,7 +1743,8 @@ class V8_EXPORT ValueDeserializer {
    * Reads and validates a header (including the format version).
    * May, for example, reject an invalid or unsupported wire format.
    */
-  V8_WARN_UNUSED_RESULT Maybe<bool> ReadHeader();
+  V8_WARN_UNUSED_RESULT Maybe<bool> ReadHeader(Local<Context> context);
+  V8_DEPRECATE_SOON("Use Local<Context> version", Maybe<bool> ReadHeader());
 
   /*
    * Deserializes a JavaScript value from the buffer.

src/api.cc

@@ -2855,10 +2855,8 @@ Maybe<bool> ValueSerializer::WriteValue(Local<Context> context,
   PREPARE_FOR_EXECUTION_PRIMITIVE(context, ValueSerializer, WriteValue, bool);
   i::Handle<i::Object> object = Utils::OpenHandle(*value);
   Maybe<bool> result = private_->serializer.WriteObject(object);
-  if (result.IsNothing()) {
-    has_pending_exception = private_->isolate->has_pending_exception();
-    RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
-  }
+  has_pending_exception = result.IsNothing();
+  RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
   return result;
 }
 
@@ -2883,6 +2881,7 @@ struct ValueDeserializer::PrivateData {
       : isolate(i), deserializer(i, data) {}
   i::Isolate* isolate;
   i::ValueDeserializer deserializer;
+  bool has_aborted = false;
   bool supports_legacy_wire_format = false;
 };
 
@@ -2893,38 +2892,61 @@ ValueDeserializer::ValueDeserializer(Isolate* isolate, const uint8_t* data,
         new PrivateData(reinterpret_cast<i::Isolate*>(isolate),
                         i::Vector<const uint8_t>(data, static_cast<int>(size)));
   } else {
-    private_ = nullptr;
+    private_ = new PrivateData(reinterpret_cast<i::Isolate*>(isolate),
+                               i::Vector<const uint8_t>(nullptr, 0));
+    private_->has_aborted = true;
   }
 }
 
 ValueDeserializer::~ValueDeserializer() { delete private_; }
 
-Maybe<bool> ValueDeserializer::ReadHeader() {
+Maybe<bool> ValueDeserializer::ReadHeader(Local<Context> context) {
+  PREPARE_FOR_EXECUTION_PRIMITIVE(context, ValueDeserializer, ReadHeader, bool);
+
+  // We could have aborted during the constructor.
+  // If so, ReadHeader is where we report it.
+  if (private_->has_aborted) {
+    isolate->Throw(*isolate->factory()->NewError(
+        i::MessageTemplate::kDataCloneDeserializationError));
+    has_pending_exception = true;
+    RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
+  }
+
+  bool read_header = false;
+  has_pending_exception = !private_->deserializer.ReadHeader().To(&read_header);
+  RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
+  DCHECK(read_header);
+
   // TODO(jbroman): Today, all wire formats are "legacy". When a more supported
   // format is added, compare the version of the internal serializer to the
   // minimum non-legacy version number.
-  if (!private_ || !private_->deserializer.ReadHeader().FromMaybe(false) ||
-      !private_->supports_legacy_wire_format) {
-    delete private_;
-    private_ = nullptr;
-    return Nothing<bool>();
+  if (!private_->supports_legacy_wire_format) {
+    isolate->Throw(*isolate->factory()->NewError(
+        i::MessageTemplate::kDataCloneDeserializationVersionError));
+    has_pending_exception = true;
+    RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
   }
+
   return Just(true);
 }
 
+Maybe<bool> ValueDeserializer::ReadHeader() {
+  Isolate* isolate = reinterpret_cast<Isolate*>(private_->isolate);
+  return ReadHeader(isolate->GetEnteredContext());
+}
+
 void ValueDeserializer::SetSupportsLegacyWireFormat(
     bool supports_legacy_wire_format) {
-  if (!private_) return;
   private_->supports_legacy_wire_format = supports_legacy_wire_format;
 }
 
 uint32_t ValueDeserializer::GetWireFormatVersion() const {
-  CHECK(private_);
+  CHECK(!private_->has_aborted);
   return private_->deserializer.GetWireFormatVersion();
 }
 
 MaybeLocal<Value> ValueDeserializer::ReadValue(Local<Context> context) {
-  CHECK(private_);
+  CHECK(!private_->has_aborted);
   PREPARE_FOR_EXECUTION(context, ValueDeserializer, ReadValue, Value);
   i::MaybeHandle<i::Object> result;
   if (GetWireFormatVersion() > 0) {
@@ -2934,22 +2956,21 @@ MaybeLocal<Value> ValueDeserializer::ReadValue(Local<Context> context) {
         private_->deserializer.ReadObjectUsingEntireBufferForLegacyFormat();
   }
   Local<Value> value;
-  if (!ToLocal(result, &value)) {
-    has_pending_exception = private_->isolate->has_pending_exception();
-    RETURN_ON_FAILED_EXECUTION(Value);
-    return MaybeLocal<Value>();
-  }
+  has_pending_exception = !ToLocal(result, &value);
+  RETURN_ON_FAILED_EXECUTION(Value);
   RETURN_ESCAPED(value);
 }
 
 void ValueDeserializer::TransferArrayBuffer(uint32_t transfer_id,
                                             Local<ArrayBuffer> array_buffer) {
+  CHECK(!private_->has_aborted);
   private_->deserializer.TransferArrayBuffer(transfer_id,
                                              Utils::OpenHandle(*array_buffer));
 }
 
 void ValueDeserializer::TransferSharedArrayBuffer(
     uint32_t transfer_id, Local<SharedArrayBuffer> shared_array_buffer) {
+  CHECK(!private_->has_aborted);
   private_->deserializer.TransferArrayBuffer(
       transfer_id, Utils::OpenHandle(*shared_array_buffer));
 }

src/counters.h

@@ -661,6 +661,7 @@ class RuntimeCallTimer {
   V(UnboundScript_GetSourceMappingURL)                     \
   V(UnboundScript_GetSourceURL)                            \
   V(Value_TypeOf)                                          \
+  V(ValueDeserializer_ReadHeader)                          \
   V(ValueDeserializer_ReadValue)                           \
   V(ValueSerializer_WriteValue)
 

src/messages.h

@@ -641,7 +641,11 @@ class ErrorUtils : public AllStatic {
     "An ArrayBuffer is neutered and could not be cloned.")                     \
   T(DataCloneErrorSharedArrayBufferNotTransferred,                             \
     "A SharedArrayBuffer could not be cloned. SharedArrayBuffer must be "      \
-    "transferred.")
+    "transferred.")                                                            \
+  T(DataCloneDeserializationError, "Unable to deserialize cloned data.")       \
+  T(DataCloneDeserializationVersionError,                                      \
+    "Unable to deserialize cloned data due to invalid or unsupported "         \
+    "version.")
 
 class MessageTemplate {
  public:

src/value-serializer.cc

@@ -682,8 +682,11 @@ Maybe<bool> ValueDeserializer::ReadHeader() {
   if (position_ < end_ &&
       *position_ == static_cast<uint8_t>(SerializationTag::kVersion)) {
     ReadTag().ToChecked();
-    if (!ReadVarint<uint32_t>().To(&version_)) return Nothing<bool>();
-    if (version_ > kLatestVersion) return Nothing<bool>();
+    if (!ReadVarint<uint32_t>().To(&version_) || version_ > kLatestVersion) {
+      isolate_->Throw(*isolate_->factory()->NewError(
+          MessageTemplate::kDataCloneDeserializationVersionError));
+      return Nothing<bool>();
+    }
   }
   return Just(true);
 }
@@ -804,6 +807,11 @@ MaybeHandle<Object> ValueDeserializer::ReadObject() {
     result = ReadJSArrayBufferView(Handle<JSArrayBuffer>::cast(object));
   }
 
+  if (result.is_null() && !isolate_->has_pending_exception()) {
+    isolate_->Throw(*isolate_->factory()->NewError(
+        MessageTemplate::kDataCloneDeserializationError));
+  }
+
   return result;
 }
 
@@ -1299,8 +1307,7 @@ static Maybe<bool> SetPropertiesFromKeyValuePairs(Isolate* isolate,
 
 MaybeHandle<Object>
 ValueDeserializer::ReadObjectUsingEntireBufferForLegacyFormat() {
-  if (version_ > 0) return MaybeHandle<Object>();
-
+  DCHECK_EQ(version_, 0);
   HandleScope scope(isolate_);
   std::vector<Handle<Object>> stack;
   while (position_ < end_) {
@@ -1362,9 +1369,12 @@ ValueDeserializer::ReadObjectUsingEntireBufferForLegacyFormat() {
         new_object = js_array;
         break;
       }
-      case SerializationTag::kEndDenseJSArray:
+      case SerializationTag::kEndDenseJSArray: {
         // This was already broken in Chromium, and apparently wasn't missed.
+        isolate_->Throw(*isolate_->factory()->NewError(
+            MessageTemplate::kDataCloneDeserializationError));
         return MaybeHandle<Object>();
+      }
       default:
         if (!ReadObject().ToHandle(&new_object)) return MaybeHandle<Object>();
         break;
@@ -1380,7 +1390,11 @@ ValueDeserializer::ReadObjectUsingEntireBufferForLegacyFormat() {
 #endif
   position_ = end_;
 
-  if (stack.size() != 1) return MaybeHandle<Object>();
+  if (stack.size() != 1) {
+    isolate_->Throw(*isolate_->factory()->NewError(
+        MessageTemplate::kDataCloneDeserializationError));
+    return MaybeHandle<Object>();
+  }
   return scope.CloseAndEscape(stack[0]);
 }
 

test/unittests/value-serializer-unittest.cc

@@ -96,7 +96,7 @@ class ValueSerializerTest : public TestWithIsolate {
                                    static_cast<int>(data.size()));
     deserializer.SetSupportsLegacyWireFormat(true);
     BeforeDecode(&deserializer);
-    ASSERT_TRUE(deserializer.ReadHeader().FromMaybe(false));
+    ASSERT_TRUE(deserializer.ReadHeader(context).FromMaybe(false));
     Local<Value> result;
     ASSERT_TRUE(deserializer.ReadValue(context).ToLocal(&result));
     ASSERT_FALSE(result.IsEmpty());
@@ -119,7 +119,7 @@ class ValueSerializerTest : public TestWithIsolate {
                                    static_cast<int>(data.size()));
     deserializer.SetSupportsLegacyWireFormat(true);
     BeforeDecode(&deserializer);
-    ASSERT_TRUE(deserializer.ReadHeader().FromMaybe(false));
+    ASSERT_TRUE(deserializer.ReadHeader(context).FromMaybe(false));
     ASSERT_EQ(0, deserializer.GetWireFormatVersion());
     Local<Value> result;
     ASSERT_TRUE(deserializer.ReadValue(context).ToLocal(&result));
@@ -141,10 +141,14 @@ class ValueSerializerTest : public TestWithIsolate {
                                    static_cast<int>(data.size()));
     deserializer.SetSupportsLegacyWireFormat(true);
     BeforeDecode(&deserializer);
-    Maybe<bool> header_result = deserializer.ReadHeader();
-    if (header_result.IsNothing()) return;
+    Maybe<bool> header_result = deserializer.ReadHeader(context);
+    if (header_result.IsNothing()) {
+      EXPECT_TRUE(try_catch.HasCaught());
+      return;
+    }
     ASSERT_TRUE(header_result.ToChecked());
     ASSERT_TRUE(deserializer.ReadValue(context).IsEmpty());
+    EXPECT_TRUE(try_catch.HasCaught());
   }
 
   Local<Value> EvaluateScriptForInput(const char* utf8_source) {