Support --show-encryption without a valid password (fixes #598)

7 files changed, 98 insertions, 38 deletions

diff --git a/ChangeLog b/ChangeLog
index e31ab9f9..de16280e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2022-09-06  Jay Berkenbilt  <ejb@ql.org>
+
+        * The --show-encryption option now works even if a correct
+        password is not supplied. If you were using --show-encryption to
+        test whether you have the right password, use --requires-password
+        instead. Fixes #598.
+
 2022-09-05  Jay Berkenbilt  <ejb@ql.org>
 
         * Add a move constructor to Buffer, making it possible to move
diff --git a/include/qpdf/QPDFJob.hh b/include/qpdf/QPDFJob.hh
index 2d4ab0d2..2d552f64 100644
--- a/include/qpdf/QPDFJob.hh
+++ b/include/qpdf/QPDFJob.hh
@@ -490,22 +490,26 @@ class QPDFJob
     std::vector<int> parseNumrange(char const* range, int max);
 
     // Basic file processing
-    std::shared_ptr<QPDF> processFile(
+    void processFile(
+        std::shared_ptr<QPDF>&,
         char const* filename,
         char const* password,
         bool used_for_input,
         bool main_input);
-    std::shared_ptr<QPDF> processInputSource(
+    void processInputSource(
+        std::shared_ptr<QPDF>&,
         std::shared_ptr<InputSource> is,
         char const* password,
         bool used_for_input);
-    std::shared_ptr<QPDF> doProcess(
+    void doProcess(
+        std::shared_ptr<QPDF>&,
         std::function<void(QPDF*, char const*)> fn,
         char const* password,
         bool empty,
         bool used_for_input,
         bool main_input);
-    std::shared_ptr<QPDF> doProcessOnce(
+    void doProcessOnce(
+        std::shared_ptr<QPDF>&,
         std::function<void(QPDF*, char const*)> fn,
         char const* password,
         bool empty,
diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc
index 8bf619a2..9530420e 100644
--- a/libqpdf/QPDFJob.cc
+++ b/libqpdf/QPDFJob.cc
@@ -564,22 +564,27 @@ void
 QPDFJob::run()
 {
     checkConfiguration();
-    std::shared_ptr<QPDF> pdf_ph;
+    std::shared_ptr<QPDF> pdf_sp;
     try {
-        pdf_ph =
-            processFile(m->infilename.get(), m->password.get(), true, true);
+        processFile(pdf_sp, m->infilename.get(), m->password.get(), true, true);
     } catch (QPDFExc& e) {
-        if ((e.getErrorCode() == qpdf_e_password) &&
-            (m->check_is_encrypted || m->check_requires_password)) {
-            // Allow --is-encrypted and --requires-password to
-            // work when an incorrect password is supplied.
-            this->m->encryption_status =
-                qpdf_es_encrypted | qpdf_es_password_incorrect;
-            return;
+        if (e.getErrorCode() == qpdf_e_password) {
+            // Allow certain operations to work when an incorrect
+            // password is supplied.
+            if (m->check_is_encrypted || m->check_requires_password) {
+                this->m->encryption_status =
+                    qpdf_es_encrypted | qpdf_es_password_incorrect;
+                return;
+            }
+            if (m->show_encryption && pdf_sp) {
+                this->m->log->info("Incorrect password supplied\n");
+                showEncryption(*pdf_sp);
+                return;
+            }
         }
         throw e;
     }
-    QPDF& pdf = *pdf_ph;
+    QPDF& pdf = *pdf_sp;
     if (pdf.isEncrypted()) {
         this->m->encryption_status = qpdf_es_encrypted;
     }
@@ -1981,15 +1986,16 @@ QPDFJob::doInspection(QPDF& pdf)
     }
 }
 
-std::shared_ptr<QPDF>
+void
 QPDFJob::doProcessOnce(
+    std::shared_ptr<QPDF>& pdf,
     std::function<void(QPDF*, char const*)> fn,
     char const* password,
     bool empty,
     bool used_for_input,
     bool main_input)
 {
-    auto pdf = QPDF::create();
+    pdf = QPDF::create();
     setQPDFOptions(*pdf);
     if (empty) {
         pdf->emptyPDF();
@@ -2002,11 +2008,11 @@ QPDFJob::doProcessOnce(
         this->m->max_input_version.updateIfGreater(
             pdf->getVersionAsPDFVersion());
     }
-    return pdf;
 }
 
-std::shared_ptr<QPDF>
+void
 QPDFJob::doProcess(
+    std::shared_ptr<QPDF>& pdf,
     std::function<void(QPDF*, char const*)> fn,
     char const* password,
     bool empty,
@@ -2037,7 +2043,8 @@ QPDFJob::doProcess(
         m->suppress_password_recovery) {
         // There is no password, or we're not doing recovery, so just
         // do the normal processing with the supplied password.
-        return doProcessOnce(fn, password, empty, used_for_input, main_input);
+        doProcessOnce(pdf, fn, password, empty, used_for_input, main_input);
+        return;
     }
 
     // Get a list of otherwise encoded strings. Keep in scope for this
@@ -2065,7 +2072,8 @@ QPDFJob::doProcess(
     bool warned = false;
     for (auto iter = passwords.begin(); iter != passwords.end(); ++iter) {
         try {
-            return doProcessOnce(fn, *iter, empty, used_for_input, main_input);
+            doProcessOnce(pdf, fn, *iter, empty, used_for_input, main_input);
+            return;
         } catch (QPDFExc& e) {
             auto next = iter;
             ++next;
@@ -2086,8 +2094,9 @@ QPDFJob::doProcess(
     throw std::logic_error("do_process returned");
 }
 
-std::shared_ptr<QPDF>
+void
 QPDFJob::processFile(
+    std::shared_ptr<QPDF>& pdf,
     char const* filename,
     char const* password,
     bool used_for_input,
@@ -2096,17 +2105,25 @@ QPDFJob::processFile(
     auto f1 = std::mem_fn<void(char const*, char const*)>(&QPDF::processFile);
     auto fn =
         std::bind(f1, std::placeholders::_1, filename, std::placeholders::_2);
-    return doProcess(
-        fn, password, strcmp(filename, "") == 0, used_for_input, main_input);
+    doProcess(
+        pdf,
+        fn,
+        password,
+        strcmp(filename, "") == 0,
+        used_for_input,
+        main_input);
 }
 
-std::shared_ptr<QPDF>
+void
 QPDFJob::processInputSource(
-    std::shared_ptr<InputSource> is, char const* password, bool used_for_input)
+    std::shared_ptr<QPDF>& pdf,
+    std::shared_ptr<InputSource> is,
+    char const* password,
+    bool used_for_input)
 {
     auto f1 = std::mem_fn(&QPDF::processInputSource);
     auto fn = std::bind(f1, std::placeholders::_1, is, std::placeholders::_2);
-    return doProcess(fn, password, false, used_for_input, false);
+    doProcess(pdf, fn, password, false, used_for_input, false);
 }
 
 void
@@ -2117,8 +2134,7 @@ QPDFJob::validateUnderOverlay(QPDF& pdf, UnderOverlay* uo)
     }
     QPDFPageDocumentHelper main_pdh(pdf);
     int main_npages = QIntC::to_int(main_pdh.getAllPages().size());
-    uo->pdf =
-        processFile(uo->filename.c_str(), uo->password.get(), true, false);
+    processFile(uo->pdf, uo->filename.c_str(), uo->password.get(), true, false);
     QPDFPageDocumentHelper uo_pdh(*(uo->pdf));
     int uo_npages = QIntC::to_int(uo_pdh.getAllPages().size());
     try {
@@ -2375,8 +2391,13 @@ QPDFJob::copyAttachments(QPDF& pdf)
             v << prefix << ": copying attachments from " << to_copy.path
               << "\n";
         });
-        auto other = processFile(
-            to_copy.path.c_str(), to_copy.password.c_str(), false, false);
+        std::shared_ptr<QPDF> other;
+        processFile(
+            other,
+            to_copy.path.c_str(),
+            to_copy.password.c_str(),
+            false,
+            false);
         QPDFEmbeddedFileDocumentHelper other_efdh(*other);
         auto other_attachments = other_efdh.getEmbeddedFiles();
         for (auto const& iter: other_attachments) {
@@ -2702,10 +2723,10 @@ QPDFJob::handlePageSpecs(
                     new FileInputSource(page_spec.filename.c_str());
                 is = std::shared_ptr<InputSource>(fis);
             }
-            std::shared_ptr<QPDF> qpdf_ph =
-                processInputSource(is, password, true);
-            page_heap.push_back(qpdf_ph);
-            page_spec_qpdfs[page_spec.filename] = qpdf_ph.get();
+            std::shared_ptr<QPDF> qpdf_sp;
+            processInputSource(qpdf_sp, is, password, true);
+            page_heap.push_back(qpdf_sp);
+            page_spec_qpdfs[page_spec.filename] = qpdf_sp.get();
             if (cis) {
                 cis->stayOpen(false);
                 page_spec_cfis[page_spec.filename] = cis;
@@ -3209,7 +3230,9 @@ QPDFJob::setWriterOptions(QPDF& pdf, QPDFWriter& w)
         w.setSuppressOriginalObjectIDs(true);
     }
     if (m->copy_encryption) {
-        std::shared_ptr<QPDF> encryption_pdf = processFile(
+        std::shared_ptr<QPDF> encryption_pdf;
+        processFile(
+            encryption_pdf,
             m->encryption_file.c_str(),
             m->encryption_file_password.get(),
             false,
diff --git a/manual/release-notes.rst b/manual/release-notes.rst
index 8a783a87..60e90e12 100644
--- a/manual/release-notes.rst
+++ b/manual/release-notes.rst
@@ -85,6 +85,11 @@ For a detailed list of changes, please see the file
 
   - CLI: breaking changes
 
+    - The :qpdf:ref:`--show-encryption` flag now provides encryption
+      information even if a correct password is not supplied. If you
+      were relying on its not working in this case, see
+      :qpdf:ref:`--requires-password` for a reliable test.
+
     - The default json output version when :qpdf:ref:`--json` is
       specified has been changed from ``1`` to ``latest``, which is
       now ``2``.
diff --git a/qpdf/qtest/encryption.test b/qpdf/qtest/encryption.test
index 27974a82..672eb995 100644
--- a/qpdf/qtest/encryption.test
+++ b/qpdf/qtest/encryption.test
@@ -112,7 +112,7 @@ my @encrypted_files =
       1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
     );
 
-$n_tests += 8 + (2 * (@encrypted_files)) + (7 * (@encrypted_files - 6)) + 9;
+$n_tests += 8 + (2 * (@encrypted_files)) + (7 * (@encrypted_files - 6)) + 10;
 
 $td->runtest("encrypted file",
              {$td->COMMAND => "test_driver 2 encrypted-with-images.pdf"},
@@ -365,6 +365,12 @@ $td->runtest("C API: invalid password",
                   "qpdf-ctest 2 enc-R2,V1,U=view,O=view.pdf '' a.qdf"},
              {$td->FILE => "c-invalid-password.out", $td->EXIT_STATUS => 0},
              $td->NORMALIZE_NEWLINES);
+$td->runtest("show-encryption works invalid password",
+             {$td->COMMAND => "qpdf --show-encryption --password=quack" .
+                  " enc-R2,V1,U=view,O=view.pdf"},
+             {$td->FILE => "invalid-password-encrypt.out",
+                  $td->EXIT_STATUS => 0},
+             $td->NORMALIZE_NEWLINES);
 
 my @cenc = (
     [11, 'hybrid-xref.pdf', "''", 'r2', "", ""],
diff --git a/qpdf/qtest/qpdf/invalid-password-encrypt.out b/qpdf/qtest/qpdf/invalid-password-encrypt.out
new file mode 100644
index 00000000..6e97b379
--- /dev/null
+++ b/qpdf/qtest/qpdf/invalid-password-encrypt.out
@@ -0,0 +1,13 @@
+Incorrect password supplied
+R = 2
+P = -64
+User password = 
+extract for accessibility: not allowed
+extract for any purpose: not allowed
+print low resolution: not allowed
+print high resolution: not allowed
+modify document assembly: not allowed
+modify forms: not allowed
+modify annotations: not allowed
+modify other: not allowed
+modify anything: not allowed
diff --git a/qpdf/qtest/unicode-password.test b/qpdf/qtest/unicode-password.test
index eb78de42..db0c8bf8 100644
--- a/qpdf/qtest/unicode-password.test
+++ b/qpdf/qtest/unicode-password.test
@@ -147,9 +147,11 @@ foreach my $d (@unicode_pw_cases)
         }
         my $r_output = "";
         $r_output .= "trying other\n" if $tried_others;
+        my $arg = "--show-encryption";
         if ($xfail)
         {
             $r_output .= "qpdf: a.pdf: invalid password\n";
+            $arg = "--check";
         }
         else
         {
@@ -162,7 +164,7 @@ foreach my $d (@unicode_pw_cases)
         $r_xargs .= $strict ? ' --suppress-password-recovery' : '';
         $td->runtest("decrypt $pw, $r_encoding, strict=$strict",
                      {$td->COMMAND =>
-                          "qpdf --show-encryption --verbose" .
+                          "qpdf $arg --verbose" .
                           " $r_xargs a.pdf \@$r_pfile",
                           $td->FILTER => "perl show-unicode-encryption.pl"},
                      {$td->STRING => "$r_output",