Merge pull request #902 from m-holger/od

Refactor creation of object descriptions

12 files changed, 264 insertions, 133 deletions

diff --git a/include/qpdf/QPDF.hh b/include/qpdf/QPDF.hh
index d6b32fe9..54af0d17 100644
--- a/include/qpdf/QPDF.hh
+++ b/include/qpdf/QPDF.hh
@@ -1108,71 +1108,7 @@ class QPDF
         std::set<QPDFObjGen>::const_iterator iter;
     };
 
-    class JSONReactor: public JSON::Reactor
-    {
-      public:
-        JSONReactor(
-            QPDF&, std::shared_ptr<InputSource> is, bool must_be_complete);
-        virtual ~JSONReactor() = default;
-        virtual void dictionaryStart() override;
-        virtual void arrayStart() override;
-        virtual void containerEnd(JSON const& value) override;
-        virtual void topLevelScalar() override;
-        virtual bool
-        dictionaryItem(std::string const& key, JSON const& value) override;
-        virtual bool arrayItem(JSON const& value) override;
-
-        bool anyErrors() const;
-
-      private:
-        enum state_e {
-            st_initial,
-            st_top,
-            st_qpdf,
-            st_qpdf_meta,
-            st_objects,
-            st_trailer,
-            st_object_top,
-            st_stream,
-            st_object,
-            st_ignore,
-        };
-
-        void containerStart();
-        void nestedState(std::string const& key, JSON const& value, state_e);
-        void setObjectDescription(QPDFObjectHandle& oh, JSON const& value);
-        QPDFObjectHandle makeObject(JSON const& value);
-        void error(qpdf_offset_t offset, std::string const& message);
-        QPDFObjectHandle reserveObject(int obj, int gen);
-        void replaceObject(
-            QPDFObjectHandle to_replace,
-            QPDFObjectHandle replacement,
-            JSON const& value);
-
-        QPDF& pdf;
-        std::shared_ptr<InputSource> is;
-        bool must_be_complete;
-        bool errors;
-        bool parse_error;
-        bool saw_qpdf;
-        bool saw_qpdf_meta;
-        bool saw_objects;
-        bool saw_json_version;
-        bool saw_pdf_version;
-        bool saw_trailer;
-        state_e state;
-        state_e next_state;
-        std::string cur_object;
-        bool saw_value;
-        bool saw_stream;
-        bool saw_dict;
-        bool saw_data;
-        bool saw_datafile;
-        bool this_stream_needs_data;
-        std::vector<state_e> state_stack;
-        std::vector<QPDFObjectHandle> object_stack;
-        std::set<QPDFObjGen> reserved;
-    };
+    class JSONReactor;
 
     void parse(char const* password);
     void inParse(bool);
diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc
index 2b2ca5db..d474dcce 100644
--- a/libqpdf/QPDFObjectHandle.cc
+++ b/libqpdf/QPDFObjectHandle.cc
@@ -36,6 +36,8 @@
 #include <stdexcept>
 #include <stdlib.h>
 
