String::NewExternal should not crash the renderer.

The blink may pass a very long string to v8
for making a handle to it. v8 has max string length
limit and creates exception. But NewExternal code does not
check that the handle is null and crashes the renderer.

With the fix the js code receives the exception.

BUG=414615
LOG=N
R=yangguo@chromium.org, yurys@chromium.org

Review URL: https://codereview.chromium.org/603393004

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24250 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/api.cc b/src/api.cc
index f1d81dd..bda34bd 100644 (file)
--- a/src/api.cc
+++ b/src/api.cc
@@ -5483,21 +5483,15 @@ Local<String> v8::String::Concat(Handle<String> left, Handle<String> right) {
 }
 
 
-static i::Handle<i::String> NewExternalStringHandle(
-    i::Isolate* isolate,
-    v8::String::ExternalStringResource* resource) {
-  // We do not expect this to fail. Change this if it does.
-  return isolate->factory()->NewExternalStringFromTwoByte(
-      resource).ToHandleChecked();
+static i::MaybeHandle<i::String> NewExternalStringHandle(
+    i::Isolate* isolate, v8::String::ExternalStringResource* resource) {
+  return isolate->factory()->NewExternalStringFromTwoByte(resource);
 }
 
 
-static i::Handle<i::String> NewExternalOneByteStringHandle(
+static i::MaybeHandle<i::String> NewExternalOneByteStringHandle(
     i::Isolate* isolate, v8::String::ExternalOneByteStringResource* resource) {
-  // We do not expect this to fail. Change this if it does.
-  return isolate->factory()
-      ->NewExternalStringFromOneByte(resource)
-      .ToHandleChecked();
+  return isolate->factory()->NewExternalStringFromOneByte(resource);
 }
 
 
@@ -5508,9 +5502,13 @@ Local<String> v8::String::NewExternal(
   LOG_API(i_isolate, "String::NewExternal");
   ENTER_V8(i_isolate);
   CHECK(resource && resource->data());
-  i::Handle<i::String> result = NewExternalStringHandle(i_isolate, resource);
-  i_isolate->heap()->external_string_table()->AddString(*result);
-  return Utils::ToLocal(result);
+  EXCEPTION_PREAMBLE(i_isolate);
+  i::Handle<i::String> string;
+  has_pending_exception =
+      !NewExternalStringHandle(i_isolate, resource).ToHandle(&string);
+  EXCEPTION_BAILOUT_CHECK(i_isolate, Local<String>());
+  i_isolate->heap()->external_string_table()->AddString(*string);
+  return Utils::ToLocal(string);
 }
 
 
@@ -5546,10 +5544,13 @@ Local<String> v8::String::NewExternal(
   LOG_API(i_isolate, "String::NewExternal");
   ENTER_V8(i_isolate);
   CHECK(resource && resource->data());
-  i::Handle<i::String> result =
-      NewExternalOneByteStringHandle(i_isolate, resource);
-  i_isolate->heap()->external_string_table()->AddString(*result);
-  return Utils::ToLocal(result);
+  EXCEPTION_PREAMBLE(i_isolate);
+  i::Handle<i::String> string;
+  has_pending_exception =
+      !NewExternalOneByteStringHandle(i_isolate, resource).ToHandle(&string);
+  EXCEPTION_BAILOUT_CHECK(i_isolate, Local<String>());
+  i_isolate->heap()->external_string_table()->AddString(*string);
+  return Utils::ToLocal(string);
 }
 
 

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 8fd8743..b4cf5cf 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -742,6 +742,53 @@ THREADED_TEST(UsingExternalOneByteString) {
 }
 
 
+class DummyResource : public v8::String::ExternalStringResource {
+ public:
+  virtual const uint16_t* data() const { return string_; }
+  virtual size_t length() const { return 1 << 30; }
+
+ private:
+  uint16_t string_[10];
+};
+
+
+class DummyOneByteResource : public v8::String::ExternalOneByteStringResource {
+ public:
+  virtual const char* data() const { return string_; }
+  virtual size_t length() const { return 1 << 30; }
+
+ private:
+  char string_[10];
+};
+
+
+THREADED_TEST(NewExternalForVeryLongString) {
+  {
+    LocalContext env;
+    v8::HandleScope scope(env->GetIsolate());
+    v8::TryCatch try_catch;
+    DummyOneByteResource r;
+    v8::Local<v8::String> str = v8::String::NewExternal(CcTest::isolate(), &r);
+    CHECK(str.IsEmpty());
+    CHECK(try_catch.HasCaught());
+    String::Utf8Value exception_value(try_catch.Exception());
+    CHECK_EQ("RangeError: Invalid string length", *exception_value);
+  }
+
+  {
+    LocalContext env;
+    v8::HandleScope scope(env->GetIsolate());
+    v8::TryCatch try_catch;
+    DummyResource r;
+    v8::Local<v8::String> str = v8::String::NewExternal(CcTest::isolate(), &r);
+    CHECK(str.IsEmpty());
+    CHECK(try_catch.HasCaught());
+    String::Utf8Value exception_value(try_catch.Exception());
+    CHECK_EQ("RangeError: Invalid string length", *exception_value);
+  }
+}
+
+
 THREADED_TEST(ScavengeExternalString) {
   i::FLAG_stress_compaction = false;
   i::FLAG_gc_global = false;