Remove remaining incorrect assert calls from implementation

Jay Berkenbilt
1 parent b20f0519
Showing 10 changed files with 25 additions and 20 deletions
README-maintainer
libqpdf/AES_PDF_native.cc
libqpdf/Pl_AES_PDF.cc
libqpdf/Pl_Buffer.cc
libqpdf/Pl_LZWDecoder.cc
libqpdf/QPDFWriter.cc
libqpdf/QPDF_encryption.cc
libqpdf/QPDF_linearization.cc
libqpdf/QPDF_optimization.cc
libqpdf/QPDF_pages.cc
@@ -123,6 +123,12 @@ CODING RULES
   "Code Formatting" section in manual/contributing.rst for details.
   See also "CODE FORMATTING" below.
+* Do not use assert in non-test code for any purpose other than as a
+  sanity check during development that would be safe to remove in
+  production. assert is for strong invariant checking. When developing
+  and using assert for that purpose, make sure to use the Debug
+  configuration since assert is disabled in other configurations.
+
 * In a source file, include the header file that declares the source
   class first followed by a blank line. If a config file is needed
   first, put a blank line between that and the header followed by
@@ -4,7 +4,6 @@
 #include <qpdf/QPDFCryptoImpl.hh>
 #include <qpdf/QUtil.hh>
 #include <qpdf/rijndael.h>
-#include <assert.h>
 #include <cstring>
 #include <stdexcept>
 #include <stdlib.h>
@@ -3,7 +3,6 @@
 #include <qpdf/QIntC.hh>
 #include <qpdf/QPDFCryptoProvider.hh>
 #include <qpdf/QUtil.hh>
-#include <assert.h>
 #include <cstring>
 #include <stdexcept>
 #include <stdlib.h>
@@ -115,7 +114,10 @@ Pl_AES_PDF::finish()
             // encountered files for which the output is not a
             // multiple of the block size.  In this case, pad with
             // zeroes and hope for the best.
-            assert(this->buf_size > this->offset);
+            if (this->offset >= this->buf_size) {
+                throw std::logic_error("buffer overflow in AES encryption"
+                                       " pipeline");
+            }
             std::memset(
                 this->inbuf + this->offset, 0, this->buf_size - this->offset);
             this->offset = this->buf_size;
@@ -147,7 +149,10 @@ Pl_AES_PDF::initializeVector()
 void
 Pl_AES_PDF::flush(bool strip_padding)
 {
-    assert(this->offset == this->buf_size);
+    if (this->offset != this->buf_size) {
+        throw std::logic_error(
+            "AES pipeline: flush called when buffer was not full");
+    }
     if (first) {
         first = false;
 #include <qpdf/Pl_Buffer.hh>
 #include <algorithm>
-#include <assert.h>
 #include <stdexcept>
 #include <stdlib.h>
 #include <string.h>
@@ -3,7 +3,6 @@
 #include <qpdf/QIntC.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
-#include <assert.h>
 #include <stdexcept>
 #include <string.h>
@@ -11,18 +11,17 @@
 #include <qpdf/Pl_PNGFilter.hh>
 #include <qpdf/Pl_RC4.hh>
 #include <qpdf/Pl_StdioFile.hh>
-#include <qpdf/QTC.hh>
-#include <qpdf/QUtil.hh>
-#include <qpdf/RC4.hh>
-#include <assert.h>
-
 #include <qpdf/QIntC.hh>
 #include <qpdf/QPDF.hh>
 #include <qpdf/QPDFObjectHandle.hh>
 #include <qpdf/QPDF_Name.hh>
 #include <qpdf/QPDF_String.hh>
+#include <qpdf/QTC.hh>
+#include <qpdf/QUtil.hh>
+#include <qpdf/RC4.hh>
 #include <algorithm>
+#include <cassert>
 #include <stdlib.h>
 QPDFWriter::Members::Members(QPDF& pdf) :
@@ -15,7 +15,7 @@
 #include <qpdf/RC4.hh>
 #include <algorithm>
-#include <assert.h>
+#include <cassert>
 #include <string.h>
 static unsigned char const padding_string[] = {
@@ -12,7 +12,6 @@
 #include <qpdf/QUtil.hh>
 #include <algorithm>
-#include <assert.h>
 #include <iostream>
 #include <math.h>
 #include <string.h>
@@ -172,9 +171,6 @@ QPDF::readLinearizationData()
     // This function throws an exception (which is trapped by
     // checkLinearization()) for any errors that prevent loading.
-    // Hint table parsing code needs at least 32 bits in a long.
-    assert(sizeof(long) >= 4);
-
     if (!isLinearized()) {
         throw std::logic_error("called readLinearizationData for file"
                                " that is not linearized");
@@ -6,7 +6,7 @@
 #include <qpdf/QPDF_Array.hh>
 #include <qpdf/QPDF_Dictionary.hh>
 #include <qpdf/QTC.hh>
-#include <assert.h>
+#include <cassert>
 QPDF::ObjUser::ObjUser() :
     ou_type(ou_bad),
 #include <qpdf/QPDF.hh>
-#include <assert.h>
-
 #include <qpdf/QPDFExc.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
@@ -233,6 +231,11 @@ QPDF::insertPage(QPDFObjectHandle newpage, int pos)
         QTC::TC("qpdf", "QPDF insert indirect page");
     }
+    if ((pos < 0) || (QIntC::to_size(pos) > this->m->all_pages.size())) {
+        throw std::runtime_error(
+            "QPDF::insertPage called with pos out of range");
+    }
+
     QTC::TC(
         "qpdf",
         "QPDF insert page",
@@ -249,7 +252,6 @@ QPDF::insertPage(QPDFObjectHandle newpage, int pos)
     QPDFObjectHandle pages = getRoot().getKey("/Pages");
     QPDFObjectHandle kids = pages.getKey("/Kids");
-    assert((pos >= 0) && (QIntC::to_size(pos) <= this->m->all_pages.size()));
     newpage.replaceKey("/Parent", pages);
     kids.insertItem(pos, newpage);