Refactor stream copying: simplify `replaceStreamData` logic and remove redundant…

… `StreamDataProvider` management.

Refactor stream copying: simplify `replaceStreamData` logic and remove redundant…
… `StreamDataProvider` management.
m-holger
1 parent d207f615
Showing 5 changed files with 41 additions and 45 deletions
libqpdf/QPDF.cc
libqpdf/QPDF_Stream.cc
libqpdf/qpdf/QPDFObjectHandle_private.hh
libqpdf/qpdf/QPDF_private.hh
qpdf/qpdf.testcov
@@ -680,65 +680,56 @@ QPDF::replaceForeignIndirectObjects(QPDFObjectHandle foreign, ObjCopier&amp; obj_cop
 }
  
 void
-QPDF::copyStreamData(QPDFObjectHandle result, QPDFObjectHandle foreign)
+QPDF::copyStreamData(QPDFObjectHandle result, QPDFObjectHandle foreign_oh)
 {
     // This method was originally written for copying foreign streams, but it is used by
-    // QPDFObjectHandle to copy streams from the same QPDF object as well.
-
-    QPDFObjectHandle dict = result.getDict();
-    QPDFObjectHandle old_dict = foreign.getDict();
-    if (m->copied_stream_data_provider == nullptr) {
-        m->copied_stream_data_provider = new CopiedStreamDataProvider(*this);
-        m->copied_streams =
-            std::shared_ptr<QPDFObjectHandle::StreamDataProvider>(m->copied_stream_data_provider);
+    // Stream::copy to copy streams from the same QPDF object as well.
+
+    Dictionary dict = result.getDict();
+    Dictionary old_dict = foreign_oh.getDict();
+    if (!m->copied_stream_data_provider) {
+        m->copied_stream_data_provider = std::make_shared<CopiedStreamDataProvider>(*this);
     }
     QPDFObjGen local_og(result.getObjGen());
     // Copy information from the foreign stream so we can pipe its data later without keeping the
     // original QPDF object around.
  
     QPDF& foreign_stream_qpdf =
-        foreign.getQPDF("unable to retrieve owning qpdf from foreign stream");
+        foreign_oh.getQPDF("unable to retrieve owning qpdf from foreign stream");
  
-    auto stream = foreign.as_stream();
-    if (!stream) {
+    Stream foreign = foreign_oh;
+    if (!foreign) {
         throw std::logic_error("unable to retrieve underlying stream object from foreign stream");
     }
-    std::shared_ptr<Buffer> stream_buffer = stream.getStreamDataBuffer();
-    if ((foreign_stream_qpdf.m->immediate_copy_from) && (stream_buffer == nullptr)) {
+    std::shared_ptr<Buffer> stream_buffer = foreign.getStreamDataBuffer();
+    if (foreign_stream_qpdf.m->immediate_copy_from && !stream_buffer) {
         // Pull the stream data into a buffer before attempting the copy operation. Do it on the
         // source stream so that if the source stream is copied multiple times, we don't have to
         // keep duplicating the memory.
-        QTC::TC("qpdf", "QPDF immediate copy stream data");
         foreign.replaceStreamData(
-            foreign.getRawStreamData(),
-            old_dict.getKey("/Filter"),
-            old_dict.getKey("/DecodeParms"));
-        stream_buffer = stream.getStreamDataBuffer();
+            foreign.getRawStreamData(), old_dict["/Filter"], old_dict["/DecodeParms"]);
+        stream_buffer = foreign.getStreamDataBuffer();
     }
-    std::shared_ptr<QPDFObjectHandle::StreamDataProvider> stream_provider =
-        stream.getStreamDataProvider();
-    if (stream_buffer.get()) {
-        QTC::TC("qpdf", "QPDF copy foreign stream with buffer");
-        result.replaceStreamData(
-            stream_buffer, dict.getKey("/Filter"), dict.getKey("/DecodeParms"));
-    } else if (stream_provider.get()) {
+    auto stream_provider = foreign.getStreamDataProvider();
+    if (stream_buffer) {
+        result.replaceStreamData(stream_buffer, dict["/Filter"], dict["/DecodeParms"]);
+    } else if (stream_provider) {
         // In this case, the remote stream's QPDF must stay in scope.
-        QTC::TC("qpdf", "QPDF copy foreign stream with provider");
-        m->copied_stream_data_provider->registerForeignStream(local_og, foreign);
+        m->copied_stream_data_provider->registerForeignStream(local_og, foreign_oh);
         result.replaceStreamData(
-            m->copied_streams, dict.getKey("/Filter"), dict.getKey("/DecodeParms"));
+            m->copied_stream_data_provider, dict["/Filter"], dict["/DecodeParms"]);
     } else {
         auto foreign_stream_data = std::make_shared<ForeignStreamData>(
             foreign_stream_qpdf.m->encp,
             foreign_stream_qpdf.m->file,
             foreign,
             foreign.offset(),
-            stream.getLength(),
+            foreign.getLength(),
             dict,
-            stream.isRootMetadata());
+            foreign.isRootMetadata());
         m->copied_stream_data_provider->registerForeignStream(local_og, foreign_stream_data);
         result.replaceStreamData(
-            m->copied_streams, dict.getKey("/Filter"), dict.getKey("/DecodeParms"));
+            m->copied_stream_data_provider, dict["/Filter"], dict["/DecodeParms"]);
     }
 }
  
