Replace some assertions with std::logic_error

Ideally, the library should never call assert outside of test code,
but it does in several places.  For some cases where the assertion
might conceivably fail because of a problem with the input data,
replace assertions with exceptions so that they can be trapped by the
calling application.  This commit surely misses some cases and
replaced some cases unnecessarily, but it should still be an
improvement.

5 files changed, 82 insertions, 21 deletions

diff --git a/ChangeLog b/ChangeLog
index f87c4418..fd15a753 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2013-10-05  Jay Berkenbilt  <ejb@ql.org>
 
+	* Replace some assert() calls with std::logic_error exceptions.
+	Ideally there shouldn't be assert() calls outside of testing.
+	This change may make a few more potential code errors in handling
+	invalid data recoverable.
+
 	* Security fix: In places where std::vector<T>(size_t) was used,
 	either validate that the size parameter is sane or refactor code
 	to avoid the need to pre-allocate the vector.  This reduces the
diff --git a/libqpdf/Pl_LZWDecoder.cc b/libqpdf/Pl_LZWDecoder.cc
index f7c6fb30..82a668e3 100644
--- a/libqpdf/Pl_LZWDecoder.cc
+++ b/libqpdf/Pl_LZWDecoder.cc
@@ -1,6 +1,7 @@
 #include <qpdf/Pl_LZWDecoder.hh>
 
 #include <qpdf/QTC.hh>
+#include <qpdf/QUtil.hh>
 #include <stdexcept>
 #include <string.h>
 #include <assert.h>
@@ -100,14 +101,23 @@ Pl_LZWDecoder::getFirstChar(int code)
     {
 	result = static_cast<unsigned char>(code);
     }
-    else
+    else if (code > 257)
     {
-	assert(code > 257);
 	unsigned int idx = code - 258;
-	assert(idx < table.size());
+	if (idx >= table.size())
+        {
+            throw std::logic_error(
+                "Pl_LZWDecoder::getFirstChar: table overflow");
+        }
 	Buffer& b = table[idx];
 	result = b.getBuffer()[0];
     }
+    else
+    {
+        throw std::logic_error(
+            "Pl_LZWDecoder::getFirstChar called with invalid code (" +
+            QUtil::int_to_string(code) + ")");
+    }
     return result;
 }
 
@@ -124,15 +134,24 @@ Pl_LZWDecoder::addToTable(unsigned char next)
 	last_data = tmp;
 	last_size = 1;
     }
-    else
+    else if (this->last_code > 257)
     {
-	assert(this->last_code > 257);
 	unsigned int idx = this->last_code - 258;
-	assert(idx < table.size());
+	if (idx >= table.size())
+        {
+            throw std::logic_error(
+                "Pl_LZWDecoder::addToTable: table overflow");
+        }
 	Buffer& b = table[idx];
 	last_data = b.getBuffer();
 	last_size = b.getSize();
     }
+    else
+    {
+        throw std::logic_error(
+            "Pl_LZWDecoder::addToTable called with invalid code (" +
+            QUtil::int_to_string(this->last_code) + ")");
+    }
 
     Buffer entry(1 + last_size);
     unsigned char* new_data = entry.getBuffer();
diff --git a/libqpdf/QPDFWriter.cc b/libqpdf/QPDFWriter.cc
index 863e753b..949652c5 100644
--- a/libqpdf/QPDFWriter.cc
+++ b/libqpdf/QPDFWriter.cc
@@ -772,7 +772,11 @@ QPDFWriter::bytesNeeded(unsigned long long n)
 void
 QPDFWriter::writeBinary(unsigned long long val, unsigned int bytes)
 {
-    assert(bytes <= sizeof(unsigned long long));
+    if (bytes > sizeof(unsigned long long))
+    {
+        throw std::logic_error(
+            "QPDFWriter::writeBinary called with too many bytes");
+    }
     unsigned char data[sizeof(unsigned long long)];
     for (unsigned int i = 0; i < bytes; ++i)
     {
@@ -1097,7 +1101,11 @@ QPDFWriter::writeTrailer(trailer_e which, int size, bool xref_stream,
 		    qpdf_offset_t pos = this->pipeline->getCount();
 		    writeString(QUtil::int_to_string(prev));
 		    int nspaces = pos - this->pipeline->getCount() + 21;
-		    assert(nspaces >= 0);
+		    if (nspaces < 0)
+                    {
+                        throw std::logic_error(
+                            "QPDFWriter: no padding required in trailer");
+                    }
 		    writePad(nspaces);
 		}
 	    }
@@ -2814,9 +2822,11 @@ QPDFWriter::writeLinearized()
 		// place as in pass 1.
 		writePad(first_xref_end - endpos);
 
-		// A failure of this insertion means we didn't allow
-		// enough padding for the first pass xref stream.
-		assert(this->pipeline->getCount() == first_xref_end);
+		if (this->pipeline->getCount() != first_xref_end)
+                {
+                    throw std::logic_error(
+                        "insufficient padding for first pass xref stream");
+                }
 	    }
 	    writeString("\n");
 	}
