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.

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  <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*.
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<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();
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<StreamDataProvider> 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<Buffer> 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
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<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& 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<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
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 8da1663f..02931170 100644
--- a/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf
+++ b/qpdf/qtest/qpdf/replaced-stream-data-flate.pdf
diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc
index 89b422ac..8373bd04 100644
--- a/qpdf/test_driver.cc
+++ b/qpdf/test_driver.cc
@@ -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)
     {
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<QPDFObjectHandle::StreamDataProvider> provider(p);
         image.replaceStreamData(provider,
                                 QPDFObjectHandle::newNull(),
-                                QPDFObjectHandle::newNull(),
-                                p->getLength());
+                                QPDFObjectHandle::newNull());
 
         QPDFObjectHandle xobject = QPDFObjectHandle::newDictionary();
         xobject.replaceKey("/Im1", image);