Custom toString() methods may delete annots.

fpdfsdk/javascript/Annot.cpp

 // Copyright 2016 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "fpdfsdk/javascript/Annot.h"
 
 #include "fpdfsdk/javascript/JS_Define.h"
 #include "fpdfsdk/javascript/JS_Object.h"
@@ skipping 20 matching lines @@
 BEGIN_JS_STATIC_METHOD(CJS_Annot)
 END_JS_STATIC_METHOD()
 
 IMPLEMENT_JS_CLASS(CJS_Annot, Annot)
 
 Annot::Annot(CJS_Object* pJSObject) : CJS_EmbedObj(pJSObject) {}
 
 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.

[dsinclair, 2017/01/12 19:14:40]

This seems like a crazy side effect. Should we log a bug to make the ToString
methods const?

+  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;
     flags |= ANNOTFLAG_NOVIEW;
     flags &= ~ANNOTFLAG_PRINT;
   } else {
     flags &= ~ANNOTFLAG_HIDDEN;
     flags &= ~ANNOTFLAG_INVISIBLE;
     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;
 }
 
 bool Annot::type(IJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sError) {
   if (vp.IsSetting()) {
     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;
 }
 
 void Annot::SetSDKAnnot(CPDFSDK_BAAnnot* annot) {
   m_pAnnot.Reset(annot);
 }