@@ -2897,8 +2907,13 @@ QPDFWriter::writeLinearized()
 
 		// If this assertion fails, maybe we didn't have
 		// enough padding above.
-		assert(this->pipeline->getCount() ==
-                       second_xref_end + hint_length);
+		if (this->pipeline->getCount() !=
+                    second_xref_end + hint_length)
+                {
+                    throw std::logic_error(
+                        "count mismatch after xref stream;"
+                        " possible insufficient padding?");
+                }
 	    }
 	}
 	else
diff --git a/libqpdf/QPDF_linearization.cc b/libqpdf/QPDF_linearization.cc
index 7d2560d3..4b28e480 100644
--- a/libqpdf/QPDF_linearization.cc
+++ b/libqpdf/QPDF_linearization.cc
@@ -606,15 +606,21 @@ QPDF::checkLinearizationInternal()
     // agree with pdlin.  As of this writing, the test suite doesn't
     // contain any files with threads.
 
-    assert(! this->part6.empty());
+    if (this->part6.empty())
+    {
+        throw std::logic_error("linearization part 6 unexpectedly empty");
+    }
     qpdf_offset_t min_E = -1;
     qpdf_offset_t max_E = -1;
     for (std::vector<QPDFObjectHandle>::iterator iter = this->part6.begin();
 	 iter != this->part6.end(); ++iter)
     {
 	QPDFObjGen og((*iter).getObjGen());
-	// All objects have to have been dereferenced to be classified.
-	assert(this->obj_cache.count(og) > 0);
+	if (this->obj_cache.count(og) == 0)
+        {
+            // All objects have to have been dereferenced to be classified.
+            throw std::logic_error("linearization part6 object not in cache");
+        }
 	ObjCache const& oc = this->obj_cache[og];
 	min_E = std::max(min_E, oc.end_before_space);
 	max_E = std::max(max_E, oc.end_after_space);
@@ -832,14 +838,23 @@ QPDF::checkHPageOffset(std::list<std::string>& errors,
 	for (int i = 0; i < he.nshared_objects; ++i)
 	{
 	    int idx = he.shared_identifiers[i];
-	    assert(shared_idx_to_obj.count(idx) > 0);
+	    if (shared_idx_to_obj.count(idx) == 0)
+            {
+                throw std::logic_error(
+                    "unable to get object for item in"
+                    " shared objects hint table");
+            }
 	    hint_shared.insert(shared_idx_to_obj[idx]);
 	}
 
 	for (int i = 0; i < ce.nshared_objects; ++i)
 	{
 	    int idx = ce.shared_identifiers[i];
-	    assert(idx < this->c_shared_object_data.nshared_total);
+	    if (idx >= this->c_shared_object_data.nshared_total)
+            {
+                throw std::logic_error(
+                    "index out of bounds for shared object hint table");
+            }
 	    int obj = this->c_shared_object_data.entries[idx].object;
 	    computed_shared.insert(obj);
 	}
@@ -1754,8 +1769,12 @@ QPDF::calculateLinearizationData(std::map<int, int> const& object_stream_data)
 	    shared.push_back(CHSharedObjectEntry(obj));
 	}
     }
-    assert(static_cast<size_t>(this->c_shared_object_data.nshared_total) ==
-	   this->c_shared_object_data.entries.size());
+    if (static_cast<size_t>(this->c_shared_object_data.nshared_total) !=
+        this->c_shared_object_data.entries.size())
+    {
+        throw std::logic_error(
+            "shared object hint table has wrong number of entries");
+    }
 
     // Now compute the list of shared objects for each page after the
     // first page.
diff --git a/libqpdf/QPDF_pages.cc b/libqpdf/QPDF_pages.cc
index 0604cd40..b88ed5fa 100644
--- a/libqpdf/QPDF_pages.cc
+++ b/libqpdf/QPDF_pages.cc
@@ -120,7 +120,10 @@ QPDF::flattenPagesTree()
 
     pages.replaceKey("/Kids", QPDFObjectHandle::newArray(this->all_pages));
     // /Count has not changed
-    assert(pages.getKey("/Count").getIntValue() == len);
+    if (pages.getKey("/Count").getIntValue() != len)
+    {
+        throw std::logic_error("/Count is wrong after flattening pages tree");
+    }
 }
 
 void