From 403dbd5f5bf61c989faee4e4038d13af2f4e9931 Mon Sep 17 00:00:00 2001 From: m-holger Date: Wed, 1 Oct 2025 20:06:38 +0100 Subject: [PATCH] Refactor stream copying: simplify `replaceStreamData` logic and remove redundant `StreamDataProvider` management. --- libqpdf/QPDF.cc | 55 +++++++++++++++++++++++-------------------------------- libqpdf/QPDF_Stream.cc | 20 +++++++++++++------- libqpdf/qpdf/QPDFObjectHandle_private.hh | 4 ++++ libqpdf/qpdf/QPDF_private.hh | 4 +--- qpdf/qpdf.testcov | 3 --- 5 files changed, 41 insertions(+), 45 deletions(-) diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index b3abf7a..7888faf 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -680,65 +680,56 @@ QPDF::replaceForeignIndirectObjects(QPDFObjectHandle foreign, ObjCopier& 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(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(*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 stream_buffer = stream.getStreamDataBuffer(); - if ((foreign_stream_qpdf.m->immediate_copy_from) && (stream_buffer == nullptr)) { + std::shared_ptr 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 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( 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"]); } } diff --git a/libqpdf/QPDF_Stream.cc b/libqpdf/QPDF_Stream.cc index d140e13..99332ea 100644 --- a/libqpdf/QPDF_Stream.cc +++ b/libqpdf/QPDF_Stream.cc @@ -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(std::move(data)); + s->stream_provider = nullptr; + replaceFilterData(filter, decode_parms, s->stream_data->getSize()); +} + +void +Stream::replaceStreamData( std::shared_ptr 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(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 diff --git a/libqpdf/qpdf/QPDFObjectHandle_private.hh b/libqpdf/qpdf/QPDFObjectHandle_private.hh index a97472d..2073456 100644 --- a/libqpdf/qpdf/QPDFObjectHandle_private.hh +++ b/libqpdf/qpdf/QPDFObjectHandle_private.hh @@ -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 data, QPDFObjectHandle const& filter, QPDFObjectHandle const& decode_parms); diff --git a/libqpdf/qpdf/QPDF_private.hh b/libqpdf/qpdf/QPDF_private.hh index 356f756..1d8b099 100644 --- a/libqpdf/qpdf/QPDF_private.hh +++ b/libqpdf/qpdf/QPDF_private.hh @@ -846,9 +846,7 @@ class QPDF::Members bool ever_called_get_all_pages{false}; std::vector warnings; std::map object_copiers; - std::shared_ptr copied_streams; - // copied_stream_data_provider is owned by copied_streams - CopiedStreamDataProvider* copied_stream_data_provider{nullptr}; + std::shared_ptr copied_stream_data_provider; bool reconstructed_xref{false}; bool in_read_xref_stream{false}; bool fixed_dangling_refs{false}; diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 9b6ec8e..947f331 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -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 -- libgit2 0.21.4