+using namespace std::literals;
+
 namespace
 {
     class TerminateParsing
@@ -800,12 +802,10 @@ QPDFObjectHandle::getArrayNItems()
 QPDFObjectHandle
 QPDFObjectHandle::getArrayItem(int n)
 {
-    QPDFObjectHandle result;
     auto array = asArray();
     if (array && (n < array->getNItems()) && (n >= 0)) {
-        result = array->getItem(n);
+        return array->getItem(n);
     } else {
-        result = newNull();
         if (array) {
             objectWarning("returning null for out of bounds array access");
             QTC::TC("qpdf", "QPDFObjectHandle array bounds");
@@ -813,15 +813,10 @@ QPDFObjectHandle::getArrayItem(int n)
             typeWarning("array", "returning null");
             QTC::TC("qpdf", "QPDFObjectHandle array null for non-array");
         }
-        QPDF* context = nullptr;
-        std::string description;
-        if (obj->getDescription(context, description)) {
-            result.setObjectDescription(
-                context,
-                description + " -> null returned from invalid array access");
-        }
+        static auto constexpr msg =
+            " -> null returned from invalid array access"sv;
+        return QPDF_Null::create(obj, msg, "");
     }
-    return result;
 }
 
 bool
@@ -1030,24 +1025,15 @@ QPDFObjectHandle::hasKey(std::string const& key)
 QPDFObjectHandle
 QPDFObjectHandle::getKey(std::string const& key)
 {
-    QPDFObjectHandle result;
-    auto dict = asDictionary();
-    if (dict) {
-        result = dict->getKey(key);
+    if (auto dict = asDictionary()) {
+        return dict->getKey(key);
     } else {
         typeWarning("dictionary", "returning null for attempted key retrieval");
         QTC::TC("qpdf", "QPDFObjectHandle dictionary null for getKey");
-        result = newNull();
-        QPDF* qpdf = nullptr;
-        std::string description;
-        if (obj->getDescription(qpdf, description)) {
-            result.setObjectDescription(
-                qpdf,
-                (description + " -> null returned from getting key " + key +
-                 " from non-Dictionary"));
-        }
+        static auto constexpr msg =
+            " -> null returned from getting key $VD from non-Dictionary"sv;
+        return QPDF_Null::create(obj, msg, "");
     }
-    return result;
 }
 
 QPDFObjectHandle
@@ -2176,7 +2162,8 @@ QPDFObjectHandle::setObjectDescription(
     // This is called during parsing on newly created direct objects,
     // so we can't call dereference() here.
     if (isInitialized() && obj.get()) {
-        auto descr = std::make_shared<std::string>(object_description);
+        auto descr =
+            std::make_shared<QPDFValue::Description>(object_description);
         obj->setDescription(owning_qpdf, descr);
     }
 }
diff --git a/libqpdf/QPDFValue.cc b/libqpdf/QPDFValue.cc
index ca3205b7..30d534dc 100644
--- a/libqpdf/QPDFValue.cc
+++ b/libqpdf/QPDFValue.cc
@@ -9,3 +9,58 @@ QPDFValue::do_create(QPDFValue* object)
     obj->value = std::shared_ptr<QPDFValue>(object);
     return obj;
 }
+
+std::string
+QPDFValue::getDescription()
+{
+    if (object_description) {
+        switch (object_description->index()) {
+        case 0:
+            {
+                // Simple template string
+                auto description = std::get<0>(*object_description);
+
+                if (auto pos = description.find("$OG");
+                    pos != std::string::npos) {
+                    description.replace(pos, 3, og.unparse(' '));
+                }
+                if (auto pos = description.find("$PO");
+                    pos != std::string::npos) {
+                    qpdf_offset_t shift = (type_code == ::ot_dictionary) ? 2
+                        : (type_code == ::ot_array)                      ? 1
+                                                                         : 0;
+
+                    description.replace(
+                        pos, 3, std::to_string(parsed_offset + shift));
+                }
+                return description;
+            }
+        case 1:
+            {
+                // QPDF::JSONReactor generated description
+                auto j_descr = std::get<1>(*object_description);
+                return (
+                    *j_descr.input +
+                    (j_descr.object.empty() ? "" : ", " + j_descr.object) +
+                    " at offset " + std::to_string(parsed_offset));
+            }
+        case 2:
+            {
+                // Child object description
+                auto j_descr = std::get<2>(*object_description);
+                std::string result;
+                if (auto p = j_descr.parent.lock()) {
+                    result = p->getDescription();
+                }
+                result += j_descr.static_descr;
+                if (auto pos = result.find("$VD"); pos != std::string::npos) {
+                    result.replace(pos, 3, j_descr.var_descr);
+                }
+                return result;
+            }
+        }
+    } else if (og.isIndirect()) {
+        return "object " + og.unparse(' ');
+    }
+    return {};
+}
diff --git a/libqpdf/QPDF_Dictionary.cc b/libqpdf/QPDF_Dictionary.cc
index 5349a2a8..43ad8a85 100644
--- a/libqpdf/QPDF_Dictionary.cc
+++ b/libqpdf/QPDF_Dictionary.cc
@@ -1,6 +1,10 @@
 #include <qpdf/QPDF_Dictionary.hh>
 
+#include <qpdf/QPDFObject_private.hh>
 #include <qpdf/QPDF_Name.hh>
+#include <qpdf/QPDF_Null.hh>
+
+using namespace std::literals;
 
 QPDF_Dictionary::QPDF_Dictionary(
     std::map<std::string, QPDFObjectHandle> const& items) :
@@ -97,12 +101,8 @@ QPDF_Dictionary::getKey(std::string const& key)
         // May be a null object
         return item->second;
     } else {
-        auto null = QPDFObjectHandle::newNull();
-        if (qpdf != nullptr) {
-            null.setObjectDescription(
-                qpdf, getDescription() + " -> dictionary key " + key);
-        }
-        return null;
+        static auto constexpr msg = " -> dictionary key $VD"sv;
+        return QPDF_Null::create(shared_from_this(), msg, key);
     }
 }
 