@@ -628,6 +628,16 @@ Stream::pipeStreamData(
  
 void
 Stream::replaceStreamData(
+    std::string&& data, QPDFObjectHandle const& filter, QPDFObjectHandle const& decode_parms)
+{
+    auto s = stream();
+    s->stream_data = std::make_shared<Buffer>(std::move(data));
+    s->stream_provider = nullptr;
+    replaceFilterData(filter, decode_parms, s->stream_data->getSize());
+}
+
+void
+Stream::replaceStreamData(
     std::shared_ptr<Buffer> data,
     QPDFObjectHandle const& filter,
     QPDFObjectHandle const& decode_parms)
@@ -635,7 +645,7 @@ Stream::replaceStreamData(
     auto s = stream();
     s->stream_data = data;
     s->stream_provider = nullptr;
-    replaceFilterData(filter, decode_parms, data->getSize());
+    replaceFilterData(filter, decode_parms, data->size());
 }
  
 void
@@ -781,12 +791,8 @@ void
 QPDFObjectHandle::replaceStreamData(
     std::string const& data, QPDFObjectHandle const& filter, QPDFObjectHandle const& decode_parms)
 {
-    auto b = std::make_shared<Buffer>(data.length());
-    unsigned char* bp = b->getBuffer();
-    if (bp) {
-        memcpy(bp, data.c_str(), data.length());
-    }
-    as_stream(error).replaceStreamData(b, filter, decode_parms);
+    std::string s(data);
+    as_stream(error).replaceStreamData(std::move(s), filter, decode_parms);
 }
  
 void
@@ -520,6 +520,10 @@ namespace qpdf
         std::string getStreamData(qpdf_stream_decode_level_e level);
         std::string getRawStreamData();
         void replaceStreamData(
+            std::string&& data,
+            QPDFObjectHandle const& filter,
+            QPDFObjectHandle const& decode_parms);
+        void replaceStreamData(
             std::shared_ptr<Buffer> data,
             QPDFObjectHandle const& filter,
             QPDFObjectHandle const& decode_parms);
@@ -846,9 +846,7 @@ class QPDF::Members
     bool ever_called_get_all_pages{false};
     std::vector<QPDFExc> warnings;
     std::map<unsigned long long, ObjCopier> object_copiers;
-    std::shared_ptr<QPDFObjectHandle::StreamDataProvider> copied_streams;
-    // copied_stream_data_provider is owned by copied_streams
-    CopiedStreamDataProvider* copied_stream_data_provider{nullptr};
+    std::shared_ptr<CopiedStreamDataProvider> copied_stream_data_provider;
     bool reconstructed_xref{false};
     bool in_read_xref_stream{false};
     bool fixed_dangling_refs{false};
@@ -252,9 +252,6 @@ QPDFJob image optimize no pipeline 0
 QPDFJob image optimize no shrink 0
 QPDFJob image optimize too small 0
 QPDF pipe foreign encrypted stream 0
-QPDF copy foreign stream with provider 0
-QPDF copy foreign stream with buffer 0
-QPDF immediate copy stream data 0
 QPDFJob copy same page more than once 1
 QPDFPageObjectHelper bad token finding names 0
 QPDFJob password mode bytes 0