From e2dedde4bdb5fa68c86d412e534a4b2750739988 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Sat, 7 Jul 2012 17:33:45 -0400 Subject: [PATCH] Don't require stream data provider to know length in advance Breaking API change: length parameter has disappeared from the StreamDataProvider version of QPDFObjectHandle::replaceStreamData since it is no longer necessary to compute it in advance. This breaking change is justified by the fact that removing the length parameter provides the caller an opportunity to simplify the calling code. --- ChangeLog | 18 +++++ TODO | 4 +- examples/pdf-create.cc | 10 +-- examples/pdf-invert-images.cc | 3 +- include/qpdf/QPDFObjectHandle.hh | 31 ++++++--- libqpdf/QPDFObjectHandle.cc | 5 +- libqpdf/QPDF_Stream.cc | 62 +++++++++++------- libqpdf/qpdf/QPDF_Stream.hh | 3 +- qpdf/qpdf.testcov | 2 + .../qtest/qpdf/replaced-stream-data-flate.pdf | Bin 907 -> 1419 bytes qpdf/test_driver.cc | 17 +++-- qpdf/test_large_file.cc | 10 +-- 12 files changed, 100 insertions(+), 65 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5e3092de..6fa4c45d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2012-07-07 Jay Berkenbilt + + * NOTE: BREAKING API CHANGE. Remove previously required length + parameter from the version QPDFObjectHandle::replaceStreamData + that uses a stream data provider. Prior to qpdf 3.0.0, you had to + compute the stream length in advance so that qpdf could internally + verify that the stream data had the same length every time the + provider was invoked. Now this requirement is enforced a + different way, and the length parameter is no longer required. + Note that I take API-breaking changes very seriously and only did + it in this case since the lack of need to know length in advance + could significantly simplify people's code. If you were + previously going to a lot of trouble to compute the length of the + new stream data in advance, you now no longer have to do that. + You can just drop the length parameter and remove any code that + was previously computing the length. Thanks to Tobias Hoffmann + for pointing out how annoying the original interface was. + 2012-07-05 Jay Berkenbilt * Add QPDFWriter methods to write to an already open stdio FILE*. diff --git a/TODO b/TODO index 40c113d6..9adbaa73 100644 --- a/TODO +++ b/TODO @@ -17,8 +17,8 @@ Next * Document that your compiler has to support long long. - * Figure out why we have to specify a stream's length in advance when - providing stream data, and remove this restriction if possible. + * Make sure that the release notes call attention to the one API + breaking change: removal of length from replaceStreamData. * Add a way to create new QPDFObjectHandles with a string representation of them, such as diff --git a/examples/pdf-create.cc b/examples/pdf-create.cc index e1d75759..a9ad2389 100644 --- a/examples/pdf-create.cc +++ b/examples/pdf-create.cc @@ -17,7 +17,6 @@ class ImageProvider: public QPDFObjectHandle::StreamDataProvider virtual ~ImageProvider(); virtual void provideStreamData(int objid, int generation, Pipeline* pipeline); - size_t getLength() const; private: int width; @@ -45,12 +44,6 @@ ImageProvider::provideStreamData(int objid, int generation, pipeline->finish(); } -size_t -ImageProvider::getLength() const -{ - return 3 * width * height; -} - void usage() { std::cerr << "Usage: " << whoami << " filename" << std::endl @@ -111,8 +104,7 @@ static void create_pdf(char const* filename) PointerHolder provider(p); image.replaceStreamData(provider, QPDFObjectHandle::newNull(), - QPDFObjectHandle::newNull(), - p->getLength()); + QPDFObjectHandle::newNull()); // Create direct objects as needed by the page dictionary. QPDFObjectHandle procset = QPDFObjectHandle::newArray(); diff --git a/examples/pdf-invert-images.cc b/examples/pdf-invert-images.cc index 2dc73251..997fc37c 100644 --- a/examples/pdf-invert-images.cc +++ b/examples/pdf-invert-images.cc @@ -141,8 +141,7 @@ int main(int argc, char* argv[]) image.replaceStreamData( p, QPDFObjectHandle::newNull(), - QPDFObjectHandle::newNull(), - inv->image_data[objid][gen]->getSize()); + QPDFObjectHandle::newNull()); } } } diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index a61277a9..421b0144 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -294,18 +294,31 @@ class QPDFObjectHandle // directly providing a buffer with the stream data, call the // given provider's provideStreamData method. See comments on the // StreamDataProvider class (defined above) for details on the - // method. The provider must write the number of bytes as - // indicated by the length parameter, and the data must be - // consistent with filter and decode_parms as provided. Although - // it is more complex to use this form of replaceStreamData than - // the one that takes a buffer, it makes it possible to avoid - // allocating memory for the stream data. Example programs are - // provided that use both forms of replaceStreamData. + // method. The data must be consistent with filter and + // decode_parms as provided. Although it is more complex to use + // this form of replaceStreamData than the one that takes a + // buffer, it makes it possible to avoid allocating memory for the + // stream data. Example programs are provided that use both forms + // of replaceStreamData. + + // Note about stream length: for any given stream, the provider + // must provide the same amount of data each time it is called. + // This is critical for making linearization work properly. + // Versions of qpdf before 3.0.0 required a length to be specified + // here. Starting with version 3.0.0, this is no longer necessary + // (or permitted). The first time the stream data provider is + // invoked for a given stream, the actual length is stored. + // Subsequent times, it is enforced that the length be the same as + // the first time. + + // If you have gotten a compile error here while building code + // that worked with older versions of qpdf, just omit the length + // parameter. You can also simplify your code by not having to + // compute the length in advance. QPDF_DLL void replaceStreamData(PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length); + QPDFObjectHandle const& decode_parms); // return 0 for direct objects QPDF_DLL diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 4d6c5f79..c8b67416 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -414,12 +414,11 @@ QPDFObjectHandle::replaceStreamData(PointerHolder data, void QPDFObjectHandle::replaceStreamData(PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length) + QPDFObjectHandle const& decode_parms) { assertType("Stream", isStream()); dynamic_cast(obj.getPointer())->replaceStreamData( - provider, filter, decode_parms, length); + provider, filter, decode_parms); } int diff --git a/libqpdf/QPDF_Stream.cc b/libqpdf/QPDF_Stream.cc index ed5fb5be..c089bcc1 100644 --- a/libqpdf/QPDF_Stream.cc +++ b/libqpdf/QPDF_Stream.cc @@ -380,24 +380,33 @@ QPDF_Stream::pipeStreamData(Pipeline* pipeline, bool filter, this->stream_provider->provideStreamData( this->objid, this->generation, &count); qpdf_offset_t actual_length = count.getCount(); - qpdf_offset_t desired_length = - this->stream_dict.getKey("/Length").getIntValue(); - if (actual_length == desired_length) - { - QTC::TC("qpdf", "QPDF_Stream pipe use stream provider"); - } - else - { - QTC::TC("qpdf", "QPDF_Stream provider length mismatch"); - throw std::logic_error( - "stream data provider for " + - QUtil::int_to_string(this->objid) + " " + - QUtil::int_to_string(this->generation) + - " provided " + - QUtil::int_to_string(actual_length) + - " bytes instead of expected " + - QUtil::int_to_string(desired_length) + " bytes"); - } + qpdf_offset_t desired_length = 0; + if (this->stream_dict.hasKey("/Length")) + { + desired_length = this->stream_dict.getKey("/Length").getIntValue(); + if (actual_length == desired_length) + { + QTC::TC("qpdf", "QPDF_Stream pipe use stream provider"); + } + else + { + QTC::TC("qpdf", "QPDF_Stream provider length mismatch"); + throw std::logic_error( + "stream data provider for " + + QUtil::int_to_string(this->objid) + " " + + QUtil::int_to_string(this->generation) + + " provided " + + QUtil::int_to_string(actual_length) + + " bytes instead of expected " + + QUtil::int_to_string(desired_length) + " bytes"); + } + } + else + { + QTC::TC("qpdf", "QPDF_Stream provider length not provided"); + this->stream_dict.replaceKey( + "/Length", QPDFObjectHandle::newInteger(actual_length)); + } } else if (this->offset == 0) { @@ -430,12 +439,11 @@ void QPDF_Stream::replaceStreamData( PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length) + QPDFObjectHandle const& decode_parms) { this->stream_provider = provider; this->stream_data = 0; - replaceFilterData(filter, decode_parms, length); + replaceFilterData(filter, decode_parms, 0); } void @@ -445,6 +453,14 @@ QPDF_Stream::replaceFilterData(QPDFObjectHandle const& filter, { this->stream_dict.replaceOrRemoveKey("/Filter", filter); this->stream_dict.replaceOrRemoveKey("/DecodeParms", decode_parms); - this->stream_dict.replaceKey("/Length", - QPDFObjectHandle::newInteger((int)length)); + if (length == 0) + { + QTC::TC("qpdf", "QPDF_Stream unknown stream length"); + this->stream_dict.removeKey("/Length"); + } + else + { + this->stream_dict.replaceKey( + "/Length", QPDFObjectHandle::newInteger((int)length)); + } } diff --git a/libqpdf/qpdf/QPDF_Stream.hh b/libqpdf/qpdf/QPDF_Stream.hh index e74ae201..ce46d994 100644 --- a/libqpdf/qpdf/QPDF_Stream.hh +++ b/libqpdf/qpdf/QPDF_Stream.hh @@ -30,8 +30,7 @@ class QPDF_Stream: public QPDFObject void replaceStreamData( PointerHolder provider, QPDFObjectHandle const& filter, - QPDFObjectHandle const& decode_parms, - size_t length); + QPDFObjectHandle const& decode_parms); // Replace object ID and generation. This may only be called if // object ID and generation are 0. It is used by QPDFObjectHandle diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 93bbf88d..b2c98496 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -215,3 +215,5 @@ QPDFObjectHandle shallow copy dictionary 0 QPDFObjectHandle shallow copy scalar 0 QPDFObjectHandle newStream with string 0 QPDF unknown key not inherited 0 +QPDF_Stream provider length not provided 0 +QPDF_Stream unknown stream length 0 diff --git a/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf b/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf index 8da1663f0e8bf12e342e788077fd6bd85a9fa86a..029311702ca07a4df91a8e8e6d5153a435b600a7 100644 GIT binary patch literal 1419 zcmc(f-D(p-6vt~3g?$6>j)g!AwzIRdpHL!g(+$_1)lt(k)80Z6X0L7~u@te)z$r4_6=B3TlQA}|jz60k;CO~= z+73@~V_9HVWXVN3z>1YFKHgGHD4=vGPra^$Nd`S3!JS144vJI-f_k}xSXOElycI)j zGe9>GpMqgRaDsCOb24-YLYyym63Jz<0>P=a9n9CN-d2|%Vmb{UjV8ttj!KuQT+637 zc_#8YNK=e1g^UIqO&X&PQ9J~45XBpD7TpH+_!$a>X_4N=vCIWlc4)WAxUE8E%@`UOr(d z&S*6GFO!0>8Q3^GJD~AJ`N_elB?{(~4VdL7douA%j$#g+ydOwkWsYYwpS*@ymZPF5 zHI2(a!D8}F7U_x4Me7aBEfowvKp{_o3(PPuFf_Kn5Hm6c+JmOf7>iyLbD&Sq)R`iy zD=A9M%t p = provider; qstream.replaceStreamData( p, QPDFObjectHandle::newName("/FlateDecode"), - QPDFObjectHandle::newNull(), b->getSize()); + QPDFObjectHandle::newNull()); + provider->badLength(false); + QPDFWriter w(pdf, "a.pdf"); + w.setStaticID(true); + // Linearize to force the provider to be called multiple times. + w.setLinearization(true); + w.setStreamDataMode(qpdf_s_preserve); + w.write(); + + // Every time a provider pipes stream data, it has to provide + // the same amount of data. provider->badLength(true); try { @@ -489,11 +499,6 @@ void runtest(int n, char const* filename) { std::cout << "exception: " << e.what() << std::endl; } - provider->badLength(false); - QPDFWriter w(pdf, "a.pdf"); - w.setStaticID(true); - w.setStreamDataMode(qpdf_s_preserve); - w.write(); } else if (n == 9) { diff --git a/qpdf/test_large_file.cc b/qpdf/test_large_file.cc index fc2c7bbc..f54adb33 100644 --- a/qpdf/test_large_file.cc +++ b/qpdf/test_large_file.cc @@ -109,7 +109,6 @@ class ImageProvider: public QPDFObjectHandle::StreamDataProvider virtual ~ImageProvider(); virtual void provideStreamData(int objid, int generation, Pipeline* pipeline); - size_t getLength() const; private: int n; @@ -142,12 +141,6 @@ ImageProvider::provideStreamData(int objid, int generation, pipeline->finish(); } -size_t -ImageProvider::getLength() const -{ - return width * height; -} - void usage() { std::cerr << "Usage: " << whoami << " {read|write} {large|small} outfile" @@ -229,8 +222,7 @@ static void create_pdf(char const* filename) PointerHolder provider(p); image.replaceStreamData(provider, QPDFObjectHandle::newNull(), - QPDFObjectHandle::newNull(), - p->getLength()); + QPDFObjectHandle::newNull()); QPDFObjectHandle xobject = QPDFObjectHandle::newDictionary(); xobject.replaceKey("/Im1", image);