Refactor `enqueueObject` to `enqueue`, update related method signatures and logi…

…c across `QPDFWriter` for consistency and improved readability.

Refactor `enqueueObject` to `enqueue`, update related method signatures and logi…
…c across `QPDFWriter` for consistency and improved readability.
m-holger
1 parent 7e1237af
Showing 4 changed files with 64 additions and 45 deletions
libqpdf/QPDFWriter.cc
qpdf/qtest/qpdf/bad-direct-root.out
qpdf/qtest/qpdf/bad-direct-root.pdf
qpdf/qtest/specific-file.test
@@ -395,7 +395,7 @@ class QPDFWriter::Members: QPDF::Doc::Writer
     void preserveObjectStreams();
     void generateObjectStreams();
     void initializeSpecialStreams();
-    void enqueueObject(QPDFObjectHandle object);
+    void enqueue(QPDFObjectHandle const& object);
     void enqueueObjectsStandard();
     void enqueueObjectsPCLm();
     void enqueuePart(std::vector<QPDFObjectHandle>& part);
@@ -1366,18 +1366,19 @@ QPDFWriter::Members::assignCompressedObjectNumbers(QPDFObjGen og)
 }
 void
-QPDFWriter::Members::enqueueObject(QPDFObjectHandle object)
+QPDFWriter::Members::enqueue(QPDFObjectHandle const& object)
 {
-    if (object.isIndirect()) {
-        // This owner check can only be done for indirect objects. It is possible for a direct
-        // object to have an owning QPDF that is from another file if a direct QPDFObjectHandle from
-        // one file was insert into another file without copying. Doing that is safe even if the
-        // original QPDF gets destroyed, which just disconnects the QPDFObjectHandle from its owner.
-        if (object.getOwningQPDF() != &qpdf) {
-            throw std::logic_error(
-                "QPDFObjectHandle from different QPDF found while writing.  Use "
-                "QPDF::copyForeignObject to add objects from another file.");
-        }
+    if (object.indirect()) {
+        util::assertion(
+            // This owner check can only be done for indirect objects. It is possible for a direct
+            // object to have an owning QPDF that is from another file if a direct QPDFObjectHandle
+            // from one file was insert into another file without copying. Doing that is safe even
+            // if the original QPDF gets destroyed, which just disconnects the QPDFObjectHandle from
+            // its owner.
+            object.qpdf() == &qpdf,
+            "QPDFObjectHandle from different QPDF found while writing.  "
+            "Use QPDF::copyForeignObject to add objects from another file." //
+        );
         if (qdf_mode && object.isStreamOfType("/XRef")) {
             // As a special case, do not output any extraneous XRef streams in QDF mode. Doing so
@@ -1397,7 +1398,7 @@ QPDFWriter::Members::enqueueObject(QPDFObjectHandle object)
                 // stream.  Object streams always have generation 0.
                 // Detect loops by storing invalid object ID -1, which will get overwritten later.
                 o.renumber = -1;
-                enqueueObject(qpdf.getObject(o.object_stream, 0));
+                enqueue(qpdf.getObject(o.object_stream, 0));
             } else {
                 object_queue.emplace_back(object);
                 o.renumber = next_objid++;
@@ -1413,25 +1414,25 @@ QPDFWriter::Members::enqueueObject(QPDFObjectHandle object)
                     ++next_objid;
                 }
             }
-        } else if (o.renumber == -1) {
-            // This can happen if a specially constructed file indicates that an object stream is
-            // inside itself.
         }
         return;
-    } else if (!linearized) {
-        if (object.isArray()) {
-            for (auto& item: object.as_array()) {
-                enqueueObject(item);
-            }
-        } else if (auto d = object.as_dictionary()) {
-            for (auto const& item: d) {
-                if (!item.second.null()) {
-                    enqueueObject(item.second);
-                }
-            }
+    }
+
+    if (linearized) {
+        return;
+    }
+
+    if (Array array = object) {
+        for (auto& item: array) {
+            enqueue(item);
+        }
+        return;
+    }
+
+    for (auto const& item: Dictionary(object)) {
+        if (!item.second.null()) {
+            enqueue(item.second);
         }
-    } else {
-        // ignore
     }
 }
