Introduce `util::internal_error_if` for concise handling of internal errors

- replace repetitive error handling with utility functions in BufferInputSource, OffsetBuffer, and QPDF_objects. - Remove some "internal error" labels from errors that could be due to user logic errors.

Introduce `util::internal_error_if` for concise handling of internal errors
- replace repetitive error handling with utility functions in BufferInputSource, OffsetBuffer, and QPDF_objects. - Remove some "internal error" labels from errors that could be due to user logic errors.
m-holger
1 parent 7c148c15
Showing 4 changed files with 29 additions and 34 deletions
libqpdf/BitStream.cc
libqpdf/BufferInputSource.cc
libqpdf/QPDF_objects.cc
libqpdf/qpdf/Util.hh
 #include <qpdf/BitStream.hh>
 #include <qpdf/QIntC.hh>
+#include <qpdf/Util.hh>
 // See comments in bits_functions.hh
 #define BITS_READ 1
 #include <qpdf/bits_functions.hh>
+using namespace qpdf;
+
 BitStream::BitStream(unsigned char const* p, size_t nbytes) :
     start(p),
     nbytes(nbytes)
@@ -56,9 +59,8 @@ BitStream::skipToNextByte()
 {
     if (bit_offset != 7) {
         size_t bits_to_skip = bit_offset + 1;
-        if (bits_available < bits_to_skip) {
-            throw std::logic_error("INTERNAL ERROR: overflow skipping to next byte in bitstream");
-        }
+        util::internal_error_if(
+            bits_available < bits_to_skip, "overflow skipping to next byte in bitstream");
         bit_offset = 7;
         ++p;
         bits_available -= bits_to_skip;
@@ -41,9 +41,7 @@ BufferInputSource::~BufferInputSource()
 qpdf_offset_t
 BufferInputSource::findAndSkipNextEOL()
 {
-    if (cur_offset < 0) {
-        throw std::logic_error("INTERNAL ERROR: BufferInputSource offset < 0");
-    }
+    util::internal_error_if(cur_offset < 0, "BufferInputSource offset < 0");
     qpdf_offset_t end_pos = max_offset;
     if (cur_offset >= end_pos) {
         last_offset = end_pos;
@@ -99,14 +97,10 @@ BufferInputSource::seek(qpdf_offset_t offset, int whence)
         cur_offset = max_offset + offset;
         break;
-    case SEEK_CUR:
+    default:
+        util::assertion(whence == SEEK_CUR, "invalid argument to BufferInputSource::seek");
         QIntC::range_check(cur_offset, offset);
         cur_offset += offset;
-        break;
-
-    default:
-        throw std::logic_error("INTERNAL ERROR: invalid argument to BufferInputSource::seek");
-        break;
     }
     if (cur_offset < 0) {
@@ -123,9 +117,7 @@ BufferInputSource::rewind()
 size_t
 BufferInputSource::read(char* buffer, size_t length)
 {
-    if (cur_offset < 0) {
-        throw std::logic_error("INTERNAL ERROR: BufferInputSource offset < 0");
-    }
+    util::internal_error_if(cur_offset < 0, "BufferInputSource offset < 0");
     qpdf_offset_t end_pos = max_offset;
     if (cur_offset >= end_pos) {
         last_offset = end_pos;
@@ -230,9 +222,7 @@ BufferInputSource::unreadCh(char ch)
 qpdf_offset_t
 is::OffsetBuffer::findAndSkipNextEOL()
 {
-    if (pos < 0) {
-        throw std::logic_error("INTERNAL ERROR: is::OffsetBuffer offset < 0");
-    }
+    util::internal_error_if(pos < 0, "is::OffsetBuffer offset < 0");
     auto end_pos = static_cast<qpdf_offset_t>(view_.size());
     if (pos >= end_pos) {
         last_offset = end_pos + global_offset;
@@ -276,14 +266,10 @@ is::OffsetBuffer::seek(qpdf_offset_t offset, int whence)
         pos = static_cast<qpdf_offset_t>(view_.size()) + offset;
         break;
-    case SEEK_CUR:
+    default:
+        util::assertion(whence == SEEK_CUR, "invalid argument to BufferInputSource::seek");
         QIntC::range_check(pos, offset);
         pos += offset;
-        break;
-
-    default:
-        throw std::logic_error("INTERNAL ERROR: invalid argument to BufferInputSource::seek");
-        break;
     }
     if (pos < 0) {
@@ -294,9 +280,7 @@ is::OffsetBuffer::seek(qpdf_offset_t offset, int whence)
 size_t
 is::OffsetBuffer::read(char* buffer, size_t length)
 {
-    if (pos < 0) {
-        throw std::logic_error("INTERNAL ERROR: is::OffsetBuffer offset < 0");
-    }
+    util::internal_error_if(pos < 0, "is::OffsetBuffer offset < 0");
     auto end_pos = static_cast<qpdf_offset_t>(view_.size());
     if (pos >= end_pos) {
         last_offset = end_pos + global_offset;
@@ -238,13 +238,11 @@ Objects::parse(char const* password)
 void
 Objects::inParse(bool v)
 {
-    if (m->in_parse == v) {
+    util::internal_error_if(
+        m->in_parse == v, "QPDF: re-entrant parsing detected"
         // This happens if QPDFParser::parse tries to resolve an indirect object while it is
         // parsing.
-        throw std::logic_error(
-            "QPDF: re-entrant parsing detected. This is a qpdf bug."
-            " Please report at https://github.com/qpdf/qpdf/issues.");
-    }
+    );
     m->in_parse = v;
 }
@@ -535,7 +533,7 @@ Objects::read_xref(qpdf_offset_t xref_offset, bool in_stream_recovery)
         max_obj = std::max(max_obj, *(m->deleted_objects.rbegin()));
     }
     if (size < 1 || (size - 1) != max_obj) {
-        if ((size - 2) == max_obj ){//&& qpdf.getObject(max_obj, 0).isStreamOfType("/XRef")) {
+        if ((size - 2) == max_obj) { //&& qpdf.getObject(max_obj, 0).isStreamOfType("/XRef")) {
             warn(damagedPDF(
                 "",
                 -1,
@@ -7,6 +7,8 @@
 #include <string>
 #include <utility>
+using namespace std::literals;
+
 namespace qpdf::util
 {
     // qpdf::util is a collection of useful utility functions for qpdf internal use. It includes
@@ -17,13 +19,22 @@ namespace qpdf::util
     //
     // DO NOT USE unless it is impractical or unnecessary to cover violations during CI Testing.
     inline void
-    assertion(bool cond, std::string const msg)
+    assertion(bool cond, std::string const& msg)
     {
         if (!cond) {
             throw std::logic_error(msg);
         }
     }
+    inline void
+    internal_error_if(bool cond, std::string const& msg)
+    {
+        if (cond) {
+            throw std::logic_error("INTERNAL ERROR: "s.append(msg).append(
+                "\nThis is a qpdf bug. Please report at https://github.com/qpdf/qpdf/issues"));
+        }
+    }
+
     inline constexpr char
     hex_decode_char(char digit)
     {