Dismiss exceptions while setting properties/elements from JS handlers.
Otherwise, we can't use any other V8 functionality until the
exception is handled.
Bug: chromium:1098213
Change-Id: Ie846194afab52e5d61795c7af74b1e29b46db953
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/70913
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 3a01aa2..2901e07 100644
--- a/fxjs/cfx_v8.cpp
+++ b/fxjs/cfx_v8.cpp
@@ -26,11 +26,12 @@
return fxv8::ReentrantGetObjectPropertyNamesHelper(m_pIsolate.Get(), pObj);
}
-bool CFX_V8::PutObjectProperty(v8::Local<v8::Object> pObj,
+void CFX_V8::PutObjectProperty(v8::Local<v8::Object> pObj,
ByteStringView bsUTF8PropertyName,
v8::Local<v8::Value> pPut) {
- return fxv8::ReentrantPutObjectPropertyHelper(m_pIsolate.Get(), pObj,
- bsUTF8PropertyName, pPut);
+ v8::TryCatch squash_exceptions(GetIsolate());
+ fxv8::ReentrantPutObjectPropertyHelper(GetIsolate(), pObj, bsUTF8PropertyName,
+ pPut);
}
void CFX_V8::DisposeIsolate() {
@@ -46,11 +47,11 @@
return fxv8::NewObjectHelper(GetIsolate());
}
-bool CFX_V8::PutArrayElement(v8::Local<v8::Array> pArray,
+void CFX_V8::PutArrayElement(v8::Local<v8::Array> pArray,
unsigned index,
v8::Local<v8::Value> pValue) {
- return fxv8::ReentrantPutArrayElementHelper(GetIsolate(), pArray, index,
- pValue);
+ v8::TryCatch squash_exceptions(GetIsolate());
+ fxv8::ReentrantPutArrayElementHelper(GetIsolate(), pArray, index, pValue);
}
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 cb152ac..8cc49e6 100644
--- a/fxjs/cfx_v8.h
+++ b/fxjs/cfx_v8.h
@@ -44,7 +44,7 @@
unsigned GetArrayLength(v8::Local<v8::Array> pArray);
v8::Local<v8::Value> GetArrayElement(v8::Local<v8::Array> pArray,
unsigned index);
- bool PutArrayElement(v8::Local<v8::Array> pArray,
+ void PutArrayElement(v8::Local<v8::Array> pArray,
unsigned index,
v8::Local<v8::Value> pValue);
@@ -52,7 +52,7 @@
std::vector<WideString> GetObjectPropertyNames(v8::Local<v8::Object> pObj);
v8::Local<v8::Value> GetObjectProperty(v8::Local<v8::Object> pObj,
ByteStringView bsUTF8PropertyName);
- bool PutObjectProperty(v8::Local<v8::Object> pObj,
+ void 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 d9358da..428f937 100644
--- a/fxjs/cfx_v8_unittest.cpp
+++ b/fxjs/cfx_v8_unittest.cpp
@@ -50,13 +50,13 @@
// 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));
+ 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));
+ cfx_v8()->PutArrayElement(empty_array, 0, marker);
EXPECT_TRUE(cfx_v8()->GetArrayElement(empty_array, 0).IsEmpty());
EXPECT_EQ(0u, cfx_v8()->GetArrayLength(empty_array));
}
@@ -180,7 +180,7 @@
EXPECT_TRUE(cfx_v8()->GetArrayElement(array, 2)->IsUndefined());
EXPECT_EQ(0u, cfx_v8()->GetArrayLength(array));
- EXPECT_TRUE(cfx_v8()->PutArrayElement(array, 3, cfx_v8()->NewNumber(12)));
+ 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());
@@ -208,8 +208,7 @@
EXPECT_TRUE(cfx_v8()->GetObjectProperty(object, "clams")->IsUndefined());
EXPECT_EQ(0u, cfx_v8()->GetObjectPropertyNames(object).size());
- EXPECT_TRUE(
- cfx_v8()->PutObjectProperty(object, "clams", cfx_v8()->NewNumber(12)));
+ 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());
@@ -264,6 +263,6 @@
})
.FromJust());
setter_sentinel = false;
- EXPECT_FALSE(cfx_v8()->PutObjectProperty(object, "clams", name));
+ cfx_v8()->PutObjectProperty(object, "clams", name);
EXPECT_TRUE(setter_sentinel);
}
diff --git a/testing/resources/javascript/bug_1098213.in b/testing/resources/javascript/bug_1098213.in
new file mode 100644
index 0000000..0c99311
--- /dev/null
+++ b/testing/resources/javascript/bug_1098213.in
@@ -0,0 +1,65 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /OpenAction 8 0 R
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 2
+ /Kids [
+ 3 0 R
+ 4 0 R
+ ]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /CropBox [179 173 200 75]
+>>
+endobj
+{{object 4 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /CropBox [127 214 186 29]
+ /Annots [
+ 5 0 R
+ 6 0 R
+ 7 0 R
+ ]
+>>
+endobj
+{{object 5 0}} <<
+ /Type /Annot
+>
+endobj
+{{object 6 0}} <<
+ /Type /Annot
+>>
+endobj
+{{object 7 0}} <<
+ /Type /Annot
+>>
+endobj
+{{object 8 0}} <<
+ /Type /Action
+ /S /JavaScript
+ /JS (
+ Object.defineProperty(Array.prototype, 1, {
+ set: (v) => {
+ Object.defineProperty(Array.prototype, 1, {
+ get: () => {}
+ });
+ }
+ });
+ try { this.getAnnots(); } catch (e) { app.alert('Caught: ' + e); }
+ app.alert('Done');
+ )
+>>
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/javascript/bug_1098213_expected.txt b/testing/resources/javascript/bug_1098213_expected.txt
new file mode 100644
index 0000000..9de1818
--- /dev/null
+++ b/testing/resources/javascript/bug_1098213_expected.txt
@@ -0,0 +1 @@
+Alert: Done