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.

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.
Jay Berkenbilt
1 parent 8705e2e8
Showing 12 changed files with 100 additions and 65 deletions
ChangeLog
TODO
examples/pdf-create.cc
examples/pdf-invert-images.cc
include/qpdf/QPDFObjectHandle.hh
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDF_Stream.cc
libqpdf/qpdf/QPDF_Stream.hh
qpdf/qpdf.testcov
qpdf/qtest/qpdf/replaced-stream-data-flate.pdf
qpdf/test_driver.cc
qpdf/test_large_file.cc
+2012-07-07  Jay Berkenbilt  <ejb@ql.org>
+
+	* 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  <ejb@ql.org>
  
 	* Add QPDFWriter methods to write to an already open stdio FILE*.
@@ -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
@@ -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<QPDFObjectHandle::StreamDataProvider> 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();
@@ -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());
 		    }
 		}
 	    }
@@ -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<StreamDataProvider> provider,
 			   QPDFObjectHandle const& filter,
-			   QPDFObjectHandle const& decode_parms,
-			   size_t length);
+			   QPDFObjectHandle const& decode_parms);
  
     // return 0 for direct objects
     QPDF_DLL
@@ -414,12 +414,11 @@ QPDFObjectHandle::replaceStreamData(PointerHolder&lt;Buffer&gt; data,
 void
 QPDFObjectHandle::replaceStreamData(PointerHolder<StreamDataProvider> provider,
 				    QPDFObjectHandle const& filter,
-				    QPDFObjectHandle const& decode_parms,
-				    size_t length)
+				    QPDFObjectHandle const& decode_parms)
 {
     assertType("Stream", isStream());
     dynamic_cast<QPDF_Stream*>(obj.getPointer())->replaceStreamData(
-	provider, filter, decode_parms, length);
+	provider, filter, decode_parms);
 }
  
 int
@@ -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<QPDFObjectHandle::StreamDataProvider> 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&amp; 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));
+    }
 }
@@ -30,8 +30,7 @@ class QPDF_Stream: public QPDFObject
     void replaceStreamData(
 	PointerHolder<QPDFObjectHandle::StreamDataProvider> 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
@@ -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
@@ -478,7 +478,17 @@ void runtest(int n, char const* filename)
 	PointerHolder<QPDFObjectHandle::StreamDataProvider> 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)
     {
@@ -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<QPDFObjectHandle::StreamDataProvider> provider(p);
         image.replaceStreamData(provider,
                                 QPDFObjectHandle::newNull(),
-                                QPDFObjectHandle::newNull(),
-                                p->getLength());
+                                QPDFObjectHandle::newNull());
  
         QPDFObjectHandle xobject = QPDFObjectHandle::newDictionary();
         xobject.replaceKey("/Im1", image);