@@ -1439,9 +1440,9 @@ void
 QPDFWriter::Members::unparseChild(QPDFObjectHandle const& child, size_t level, int flags)
 {
     if (!linearized) {
-        enqueueObject(child);
+        enqueue(child);
     }
-    if (child.isIndirect()) {
+    if (child.indirect()) {
         write(obj[child].renumber).write(" 0 R");
     } else {
         unparseObject(child, level, flags);
@@ -2490,7 +2491,7 @@ void
 QPDFWriter::Members::enqueuePart(std::vector<QPDFObjectHandle>& part)
 {
     for (auto const& oh: part) {
-        enqueueObject(oh);
+        enqueue(oh);
     }
 }
@@ -2504,7 +2505,7 @@ QPDFWriter::Members::writeEncryptionDictionary()
     write("<<");
     if (V >= 4) {
         write(" /CF << /StdCF << /AuthEvent /DocOpen /CFM ");
-        write(encrypt_use_aes ? ((V < 5) ? "/AESV2" : "/AESV3") : "/V2");
+        write(encrypt_use_aes ? (V < 5 ? "/AESV2" : "/AESV3") : "/V2");
         // The PDF spec says the /Length key is optional, but the PDF previewer on some versions of
         // MacOS won't open encrypted files without it.
         write((V < 5) ? " /Length 16 >> >>" : " /Length 32 >> >>");
@@ -3108,19 +3109,19 @@ QPDFWriter::Members::enqueueObjectsStandard()
 {
     if (preserve_unreferenced_objects) {
         for (auto const& oh: qpdf.getAllObjects()) {
-            enqueueObject(oh);
+            enqueue(oh);
         }
     }
     // Put root first on queue.
     auto trailer = trimmed_trailer();
-    enqueueObject(trailer["/Root"]);
+    enqueue(trailer["/Root"]);
     // Next place any other objects referenced from the trailer dictionary into the queue, handling
     // direct objects recursively. Root is already there, so enqueuing it a second time is a no-op.
     for (auto& item: trailer) {
         if (!item.second.null()) {
-            enqueueObject(item.second);
+            enqueue(item.second);
         }
     }
 }
@@ -3134,22 +3135,19 @@ QPDFWriter::Members::enqueueObjectsPCLm()
     // enqueue all pages first
     for (auto& page: pages) {
-        // enqueue page
-        enqueueObject(page);
-
-        // enqueue page contents stream
-        enqueueObject(page["/Contents"]);
+        enqueue(page);
+        enqueue(page["/Contents"]);
         // enqueue all the strips for each page
         for (auto& image: Dictionary(page["/Resources"]["/XObject"])) {
             if (!image.second.null()) {
-                enqueueObject(image.second);
-                enqueueObject(QPDFObjectHandle::newStream(&qpdf, image_transform_content));
+                enqueue(image.second);
+                enqueue(qpdf.newStream(image_transform_content));
             }
         }
     }
-    enqueueObject(trimmed_trailer()["/Root"]);
+    enqueue(trimmed_trailer()["/Root"]);
 }
 void
+WARNING: bad-direct-root.pdf: can't find PDF header
+WARNING: bad-direct-root.pdf: file is damaged
+WARNING: bad-direct-root.pdf: can't find startxref
+WARNING: bad-direct-root.pdf: Attempting to reconstruct cross-reference table
+WARNING: bad-direct-root.pdf (trailer, offset 249): unknown token while reading object; treating as null
+WARNING: bad-direct-root.pdf (trailer, offset 261): unknown token while reading object; treating as null
+WARNING: bad-direct-root.pdf (trailer, offset 186): expected dictionary keys but found non-name objects; ignoring
+WARNING: bad-direct-root.pdf (object 1 0, offset 65): expected endobj
+WARNING: bad-direct-root.pdf (object 2 0, offset 114): unknown token while reading object; treating as null
+WARNING: bad-direct-root.pdf (object 2 0, offset 122): invalid character (/) in hexstring
+WARNING: bad-direct-root.pdf (object 2 0, offset 125): unknown token while reading object; treating as null
+WARNING: bad-direct-root.pdf (object 2 0, offset 114): expected dictionary keys but found non-name objects; ignoring
+WARNING: bad-direct-root.pdf (object 2 0, offset 141): expected endobj
+WARNING: bad-direct-root.pdf, object 2 0 at offset 85: kid 0 (from 0) MediaBox is undefined; setting to letter / ANSI A
+WARNING: bad-direct-root.pdf, object 2 0 at offset 85: /Type key should be /Page but is not; overriding
+qpdf: error encountered after writing part 4 of linearized data
@@ -14,7 +14,7 @@ cleanup();
 my $td = new TestDriver('specific-file');
-my $n_tests = 11;
+my $n_tests = 12;
 # Special PDF files that caused problems at some point
@@ -63,6 +63,11 @@ $td-&gt;runtest(&quot;Acroform /DR with indirect subkey&quot;,
 $td->runtest("check output",
              {$td->FILE => "a.pdf"},
              {$td->FILE => "dr-with-indirect-item-out.pdf"});
+$td->runtest("Bad direct Root",
+             {$td->COMMAND =>
+                  "qpdf --static-id bad-direct-root.pdf --linearize a.pdf"},
+             {$td->FILE => "bad-direct-root.out", $td->EXIT_STATUS => 2},
+             $td->NORMALIZE_NEWLINES);
 cleanup();
 $td->report($n_tests);