Prevent fatal error in CFX_V8::PutObjectProperty()
If there is a user-supplied setter that throws, then
PutObjectProperty() will call v8::FromJust() on a Nothing, and
#
# Fatal error in v8::FromJust
# Maybe value is Nothing.
#
results.
- Make CFX_V8 return bool values as success indicators.
- Cover some empty object cases in cfx_v8_unittests.cpp
- Use a local to avoid clunky formatting.
Change-Id: I4ef5f38db4a1ad74491349211dd3b3c6a745bbef
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/62970
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/cfx_v8.cpp b/fxjs/cfx_v8.cpp
index 59157fe..72bf361 100644
--- a/fxjs/cfx_v8.cpp
+++ b/fxjs/cfx_v8.cpp
@@ -43,15 +43,15 @@
return result;
}
-void CFX_V8::PutObjectProperty(v8::Local<v8::Object> pObj,
+bool CFX_V8::PutObjectProperty(v8::Local<v8::Object> pObj,
ByteStringView bsUTF8PropertyName,
v8::Local<v8::Value> pPut) {
ASSERT(!pPut.IsEmpty());
if (pObj.IsEmpty())
- return;
- pObj->Set(m_pIsolate->GetCurrentContext(), NewString(bsUTF8PropertyName),
- pPut)
- .FromJust();
+ return false;
+
+ v8::Local<v8::String> name = NewString(bsUTF8PropertyName);
+ return pObj->Set(m_pIsolate->GetCurrentContext(), name, pPut).IsJust();
}
void CFX_V8::DisposeIsolate() {
@@ -67,15 +67,13 @@
return v8::Object::New(GetIsolate());
}
-unsigned CFX_V8::PutArrayElement(v8::Local<v8::Array> pArray,
- unsigned index,
- v8::Local<v8::Value> pValue) {
+bool CFX_V8::PutArrayElement(v8::Local<v8::Array> pArray,
+ unsigned index,
+ v8::Local<v8::Value> pValue) {
ASSERT(!pValue.IsEmpty());
if (pArray.IsEmpty())
- return 0;
- if (pArray->Set(m_pIsolate->GetCurrentContext(), index, pValue).IsNothing())
- return 0;
- return 1;
+ return false;
+ return pArray->Set(m_pIsolate->GetCurrentContext(), index, pValue).IsJust();
}
v8::Local<v8::Value> CFX_V8::GetArrayElement(v8::Local<v8::Array> pArray,
diff --git a/fxjs/cfx_v8.h b/fxjs/cfx_v8.h
index 91a6851..cb152ac 100644
--- a/fxjs/cfx_v8.h
+++ b/fxjs/cfx_v8.h
@@ -44,15 +44,15 @@
unsigned GetArrayLength(v8::Local<v8::Array> pArray);
v8::Local<v8::Value> GetArrayElement(v8::Local<v8::Array> pArray,
unsigned index);
- unsigned PutArrayElement(v8::Local<v8::Array> pArray,
- unsigned index,
- v8::Local<v8::Value> pValue);
+ bool PutArrayElement(v8::Local<v8::Array> pArray,
+ unsigned index,
+ v8::Local<v8::Value> pValue);
// Objects.
std::vector<WideString> GetObjectPropertyNames(v8::Local<v8::Object> pObj);
v8::Local<v8::Value> GetObjectProperty(v8::Local<v8::Object> pObj,
ByteStringView bsUTF8PropertyName);
- void PutObjectProperty(v8::Local<v8::Object> pObj,
+ bool PutObjectProperty(v8::Local<v8::Object> pObj,
ByteStringView bsUTF8PropertyName,
v8::Local<v8::Value> pValue);
diff --git a/fxjs/cfx_v8_unittest.cpp b/fxjs/cfx_v8_unittest.cpp
index 54f4d92..7b9ecb5 100644
--- a/fxjs/cfx_v8_unittest.cpp
+++ b/fxjs/cfx_v8_unittest.cpp
@@ -10,6 +10,11 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/base/ptr_util.h"
+namespace {
+bool getter_sentinel = false;
+bool setter_sentinel = false;
+} // namespace
+
void FXV8UnitTest::V8IsolateDeleter::operator()(v8::Isolate* ptr) const {
ptr->Dispose();
}
@@ -41,6 +46,19 @@
EXPECT_EQ(L"", cfx_v8()->ToWideString(empty));
EXPECT_TRUE(cfx_v8()->ToObject(empty).IsEmpty());
EXPECT_TRUE(cfx_v8()->ToArray(empty).IsEmpty());
+
+ // Can't set properties on empty objects, but does not fault.
+ v8::Local<v8::Value> marker = cfx_v8()->NewNumber(2);
+ v8::Local<v8::Object> empty_object;
+ EXPECT_FALSE(cfx_v8()->PutObjectProperty(empty_object, "clams", marker));
+ EXPECT_TRUE(cfx_v8()->GetObjectProperty(empty_object, "clams").IsEmpty());
+ EXPECT_EQ(0u, cfx_v8()->GetObjectPropertyNames(empty_object).size());
+
+ // Can't set elements in empty arrays, but does not fault.
+ v8::Local<v8::Array> empty_array;
+ EXPECT_FALSE(cfx_v8()->PutArrayElement(empty_array, 0, marker));
+ EXPECT_TRUE(cfx_v8()->GetArrayElement(empty_array, 0).IsEmpty());
+ EXPECT_EQ(0u, cfx_v8()->GetArrayLength(empty_array));
}
TEST_F(FXV8UnitTest, NewNull) {
@@ -162,7 +180,7 @@
EXPECT_TRUE(cfx_v8()->GetArrayElement(array, 2)->IsUndefined());
EXPECT_EQ(0u, cfx_v8()->GetArrayLength(array));
- cfx_v8()->PutArrayElement(array, 3, cfx_v8()->NewNumber(12));
+ EXPECT_TRUE(cfx_v8()->PutArrayElement(array, 3, cfx_v8()->NewNumber(12)));
EXPECT_FALSE(cfx_v8()->GetArrayElement(array, 2).IsEmpty());
EXPECT_TRUE(cfx_v8()->GetArrayElement(array, 2)->IsUndefined());
EXPECT_FALSE(cfx_v8()->GetArrayElement(array, 3).IsEmpty());
@@ -190,7 +208,8 @@
EXPECT_TRUE(cfx_v8()->GetObjectProperty(object, "clams")->IsUndefined());
EXPECT_EQ(0u, cfx_v8()->GetObjectPropertyNames(object).size());
- cfx_v8()->PutObjectProperty(object, "clams", cfx_v8()->NewNumber(12));
+ EXPECT_TRUE(
+ cfx_v8()->PutObjectProperty(object, "clams", cfx_v8()->NewNumber(12)));
EXPECT_FALSE(cfx_v8()->GetObjectProperty(object, "clams").IsEmpty());
EXPECT_TRUE(cfx_v8()->GetObjectProperty(object, "clams")->IsNumber());
EXPECT_EQ(1u, cfx_v8()->GetObjectPropertyNames(object).size());
@@ -204,3 +223,47 @@
EXPECT_TRUE(cfx_v8()->ToObject(object)->IsObject());
EXPECT_TRUE(cfx_v8()->ToArray(object).IsEmpty());
}
+
+TEST_F(FXV8UnitTest, ThrowFromGetter) {
+ v8::Isolate::Scope isolate_scope(isolate());
+ v8::HandleScope handle_scope(isolate());
+ v8::Local<v8::Context> context = v8::Context::New(isolate());
+ v8::Context::Scope context_scope(context);
+
+ v8::Local<v8::Object> object = cfx_v8()->NewObject();
+ v8::Local<v8::String> name = cfx_v8()->NewString("clams");
+ EXPECT_TRUE(
+ object
+ ->SetAccessor(context, name,
+ [](v8::Local<v8::Name> property,
+ const v8::PropertyCallbackInfo<v8::Value>& info) {
+ getter_sentinel = true;
+ info.GetIsolate()->ThrowException(property);
+ })
+ .FromJust());
+ getter_sentinel = false;
+ EXPECT_TRUE(cfx_v8()->GetObjectProperty(object, "clams").IsEmpty());
+ EXPECT_TRUE(getter_sentinel);
+}
+
+TEST_F(FXV8UnitTest, ThrowFromSetter) {
+ v8::Isolate::Scope isolate_scope(isolate());
+ v8::HandleScope handle_scope(isolate());
+ v8::Local<v8::Context> context = v8::Context::New(isolate());
+ v8::Context::Scope context_scope(context);
+
+ v8::Local<v8::Object> object = cfx_v8()->NewObject();
+ v8::Local<v8::String> name = cfx_v8()->NewString("clams");
+ EXPECT_TRUE(object
+ ->SetAccessor(context, name, nullptr,
+ [](v8::Local<v8::Name> property,
+ v8::Local<v8::Value> value,
+ const v8::PropertyCallbackInfo<void>& info) {
+ setter_sentinel = true;
+ info.GetIsolate()->ThrowException(property);
+ })
+ .FromJust());
+ setter_sentinel = false;
+ EXPECT_FALSE(cfx_v8()->PutObjectProperty(object, "clams", name));
+ EXPECT_TRUE(setter_sentinel);
+}
