Do copy-on-write in CPDF_PageContentManager
When CPDF_PageContentManager modifies an object, if the modified object
has multiple references to it, then clone the object and modify the
clone. This fixes issues where modifying an object to edit one page
accidentally modifies multiple pages.
Bug: pdfium:2012
Change-Id: I4b52894ab44889bae0df9415542f018c91436c1a
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/105630
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp b/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
index bf5fefe..4be22e6 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
@@ -4,8 +4,11 @@
#include "core/fpdfapi/edit/cpdf_pagecontentmanager.h"
+#include <stdint.h>
+
#include <map>
#include <numeric>
+#include <set>
#include <sstream>
#include <utility>
#include <vector>
@@ -17,20 +20,25 @@
#include "core/fpdfapi/parser/cpdf_document.h"
#include "core/fpdfapi/parser/cpdf_reference.h"
#include "core/fpdfapi/parser/cpdf_stream.h"
+#include "core/fpdfapi/parser/object_tree_traversal_util.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
#include "third_party/base/check.h"
#include "third_party/base/containers/adapters.h"
+#include "third_party/base/containers/contains.h"
#include "third_party/base/numerics/safe_conversions.h"
CPDF_PageContentManager::CPDF_PageContentManager(
CPDF_PageObjectHolder* page_obj_holder,
CPDF_Document* document)
- : page_obj_holder_(page_obj_holder), document_(document) {
+ : page_obj_holder_(page_obj_holder),
+ document_(document),
+ objects_with_multi_refs_(GetObjectsWithMultipleReferences(document_)) {
RetainPtr<CPDF_Dictionary> page_dict = page_obj_holder_->GetMutableDict();
RetainPtr<CPDF_Object> contents_obj =
page_dict->GetMutableObjectFor("Contents");
RetainPtr<CPDF_Array> contents_array = ToArray(contents_obj);
if (contents_array) {
+ CHECK(contents_array->IsInline());
contents_ = std::move(contents_array);
return;
}
@@ -43,10 +51,19 @@
return;
contents_array.Reset(indirect_obj->AsMutableArray());
- if (contents_array)
- contents_ = std::move(contents_array);
- else if (indirect_obj->IsStream())
+ if (contents_array) {
+ if (pdfium::Contains(objects_with_multi_refs_,
+ contents_array->GetObjNum())) {
+ RetainPtr<CPDF_Array> cloned_contents_array =
+ pdfium::WrapRetain(contents_array->Clone()->AsMutableArray());
+ page_dict->SetFor("Contents", cloned_contents_array);
+ contents_ = std::move(cloned_contents_array);
+ } else {
+ contents_ = std::move(contents_array);
+ }
+ } else if (indirect_obj->IsStream()) {
contents_ = pdfium::WrapRetain(indirect_obj->AsMutableStream());
+ }
}
}
@@ -121,7 +138,35 @@
}
RetainPtr<CPDF_Stream> existing_stream = GetStreamByIndex(stream_index);
- existing_stream->SetDataFromStringstreamAndRemoveFilter(buf);
+ CHECK(existing_stream);
+ if (!pdfium::Contains(objects_with_multi_refs_,
+ existing_stream->GetObjNum())) {
+ existing_stream->SetDataFromStringstreamAndRemoveFilter(buf);
+ return;
+ }
+
+ if (GetContentsStream()) {
+ auto new_stream = document_->NewIndirect<CPDF_Stream>();
+ new_stream->SetDataFromStringstream(buf);
+ RetainPtr<CPDF_Dictionary> page_dict = page_obj_holder_->GetMutableDict();
+ page_dict->SetNewFor<CPDF_Reference>("Contents", document_,
+ new_stream->GetObjNum());
+ }
+
+ RetainPtr<CPDF_Array> contents_array = GetContentsArray();
+ if (!contents_array) {
+ return;
+ }
+
+ RetainPtr<CPDF_Reference> stream_reference =
+ ToReference(contents_array->GetMutableObjectAt(stream_index));
+ if (!stream_reference) {
+ return;
+ }
+
+ auto new_stream = document_->NewIndirect<CPDF_Stream>();
+ new_stream->SetDataFromStringstream(buf);
+ stream_reference->SetRef(document_, new_stream->GetObjNum());
}
void CPDF_PageContentManager::ScheduleRemoveStreamByIndex(size_t stream_index) {
diff --git a/core/fpdfapi/edit/cpdf_pagecontentmanager.h b/core/fpdfapi/edit/cpdf_pagecontentmanager.h
index f1605b4..5785fc3 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentmanager.h
+++ b/core/fpdfapi/edit/cpdf_pagecontentmanager.h
@@ -5,6 +5,8 @@
#ifndef CORE_FPDFAPI_EDIT_CPDF_PAGECONTENTMANAGER_H_
#define CORE_FPDFAPI_EDIT_CPDF_PAGECONTENTMANAGER_H_
+#include <stdint.h>
+
#include <set>
#include "core/fxcrt/fx_string_wrappers.h"
@@ -50,6 +52,7 @@
UnownedPtr<CPDF_PageObjectHolder> const page_obj_holder_;
UnownedPtr<CPDF_Document> const document_;
+ const std::set<uint32_t> objects_with_multi_refs_;
// When holding a CPDF_Stream, the pointer may be null.
absl::variant<RetainPtr<CPDF_Stream>, RetainPtr<CPDF_Array>> contents_;
std::set<size_t> streams_to_remove_;
diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp
index 22df401..91b9d6b 100644
--- a/fpdfsdk/fpdf_edit_embeddertest.cpp
+++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -987,8 +987,7 @@
VerifySavedRendering(saved_page1, 200, 200, FirstRemovedChecksum());
CloseSavedPage(saved_page1);
FPDF_PAGE saved_page2 = LoadSavedPage(1);
- // TODO(crbug.com/pdfium/2012): Should be `HelloWorldChecksum()`.
- VerifySavedRendering(saved_page2, 200, 200, FirstRemovedChecksum());
+ VerifySavedRendering(saved_page2, 200, 200, HelloWorldChecksum());
CloseSavedPage(saved_page2);
CloseSavedDocument();
@@ -1054,8 +1053,7 @@
VerifySavedRendering(saved_page1, 200, 200, FirstRemovedChecksum());
CloseSavedPage(saved_page1);
FPDF_PAGE saved_page2 = LoadSavedPage(1);
- // TODO(crbug.com/pdfium/2012): Should be `HelloWorldChecksum()`.
- VerifySavedRendering(saved_page2, 200, 200, FirstRemovedChecksum());
+ VerifySavedRendering(saved_page2, 200, 200, HelloWorldChecksum());
CloseSavedPage(saved_page2);
CloseSavedDocument();
@@ -1113,8 +1111,7 @@
VerifySavedRendering(saved_page1, 200, 200, FirstRemovedChecksum());
CloseSavedPage(saved_page1);
FPDF_PAGE saved_page2 = LoadSavedPage(1);
- // TODO(crbug.com/pdfium/2012): Should be HelloWorldChecksum().
- VerifySavedRendering(saved_page2, 200, 200, FirstRemovedChecksum());
+ VerifySavedRendering(saved_page2, 200, 200, HelloWorldChecksum());
CloseSavedPage(saved_page2);
CloseSavedDocument();
