Make orphaning old objects automatic during dictionary set/removal.
Remove specific fixes for bug 901654, this should happen automatically
everywhere where updating a directory key's value might be an issue.
Bug: chromium:901654
Change-Id: Id74d922306275718d909ac7a7b4f7c7e7a5d9955
Reviewed-on: https://pdfium-review.googlesource.com/c/45450
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_flateencoder.cpp b/core/fpdfapi/edit/cpdf_flateencoder.cpp
index 7172bf2..0cfcce6 100644
--- a/core/fpdfapi/edit/cpdf_flateencoder.cpp
+++ b/core/fpdfapi/edit/cpdf_flateencoder.cpp
@@ -28,7 +28,7 @@
m_dwSize = pDestAcc->GetSize();
m_pData = pDestAcc->DetachData();
m_pClonedDict = ToDictionary(pStream->GetDict()->Clone());
- m_pClonedDict->RemoveFor("Filter");
+ m_pClonedDict->RemoveAndOrphan("Filter");
ASSERT(!m_pDict);
return;
}
@@ -48,7 +48,7 @@
m_pClonedDict = ToDictionary(pStream->GetDict()->Clone());
m_pClonedDict->SetNewFor<CPDF_Number>("Length", static_cast<int>(m_dwSize));
m_pClonedDict->SetNewFor<CPDF_Name>("Filter", "FlateDecode");
- m_pClonedDict->RemoveFor(pdfium::stream::kDecodeParms);
+ m_pClonedDict->RemoveAndOrphan(pdfium::stream::kDecodeParms);
ASSERT(!m_pDict);
}
diff --git a/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp b/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
index f9057db..8f02694 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
@@ -107,7 +107,7 @@
// Only stream that can be removed is 0.
if (streams_to_remove_.find(0) != streams_to_remove_.end()) {
CPDF_Dictionary* page_dict = obj_holder_->GetDict();
- page_dict->RemoveFor("Contents");
+ page_dict->RemoveAndOrphan("Contents");
contents_stream_ = nullptr;
}
} else if (contents_array_) {
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index d1d2410..97ec7a4 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -207,10 +207,10 @@
CPDF_Object* CPDF_Dictionary::SetFor(const ByteString& key,
std::unique_ptr<CPDF_Object> pObj) {
CHECK(!IsLocked());
- if (!pObj) {
- m_Map.erase(key);
+ RemoveAndOrphan(key);
+ if (!pObj)
return nullptr;
- }
+
ASSERT(pObj->IsInline());
CPDF_Object* pRet = pObj.get();
m_Map[MaybeIntern(key)] = std::move(pObj);
@@ -240,6 +240,12 @@
return result;
}
+void CPDF_Dictionary::RemoveAndOrphan(const ByteString& key) {
+ auto result = RemoveFor(key);
+ if (m_pHolder)
+ m_pHolder->AddOrphan(std::move(result));
+}
+
void CPDF_Dictionary::ReplaceKey(const ByteString& oldkey,
const ByteString& newkey) {
CHECK(!IsLocked());
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index d3ca8a6..d0e302b 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -114,8 +114,16 @@
void ConvertToIndirectObjectFor(const ByteString& key,
CPDF_IndirectObjectHolder* pHolder);
+ // Removes |key| and returns the object bound to it for re-use.
// Invalidates iterators for the element with the key |key|.
- std::unique_ptr<CPDF_Object> RemoveFor(const ByteString& key);
+ std::unique_ptr<CPDF_Object> RemoveFor(const ByteString& key)
+ WARN_UNUSED_RESULT;
+
+ // Removes |key| but keeps the object bound to it alive for the duration
+ // that the indirect object holder is in existence, in case there are
+ // additional references.
+ // Invalidates iterators for the element with the key |key|.
+ void RemoveAndOrphan(const ByteString& key);
// Invalidates iterators for the element with the key |oldkey|.
void ReplaceKey(const ByteString& oldkey, const ByteString& newkey);
diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index 3837462..11ea67e 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -112,8 +112,8 @@
void CPDF_Stream::SetDataAndRemoveFilter(pdfium::span<const uint8_t> pData) {
SetData(pData);
- m_pDict->RemoveFor("Filter");
- m_pDict->RemoveFor(pdfium::stream::kDecodeParms);
+ m_pDict->RemoveAndOrphan("Filter");
+ m_pDict->RemoveAndOrphan(pdfium::stream::kDecodeParms);
}
void CPDF_Stream::SetDataFromStringstreamAndRemoveFilter(
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index 0872317..2c211ec 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -235,15 +235,13 @@
if (!pClone)
return false;
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
m_pDict->SetFor("V", std::move(pClone));
if (pRV) {
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
m_pDict->SetFor("RV", pDV->Clone());
}
} else {
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
+ m_pDict->RemoveAndOrphan("V");
+ m_pDict->RemoveAndOrphan("RV");
}
if (notify == NotificationOption::kNotify)
NotifyAfterValueChange();
@@ -384,15 +382,11 @@
int iIndex = FindOptionValue(csValue);
if (iIndex < 0) {
ByteString bsEncodeText = PDF_EncodeText(csValue);
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor(key));
m_pDict->SetNewFor<CPDF_String>(key, bsEncodeText, false);
- if (m_Type == kRichText && !bDefault) {
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
+ if (m_Type == kRichText && !bDefault)
m_pDict->SetNewFor<CPDF_String>("RV", bsEncodeText, false);
- }
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+ m_pDict->RemoveAndOrphan("I");
} else {
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor(key));
m_pDict->SetNewFor<CPDF_String>(key, PDF_EncodeText(csValue), false);
if (!bDefault) {
ClearSelection(NotificationOption::kDoNotNotify);
@@ -508,8 +502,8 @@
if (!NotifyListOrComboBoxBeforeChange(csValue))
return false;
}
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+ m_pDict->RemoveAndOrphan("V");
+ m_pDict->RemoveAndOrphan("I");
if (notify == NotificationOption::kNotify)
NotifyListOrComboBoxAfterChange();
return true;
@@ -596,7 +590,7 @@
SelectOption(index, false, NotificationOption::kDoNotNotify);
if (pValue->IsString()) {
if (pValue->GetUnicodeText() == opt_value)
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+ m_pDict->RemoveAndOrphan("V");
} else if (pValue->IsArray()) {
auto pArray = pdfium::MakeUnique<CPDF_Array>();
for (int i = 0; i < CountOptions(); i++) {
@@ -606,13 +600,12 @@
}
}
if (pArray->size() > 0) {
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
m_pDict->SetFor("V", std::move(pArray));
}
}
} else {
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+ m_pDict->RemoveAndOrphan("V");
+ m_pDict->RemoveAndOrphan("I");
}
}
}
@@ -859,7 +852,7 @@
if (bSelected)
pArray->AddNew<CPDF_Number>(iOptIndex);
if (pArray->IsEmpty())
- m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+ m_pDict->RemoveAndOrphan("I");
}
if (notify == NotificationOption::kNotify)
NotifyListOrComboBoxAfterChange();
diff --git a/fpdfsdk/cpdfsdk_baannot.cpp b/fpdfsdk/cpdfsdk_baannot.cpp
index 962d189..7d44485 100644
--- a/fpdfsdk/cpdfsdk_baannot.cpp
+++ b/fpdfsdk/cpdfsdk_baannot.cpp
@@ -96,7 +96,7 @@
void CPDFSDK_BAAnnot::SetAnnotName(const WideString& sName) {
if (sName.IsEmpty())
- GetAnnotDict()->RemoveFor("NM");
+ GetAnnotDict()->RemoveAndOrphan("NM");
else
GetAnnotDict()->SetNewFor<CPDF_String>("NM", PDF_EncodeText(sName), false);
}
@@ -115,7 +115,7 @@
void CPDFSDK_BAAnnot::SetAppState(const ByteString& str) {
if (str.IsEmpty())
- GetAnnotDict()->RemoveFor("AS");
+ GetAnnotDict()->RemoveAndOrphan("AS");
else
GetAnnotDict()->SetNewFor<CPDF_String>("AS", str, false);
}
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index 5a96818..10f91d2 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -793,9 +793,9 @@
} else {
if (pApDict) {
if (appearanceMode == FPDF_ANNOT_APPEARANCEMODE_NORMAL)
- pAnnotDict->RemoveFor("AP");
+ pAnnotDict->RemoveAndOrphan("AP");
else
- pApDict->RemoveFor(modeKey);
+ pApDict->RemoveAndOrphan(modeKey);
}
}
diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp
index 5ac0be8..9c3effa 100644
--- a/fpdfsdk/fpdf_flatten.cpp
+++ b/fpdfsdk/fpdf_flatten.cpp
@@ -404,6 +404,6 @@
m.e, m.f, sFormName.c_str());
pNewXObject->SetDataAndRemoveFilter(sStream.AsRawSpan());
}
- pPageDict->RemoveFor("Annots");
+ pPageDict->RemoveAndOrphan("Annots");
return FLATTEN_SUCCESS;
}
diff --git a/fpdfsdk/pwl/cpwl_appstream.cpp b/fpdfsdk/pwl/cpwl_appstream.cpp
index a81a4eb..2e02742 100644
--- a/fpdfsdk/pwl/cpwl_appstream.cpp
+++ b/fpdfsdk/pwl/cpwl_appstream.cpp
@@ -1947,7 +1947,7 @@
}
void CPWL_AppStream::Remove(const ByteString& sAPType) {
- dict_->RemoveFor(sAPType);
+ dict_->RemoveAndOrphan(sAPType);
}
ByteString CPWL_AppStream::GetBackgroundAppStream() const {
