TODO and comments item for pipeContentStreams

3 files changed, 30 insertions, 2 deletions

diff --git a/TODO b/TODO
index c7ad513e..242c59c8 100644
--- a/TODO
+++ b/TODO
@@ -10,6 +10,23 @@ Candidates for upcoming release
 
 * See if we can work in Windows Build/External Libraries (below)
 
+* QPDFObjectHandle::pipeContentStreams calls finish() after each
+  stream. In some code paths, Pl_Concatenate is used, which suppresses
+  that, but in other code paths, it's not used, and the library relies
+  on the behavior of finish() being called. Then there's the issue of
+  nested Pl_Concatenate pipelines -- calling manualFinish() on the top
+  one doesn't call manualFinish() on the lower ones, and there are no
+  exposed methods that allow us to apply things down the pipeline
+  stack, so it's hard to fix this without changing the API (at least
+  making Pipeline::getNext() public, which may be undesirable). To see
+  this problem in action, stick a Pl_Concatenate in front of the
+  pipeline in pipeContentStreams and observe the test failure. One
+  solution might be to add an additional argument indicating whether
+  or not to delay calling finish() until the end. See comments on
+  QPDFPageObjectHelper::filterPageContents,
+  QPDFObjectHandle::filterPageContents, and
+  QPDFObjectHandle::pipeContentStreams
+
 * Remember to check work `qpdf` project for private issues
   * file with very slow page extraction
   * big page even with --remove-unreferenced-resources=yes, even with --empty
diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh
index ef6ff42e..7738fd77 100644
--- a/include/qpdf/QPDFObjectHandle.hh
+++ b/include/qpdf/QPDFObjectHandle.hh
@@ -393,7 +393,12 @@ class QPDFObjectHandle
     // messages. The all_description argument is initialized to
     // something that could be used to describe the result of the
     // pipeline. It is the description amended with the identifiers of
-    // the underlying objects.
+    // the underlying objects. Please note that if there is an array
+    // of content streams, p->finish() is called after each stream. If
+    // you pass a pipeline that doesn't allow write() to be called
+    // after finish(), you can wrap it in an instance of
+    // Pl_Concatenate and then call manualFinish() on the
+    // Pl_Concatenate pipeline at the end.
     QPDF_DLL
     void pipeContentStreams(Pipeline* p, std::string const& description,
                             std::string& all_description);
@@ -419,6 +424,7 @@ class QPDFObjectHandle
     void parsePageContents(ParserCallbacks* callbacks);
     QPDF_DLL
     void filterPageContents(TokenFilter* filter, Pipeline* next = 0);
+    // See comments for QPDFPageObjectHelper::pipePageContents.
     QPDF_DLL
     void pipePageContents(Pipeline* p);
     QPDF_DLL
diff --git a/include/qpdf/QPDFPageObjectHelper.hh b/include/qpdf/QPDFPageObjectHelper.hh
index 87dad8d4..a197ece7 100644
--- a/include/qpdf/QPDFPageObjectHelper.hh
+++ b/include/qpdf/QPDFPageObjectHelper.hh
@@ -145,7 +145,12 @@ class QPDFPageObjectHelper: public QPDFObjectHelper
 
     // Pipe a page's contents through the given pipeline. This method
     // works whether the contents are a single stream or an array of
-    // streams. Call on a page object.
+    // streams. Call on a page object. Please note that if there is an
+    // array of content streams, p->finish() is called after each
+    // stream. If you pass a pipeline that doesn't allow write() to be
+    // called after finish(), you can wrap it in an instance of
+    // Pl_Concatenate and then call manualFinish() on the
+    // Pl_Concatenate pipeline at the end.
     QPDF_DLL
     void pipePageContents(Pipeline* p);