diff --git a/libqpdf/QPDF_Null.cc b/libqpdf/QPDF_Null.cc
index 6ec4556c..a82f23c0 100644
--- a/libqpdf/QPDF_Null.cc
+++ b/libqpdf/QPDF_Null.cc
@@ -1,5 +1,7 @@
 #include <qpdf/QPDF_Null.hh>
 
+#include <qpdf/QPDFObject_private.hh>
+
 QPDF_Null::QPDF_Null() :
     QPDFValue(::ot_null, "null")
 {
@@ -12,6 +14,28 @@ QPDF_Null::create()
 }
 
 std::shared_ptr<QPDFObject>
+QPDF_Null::create(
+    std::shared_ptr<QPDFObject> parent,
+    std::string_view const& static_descr,
+    std::string var_descr)
+{
+    auto n = do_create(new QPDF_Null());
+    n->setChildDescription(parent, static_descr, var_descr);
+    return n;
+}
+
+std::shared_ptr<QPDFObject>
+QPDF_Null::create(
+    std::shared_ptr<QPDFValue> parent,
+    std::string_view const& static_descr,
+    std::string var_descr)
+{
+    auto n = do_create(new QPDF_Null());
+    n->setChildDescription(parent, static_descr, var_descr);
+    return n;
+}
+
+std::shared_ptr<QPDFObject>
 QPDF_Null::copy(bool shallow)
 {
     return create();
diff --git a/libqpdf/QPDF_Stream.cc b/libqpdf/QPDF_Stream.cc
index 66916c21..8e2e16c5 100644
--- a/libqpdf/QPDF_Stream.cc
+++ b/libqpdf/QPDF_Stream.cc
@@ -123,7 +123,7 @@ QPDF_Stream::QPDF_Stream(
         throw std::logic_error("stream object instantiated with non-dictionary "
                                "object for dictionary");
     }
-    auto descr = std::make_shared<std::string>(
+    auto descr = std::make_shared<QPDFValue::Description>(
         qpdf->getFilename() + ", stream object " + og.unparse(' '));
     setDescription(qpdf, descr, offset);
 }
@@ -283,7 +283,9 @@ QPDF_Stream::getStreamJSON(
 
 void
 QPDF_Stream::setDescription(
-    QPDF* qpdf, std::shared_ptr<std::string>& description, qpdf_offset_t offset)
+    QPDF* qpdf,
+    std::shared_ptr<QPDFValue::Description>& description,
+    qpdf_offset_t offset)
 {
     this->QPDFValue::setDescription(qpdf, description, offset);
     setDictDescription();
diff --git a/libqpdf/QPDF_json.cc b/libqpdf/QPDF_json.cc
index 02dc57e8..f13b9517 100644
--- a/libqpdf/QPDF_json.cc
+++ b/libqpdf/QPDF_json.cc
@@ -4,6 +4,8 @@
 #include <qpdf/Pl_Base64.hh>
 #include <qpdf/Pl_StdioFile.hh>
 #include <qpdf/QIntC.hh>
+#include <qpdf/QPDFObject_private.hh>
+#include <qpdf/QPDFValue.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
 #include <algorithm>
@@ -221,11 +223,79 @@ provide_data(
     };
 }
 
+class QPDF::JSONReactor: public JSON::Reactor
+{
+  public:
+    JSONReactor(QPDF&, std::shared_ptr<InputSource> is, bool must_be_complete);
+    virtual ~JSONReactor() = default;
+    virtual void dictionaryStart() override;
+    virtual void arrayStart() override;
+    virtual void containerEnd(JSON const& value) override;
+    virtual void topLevelScalar() override;
+    virtual bool
+    dictionaryItem(std::string const& key, JSON const& value) override;
+    virtual bool arrayItem(JSON const& value) override;
+
+    bool anyErrors() const;
+
+  private:
+    enum state_e {
+        st_initial,
+        st_top,
+        st_qpdf,
+        st_qpdf_meta,
+        st_objects,
+        st_trailer,
+        st_object_top,
+        st_stream,
+        st_object,
+        st_ignore,
+    };
+
+    void containerStart();
+    void nestedState(std::string const& key, JSON const& value, state_e);
+    void setObjectDescription(QPDFObjectHandle& oh, JSON const& value);
+    QPDFObjectHandle makeObject(JSON const& value);
+    void error(qpdf_offset_t offset, std::string const& message);
+    QPDFObjectHandle reserveObject(int obj, int gen);
+    void replaceObject(
+        QPDFObjectHandle to_replace,
+        QPDFObjectHandle replacement,
+        JSON const& value);
+
+    QPDF& pdf;
+    std::shared_ptr<InputSource> is;
+    bool must_be_complete;
+    std::shared_ptr<QPDFValue::Description> descr;
+    bool errors;
+    bool parse_error;
+    bool saw_qpdf;
+    bool saw_qpdf_meta;
+    bool saw_objects;
+    bool saw_json_version;
+    bool saw_pdf_version;
+    bool saw_trailer;
+    state_e state;
+    state_e next_state;
+    std::string cur_object;
+    bool saw_value;
+    bool saw_stream;
+    bool saw_dict;
+    bool saw_data;
+    bool saw_datafile;
+    bool this_stream_needs_data;
+    std::vector<state_e> state_stack;
+    std::vector<QPDFObjectHandle> object_stack;
+    std::set<QPDFObjGen> reserved;
+};
+
 QPDF::JSONReactor::JSONReactor(
     QPDF& pdf, std::shared_ptr<InputSource> is, bool must_be_complete) :
     pdf(pdf),
     is(is),
     must_be_complete(must_be_complete),
+    descr(std::make_shared<QPDFValue::Description>(QPDFValue::JSON_Descr(
+        std::make_shared<std::string>(is->getName()), ""))),
     errors(false),
     parse_error(false),
     saw_qpdf(false),
@@ -675,12 +745,13 @@ QPDF::JSONReactor::arrayItem(JSON const& value)
 void
 QPDF::JSONReactor::setObjectDescription(QPDFObjectHandle& oh, JSON const& value)
 {
-    std::string description = this->is->getName();
-    if (!this->cur_object.empty()) {
-        description += ", " + this->cur_object;
+    auto j_descr = std::get<QPDFValue::JSON_Descr>(*descr);
+    if (j_descr.object != cur_object) {
+        descr = std::make_shared<QPDFValue::Description>(
+            QPDFValue::JSON_Descr(j_descr.input, cur_object));
     }
-    description += " at offset " + std::to_string(value.getStart());
-    oh.setObjectDescription(&this->pdf, description);
+
+    oh.getObjectPtr()->setDescription(&pdf, descr, value.getStart());
 }
 
 QPDFObjectHandle
diff --git a/libqpdf/qpdf/QPDFObject_private.hh b/libqpdf/qpdf/QPDFObject_private.hh
index 4d388503..0dc04699 100644
--- a/libqpdf/qpdf/QPDFObject_private.hh
+++ b/libqpdf/qpdf/QPDFObject_private.hh
@@ -12,6 +12,7 @@
 #include <qpdf/Types.h>
 
 #include <string>
+#include <string_view>
 
 class QPDF;
 class QPDFObjectHandle;
@@ -71,11 +72,30 @@ class QPDFObject
     void
     setDescription(
         QPDF* qpdf,
-        std::shared_ptr<std::string>& description,
+        std::shared_ptr<QPDFValue::Description>& description,
         qpdf_offset_t offset = -1)
     {
         return value->setDescription(qpdf, description, offset);
     }
+    void
+    setChildDescription(
+        std::shared_ptr<QPDFObject> parent,
+        std::string_view const& static_descr,
+        std::string var_descr)
+    {
+        auto qpdf = parent ? parent->value->qpdf : nullptr;
+        value->setChildDescription(
+            qpdf, parent->value, static_descr, var_descr);
+    }
+    void
+    setChildDescription(
+        std::shared_ptr<QPDFValue> parent,
+        std::string_view const& static_descr,
+        std::string var_descr)
+    {
+        auto qpdf = parent ? parent->qpdf : nullptr;
+        value->setChildDescription(qpdf, parent, static_descr, var_descr);
+    }
     bool
     getDescription(QPDF*& qpdf, std::string& description)
     {
diff --git a/libqpdf/qpdf/QPDFParser.hh b/libqpdf/qpdf/QPDFParser.hh
index 65279700..c2bf6bbe 100644
--- a/libqpdf/qpdf/QPDFParser.hh
+++ b/libqpdf/qpdf/QPDFParser.hh
@@ -2,6 +2,7 @@
 #define QPDFPARSER_HH
 
 #include <qpdf/QPDFObjectHandle.hh>
+#include <qpdf/QPDFValue.hh>
 
 #include <memory>
 #include <string>
@@ -21,8 +22,8 @@ class QPDFParser
         tokenizer(tokenizer),
         decrypter(decrypter),
         context(context),
-        description(std::make_shared<std::string>(
-            input->getName() + ", " + object_description + " at offset $PO"))
+        description(std::make_shared<QPDFValue::Description>(std::string(
+            input->getName() + ", " + object_description + " at offset $PO")))
     {
     }
     virtual ~QPDFParser() = default;
@@ -49,7 +50,7 @@ class QPDFParser
     QPDFTokenizer& tokenizer;
     QPDFObjectHandle::StringDecrypter* decrypter;
     QPDF* context;
-    std::shared_ptr<std::string> description;
+    std::shared_ptr<QPDFValue::Description> description;
 };
 
 #endif // QPDFPARSER_HH
diff --git a/libqpdf/qpdf/QPDFValue.hh b/libqpdf/qpdf/QPDFValue.hh
index c64f0d6e..e8a1834f 100644
--- a/libqpdf/qpdf/QPDFValue.hh
+++ b/libqpdf/qpdf/QPDFValue.hh
@@ -8,12 +8,14 @@
 #include <qpdf/Types.h>
 
 #include <string>
+#include <string_view>
+#include <variant>
 
 class QPDF;
 class QPDFObjectHandle;
 class QPDFObject;
 
-class QPDFValue
+class QPDFValue: public std::enable_shared_from_this<QPDFValue>
 {
     friend class QPDFObject;
 
@@ -23,10 +25,43 @@ class QPDFValue
     virtual std::shared_ptr<QPDFObject> copy(bool shallow = false) = 0;
     virtual std::string unparse() = 0;
     virtual JSON getJSON(int json_version) = 0;
+
+    struct JSON_Descr
+    {
+        JSON_Descr(
+            std::shared_ptr<std::string> input, std::string const& object) :
+            input(input),
+            object(object)
+        {
+        }
+
+        std::shared_ptr<std::string> input;
+        std::string object;
+    };
+
+    struct ChildDescr
+    {
+        ChildDescr(
+            std::shared_ptr<QPDFValue> parent,
+            std::string_view const& static_descr,
+            std::string var_descr) :
+            parent(parent),
+            static_descr(static_descr),
+            var_descr(var_descr)
+        {
+        }
+
+        std::weak_ptr<QPDFValue> parent;
+        std::string_view const& static_descr;
+        std::string var_descr;
+    };
+
+    using Description = std::variant<std::string, JSON_Descr, ChildDescr>;
+
     virtual void
     setDescription(
         QPDF* qpdf_p,
-        std::shared_ptr<std::string>& description,
+        std::shared_ptr<Description>& description,
         qpdf_offset_t offset)
     {
         qpdf = qpdf_p;
@@ -36,35 +71,25 @@ class QPDFValue
     void
     setDefaultDescription(QPDF* a_qpdf, QPDFObjGen const& a_og)
     {
-        static auto default_description{
-            std::make_shared<std::string>("object $OG")};
-        if (!object_description) {
-            object_description = default_description;
-        }
         qpdf = a_qpdf;
         og = a_og;
     }
-    std::string
-    getDescription()
+    void
+    setChildDescription(
+        QPDF* a_qpdf,
+        std::shared_ptr<QPDFValue> parent,
+        std::string_view const& static_descr,
+        std::string var_descr)
     {
-        auto description = object_description ? *object_description : "";
-        if (auto pos = description.find("$OG"); pos != std::string::npos) {
-            description.replace(pos, 3, og.unparse(' '));
-        }
-        if (auto pos = description.find("$PO"); pos != std::string::npos) {
-            qpdf_offset_t shift = (type_code == ::ot_dictionary) ? 2
-                : (type_code == ::ot_array)                      ? 1
-                                                                 : 0;
-
-            description.replace(pos, 3, std::to_string(parsed_offset + shift));
-        }
-        return description;
+        object_description = std::make_shared<Description>(
+            ChildDescr(parent, static_descr, var_descr));
+        qpdf = a_qpdf;
     }
+    std::string getDescription();
     bool
     hasDescription()
     {
-        return qpdf != nullptr && object_description &&
-            !object_description->empty();
+        return object_description || og.isIndirect();
     }
     void
     setParsedOffset(qpdf_offset_t offset)
@@ -123,7 +148,7 @@ class QPDFValue
   private:
     QPDFValue(QPDFValue const&) = delete;
     QPDFValue& operator=(QPDFValue const&) = delete;
-    std::shared_ptr<std::string> object_description;
+    std::shared_ptr<Description> object_description;
 
     const qpdf_object_type_e type_code{::ot_uninitialized};
     char const* type_name{"uninitialized"};
diff --git a/libqpdf/qpdf/QPDF_Null.hh b/libqpdf/qpdf/QPDF_Null.hh
index 1a92b214..2bbb3db5 100644
--- a/libqpdf/qpdf/QPDF_Null.hh
+++ b/libqpdf/qpdf/QPDF_Null.hh
@@ -8,6 +8,14 @@ class QPDF_Null: public QPDFValue
   public:
     virtual ~QPDF_Null() = default;
     static std::shared_ptr<QPDFObject> create();
+    static std::shared_ptr<QPDFObject> create(
+        std::shared_ptr<QPDFObject> parent,
+        std::string_view const& static_descr,
+        std::string var_descr);
+    static std::shared_ptr<QPDFObject> create(
+        std::shared_ptr<QPDFValue> parent,
+        std::string_view const& static_descr,
+        std::string var_descr);
     virtual std::shared_ptr<QPDFObject> copy(bool shallow = false);
     virtual std::string unparse();
     virtual JSON getJSON(int json_version);
diff --git a/libqpdf/qpdf/QPDF_Stream.hh b/libqpdf/qpdf/QPDF_Stream.hh
index c0694c60..cf0a2288 100644
--- a/libqpdf/qpdf/QPDF_Stream.hh
+++ b/libqpdf/qpdf/QPDF_Stream.hh
@@ -27,7 +27,9 @@ class QPDF_Stream: public QPDFValue
     virtual std::string unparse();
     virtual JSON getJSON(int json_version);
     virtual void setDescription(
-        QPDF*, std::shared_ptr<std::string>& description, qpdf_offset_t offset);
+        QPDF*,
+        std::shared_ptr<QPDFValue::Description>& description,
+        qpdf_offset_t offset);
     virtual void disconnect();
     QPDFObjectHandle getDict() const;
     bool isDataModified() const;