Custom toString() methods may delete annots.
In this case, we observe the destruction of the object, but have
unfortunately saved a pointer to it in a local variable.
BUG=679643
Review-Url: https://codereview.chromium.org/2628233002
diff --git a/fpdfsdk/javascript/Annot.cpp b/fpdfsdk/javascript/Annot.cpp
index 0c16b3b..1aef463 100644
--- a/fpdfsdk/javascript/Annot.cpp
+++ b/fpdfsdk/javascript/Annot.cpp
@@ -38,20 +38,24 @@
Annot::~Annot() {}
bool Annot::hidden(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) {
- CPDFSDK_BAAnnot* baAnnot = ToBAAnnot(m_pAnnot.Get());
- if (!baAnnot)
- return false;
-
if (vp.IsGetting()) {
- CPDF_Annot* pPDFAnnot = baAnnot->GetPDFAnnot();
+ if (!m_pAnnot) {
+ sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT);
+ return false;
+ }
+ CPDF_Annot* pPDFAnnot = ToBAAnnot(m_pAnnot.Get())->GetPDFAnnot();
vp << CPDF_Annot::IsAnnotationHidden(pPDFAnnot->GetAnnotDict());
return true;
}
bool bHidden;
- vp >> bHidden;
+ vp >> bHidden; // May invalidate m_pAnnot.
+ if (!m_pAnnot) {
+ sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT);
+ return false;
+ }
- uint32_t flags = baAnnot->GetFlags();
+ uint32_t flags = ToBAAnnot(m_pAnnot.Get())->GetFlags();
if (bHidden) {
flags |= ANNOTFLAG_HIDDEN;
flags |= ANNOTFLAG_INVISIBLE;
@@ -63,23 +67,28 @@
flags &= ~ANNOTFLAG_NOVIEW;
flags |= ANNOTFLAG_PRINT;
}
- baAnnot->SetFlags(flags);
+ ToBAAnnot(m_pAnnot.Get())->SetFlags(flags);
return true;
}
bool Annot::name(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) {
- CPDFSDK_BAAnnot* baAnnot = ToBAAnnot(m_pAnnot.Get());
- if (!baAnnot)
- return false;
-
if (vp.IsGetting()) {
- vp << baAnnot->GetAnnotName();
+ if (!m_pAnnot) {
+ sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT);
+ return false;
+ }
+ vp << ToBAAnnot(m_pAnnot.Get())->GetAnnotName();
return true;
}
CFX_WideString annotName;
- vp >> annotName;
- baAnnot->SetAnnotName(annotName);
+ vp >> annotName; // May invalidate m_pAnnot.
+ if (!m_pAnnot) {
+ sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT);
+ return false;
+ }
+
+ ToBAAnnot(m_pAnnot.Get())->SetAnnotName(annotName);
return true;
}
@@ -88,12 +97,12 @@
sError = JSGetStringFromID(IDS_STRING_JSREADONLY);
return false;
}
-
- CPDFSDK_BAAnnot* baAnnot = ToBAAnnot(m_pAnnot.Get());
- if (!baAnnot)
+ if (!m_pAnnot) {
+ sError = JSGetStringFromID(IDS_STRING_JSBADOBJECT);
return false;
-
- vp << CPDF_Annot::AnnotSubtypeToString(baAnnot->GetAnnotSubtype());
+ }
+ vp << CPDF_Annot::AnnotSubtypeToString(
+ ToBAAnnot(m_pAnnot.Get())->GetAnnotSubtype());
return true;
}
diff --git a/fpdfsdk/javascript/JS_Value.h b/fpdfsdk/javascript/JS_Value.h
index ff2c620..313f0c3 100644
--- a/fpdfsdk/javascript/JS_Value.h
+++ b/fpdfsdk/javascript/JS_Value.h
@@ -92,6 +92,7 @@
CJS_Runtime* GetJSRuntime() const { return m_pJSRuntime; }
CJS_Value* GetJSValue() { return &m_Value; }
+ // These calls may re-enter JS (and hence invalidate objects).
void operator<<(int val);
void operator>>(int&) const;
void operator<<(bool val);
@@ -127,13 +128,15 @@
virtual ~CJS_Array();
void Attach(v8::Local<v8::Array> pArray);
+ int GetLength(CJS_Runtime* pRuntime) const;
+
+ // These two calls may re-enter JS (and hence invalidate objects).
void GetElement(CJS_Runtime* pRuntime,
unsigned index,
CJS_Value& value) const;
void SetElement(CJS_Runtime* pRuntime,
unsigned index,
const CJS_Value& value);
- int GetLength(CJS_Runtime* pRuntime) const;
v8::Local<v8::Array> ToV8Array(CJS_Runtime* pRuntime) const;
diff --git a/testing/resources/javascript/bug_679643.in b/testing/resources/javascript/bug_679643.in
new file mode 100644
index 0000000..e964386
--- /dev/null
+++ b/testing/resources/javascript/bug_679643.in
@@ -0,0 +1,135 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /AcroForm 4 0 R
+ /OpenAction 10 0 R
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 1
+ /Kids [
+ 3 0 R
+ ]
+>>
+endobj
+% Page number 0.
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /Resources <<
+ /Font <</F1 15 0 R>>
+ >>
+ /Contents [21 0 R]
+ /MediaBox [0 0 612 792]
+ /Annots [7 0 R 8 0 R 9 0 R]
+>>
+endobj
+% Forms
+{{object 4 0}} <<
+ /XFA [
+ (xdp:xdp) 23 0 R
+ (form) 29 0 R
+ (</xdp:xdp>) 30 0 R
+ ]
+ /Fields [
+ 5 0 R
+ ]
+>>
+endobj
+% Fields
+{{object 5 0}} <<
+ /T (MyField)
+ /Kids [
+ 6 0 R
+ ]
+ /Rect [100 100 400 400]
+>>
+endobj
+{{object 6 0}} <<
+ /Parent 5 0 R
+ /FT /Btn
+ /Kids [
+ 7 0 R
+ 8 0 R
+ 9 0 R
+ ]
+ /Rect [200 200 220 220]
+>>
+endobj
+{{object 7 0}} <<
+ /Parent 6 0 R
+ /Type /Annot
+ /Subtype /Widget
+ /Rect [220 220 240 240]
+>>
+endobj
+{{object 8 0}} <<
+ /Parent 6 0 R
+ /Type /Annot
+ /Subtype /Widget
+ /Rect [240 240 260 260]
+>>
+endobj
+{{object 9 0}} <<
+ /Parent 6 0 R
+ /Type /Annot
+ /Subtype /Widget
+ /Rect [240 240 260 260]
+>>
+endobj
+% OpenAction action
+{{object 10 0}} <<
+ /Type /Action
+ /S /JavaScript
+ /JS 11 0 R
+>>
+endobj
+% JS program to exexute
+{{object 11 0}} <<
+>>
+stream
+var theName = "MyField";
+function Mangles() {
+ app.alert('Starting ...');
+ try {
+ var annots = this.getAnnots();
+ annots[0].name = {
+ toString: () => {
+ app.alert('Firing ...');
+ this.removeField(theName);
+ gc();
+ return false;
+ }
+ };
+ } catch (e) {
+ app.alert("failed: " + e);
+ }
+}
+Mangles();
+endstream
+endobj
+{{object 23 0}} <<
+>>stream
+<?xml version="1.0" encoding="UTF-8"?>
+<xdp:xdp xmlns:xdp="http://ns.adobe.com/xdp/">
+endstream
+endobj
+{{object 29 0}} <<
+>>stream
+<config></config>
+<template></template>
+endstream
+endobj
+{{object 30 0}} <<
+>>stream
+</xdp:xdp>
+endstream
+endobj
+{{xref}}
+trailer <<
+ /Root 1 0 R
+>>
+{{startxref}}
+%%EOF
diff --git a/testing/resources/javascript/bug_679643_expected.txt b/testing/resources/javascript/bug_679643_expected.txt
new file mode 100644
index 0000000..36d4a31
--- /dev/null
+++ b/testing/resources/javascript/bug_679643_expected.txt
@@ -0,0 +1,3 @@
+Alert: Starting ...
+Alert: Firing ...
+Alert: failed: Annot.name: Object no longer exists.
