Merge pull request #1578 from m-holger/members

Refactor QPDFFormFieldObjectHelper and QPDFEFStreamObjectHelper

Merge pull request #1578 from m-holger/members
Refactor QPDFFormFieldObjectHelper and QPDFEFStreamObjectHelper
m-holger · GitHub
2 parents 628cf873 9cbc87e1
Showing 10 changed files with 142 additions and 177 deletions
include/qpdf/QPDFEFStreamObjectHelper.hh
include/qpdf/QPDFFileSpecObjectHelper.hh
libqpdf/CMakeLists.txt
libqpdf/MD5.cc
libqpdf/Pl_MD5.cc
libqpdf/QPDFEFStreamObjectHelper.cc
libqpdf/QPDFFileSpecObjectHelper.cc
libqpdf/qpdf/Pl_MD5.hh
qpdf/qpdf.testcov
qpdf/test_driver.cc
@@ -92,17 +92,7 @@ class QPDFEFStreamObjectHelper: public QPDFObjectHelper
     void setParam(std::string const& pkey, QPDFObjectHandle const&);
     static QPDFEFStreamObjectHelper newFromStream(QPDFObjectHandle stream);
-    class Members
-    {
-        friend class QPDFEFStreamObjectHelper;
-
-      public:
-        ~Members() = default;
-
-      private:
-        Members() = default;
-        Members(Members const&) = delete;
-    };
+    class Members;
     std::shared_ptr<Members> m;
 };
@@ -87,18 +87,7 @@ class QPDFFileSpecObjectHelper: public QPDFObjectHelper
     setFilename(std::string const& unicode_name, std::string const& compat_name = "");
   private:
-    class Members
-    {
-        friend class QPDFFileSpecObjectHelper;
-
-      public:
-        ~Members() = default;
-
-      private:
-        Members() = default;
-        Members(Members const&) = delete;
-    };
-
+    class Members;
     std::shared_ptr<Members> m;
 };
@@ -44,7 +44,6 @@ set(libqpdf_SOURCES
   Pl_Flate.cc
   Pl_Function.cc
   Pl_LZWDecoder.cc
-  Pl_MD5.cc
   Pl_OStream.cc
   Pl_PNGFilter.cc
   Pl_QPDFTokenizer.cc
 #include <qpdf/MD5.hh>
+#include <qpdf/Pl_MD5.hh>
+
 #include <qpdf/QIntC.hh>
 #include <qpdf/QPDFCryptoProvider.hh>
 #include <qpdf/QUtil.hh>
@@ -163,3 +165,29 @@ MD5::checkFileChecksum(char const* const checksum, char const* filename, qpdf_of
     }
     return result;
 }
+
+void
+Pl_MD5::write(unsigned char const* buf, size_t len)
+{
+    if (enabled) {
+        if (!in_progress) {
+            md5.reset();
+            in_progress = true;
+        }
+
+        // Write in chunks in case len is too big to fit in an int. Assume int is at least 32 bits.
+        static size_t const max_bytes = 1 << 30;
+        size_t bytes_left = len;
+        unsigned char const* data = buf;
+        while (bytes_left > 0) {
+            size_t bytes = (bytes_left >= max_bytes ? max_bytes : bytes_left);
+            md5.encodeDataIncrementally(reinterpret_cast<char const*>(data), bytes);
+            bytes_left -= bytes;
+            data += bytes;
+        }
+    }
+
+    if (next()) {
+        next()->write(buf, len);
+    }
+}
-#include <qpdf/Pl_MD5.hh>
-
-#include <stdexcept>
-
-Pl_MD5::Pl_MD5(char const* identifier, Pipeline* next) :
-    Pipeline(identifier, next)
-{
-    if (!next) {
-        throw std::logic_error("Attempt to create Pl_MD5 with nullptr as next");
-    }
-}
-
-void
-Pl_MD5::write(unsigned char const* buf, size_t len)
-{
-    if (enabled) {
-        if (!in_progress) {
-            md5.reset();
-            in_progress = true;
-        }
-
-        // Write in chunks in case len is too big to fit in an int. Assume int is at least 32 bits.
-        static size_t const max_bytes = 1 << 30;
-        size_t bytes_left = len;
-        unsigned char const* data = buf;
-        while (bytes_left > 0) {
-            size_t bytes = (bytes_left >= max_bytes ? max_bytes : bytes_left);
-            md5.encodeDataIncrementally(reinterpret_cast<char const*>(data), bytes);
-            bytes_left -= bytes;
-            data += bytes;
-        }
-    }
-
-    next()->write(buf, len);
-}
-
-void
-Pl_MD5::finish()
-{
-    next()->finish();
-    if (!persist_across_finish) {
-        in_progress = false;
-    }
-}
-
-void
-Pl_MD5::enable(bool is_enabled)
-{
-    enabled = is_enabled;
-}
-
-void
-Pl_MD5::persistAcrossFinish(bool persist)
-{
-    persist_across_finish = persist;
-}
-
-std::string
-Pl_MD5::getHexDigest()
-{
-    if (!enabled) {
-        throw std::logic_error("digest requested for a disabled MD5 Pipeline");
-    }
-    in_progress = false;
-    return md5.unparse();
-}
 #include <qpdf/QPDFEFStreamObjectHelper.hh>
-#include <qpdf/Pl_Count.hh>
-#include <qpdf/Pl_Discard.hh>
+#include <qpdf/Pipeline_private.hh>
 #include <qpdf/Pl_MD5.hh>
 #include <qpdf/QIntC.hh>
 #include <qpdf/QPDF.hh>
+#include <qpdf/QPDFObjectHandle_private.hh>
 #include <qpdf/QUtil.hh>
+using namespace qpdf;
+
+class QPDFEFStreamObjectHelper::Members
+{
+};
+
 QPDFEFStreamObjectHelper::QPDFEFStreamObjectHelper(QPDFObjectHandle oh) :
-    QPDFObjectHelper(oh),
-    m(new Members())
+    QPDFObjectHelper(oh)
 {
 }
 QPDFObjectHandle
 QPDFEFStreamObjectHelper::getParam(std::string const& pkey)
 {
-    auto params = oh().getDict().getKey("/Params");
-    if (params.isDictionary()) {
-        return params.getKey(pkey);
+    if (auto result = oh().getDict()["/Params"][pkey]) {
+        return result;
     }
-    return QPDFObjectHandle::newNull();
+    return {};
 }
 void
 QPDFEFStreamObjectHelper::setParam(std::string const& pkey, QPDFObjectHandle const& pval)
 {
-    auto params = oh().getDict().getKey("/Params");
-    if (!params.isDictionary()) {
-        params = oh().getDict().replaceKeyAndGetNew("/Params", QPDFObjectHandle::newDictionary());
+    if (Dictionary Params = oh().getDict()["/Params"]) {
+        Params.replaceKey(pkey, pval);
+        return;
     }
-    params.replaceKey(pkey, pval);
+    oh().getDict().replaceKey("/Params", Dictionary({{pkey, pval}}));
 }
 std::string
 QPDFEFStreamObjectHelper::getCreationDate()
 {
-    auto val = getParam("/CreationDate");
-    if (val.isString()) {
-        return val.getUTF8Value();
+    if (String CreationDate = getParam("/CreationDate")) {
+        return CreationDate.utf8_value();
     }
-    return "";
+    return {};
 }
 std::string
 QPDFEFStreamObjectHelper::getModDate()
 {
-    auto val = getParam("/ModDate");
-    if (val.isString()) {
-        return val.getUTF8Value();
+    if (String ModDate = getParam("/ModDate")) {
+        return ModDate.utf8_value();
     }
-    return "";
+    return {};
 }
 size_t
 QPDFEFStreamObjectHelper::getSize()
 {
-    auto val = getParam("/Size");
-    if (val.isInteger()) {
-        return QIntC::to_size(val.getUIntValueAsUInt());
+    if (Integer Size = getParam("/Size")) {
+        size_t result = Size;
+        return result;
     }
     return 0;
 }
@@ -66,30 +68,27 @@ QPDFEFStreamObjectHelper::getSize()
 std::string
 QPDFEFStreamObjectHelper::getSubtype()
 {
-    auto val = oh().getDict().getKey("/Subtype");
-    if (val.isName()) {
-        auto n = val.getName();
-        if (n.length() > 1) {
-            return n.substr(1);
+    if (Name Subtype = oh().getDict()["/Subtype"]) {
+        if (Subtype.value().size() > 1) {
+            return Subtype.value().substr(1);
         }
     }
-    return "";
+    return {};
 }
 std::string
 QPDFEFStreamObjectHelper::getChecksum()
 {
-    auto val = getParam("/CheckSum");
-    if (val.isString()) {
-        return val.getStringValue();
+    if (String CheckSum = getParam("/CheckSum")) {
+        return CheckSum.value();
     }
-    return "";
+    return {};
 }
 QPDFEFStreamObjectHelper
 QPDFEFStreamObjectHelper::createEFStream(QPDF& qpdf, std::shared_ptr<Buffer> data)
 {
-    return newFromStream(qpdf.newStream(data));
+    return newFromStream(qpdf.newStream(std::move(data)));
 }
 QPDFEFStreamObjectHelper
@@ -102,28 +101,28 @@ QPDFEFStreamObjectHelper
 QPDFEFStreamObjectHelper::createEFStream(QPDF& qpdf, std::function<void(Pipeline*)> provider)
 {
     auto stream = qpdf.newStream();
-    stream.replaceStreamData(provider, QPDFObjectHandle::newNull(), QPDFObjectHandle::newNull());
+    stream.replaceStreamData(provider, {}, {});
     return newFromStream(stream);
 }
 QPDFEFStreamObjectHelper&
 QPDFEFStreamObjectHelper::setCreationDate(std::string const& date)
 {
-    setParam("/CreationDate", QPDFObjectHandle::newString(date));
+    setParam("/CreationDate", String(date));
     return *this;
 }
 QPDFEFStreamObjectHelper&
 QPDFEFStreamObjectHelper::setModDate(std::string const& date)
 {
-    setParam("/ModDate", QPDFObjectHandle::newString(date));
+    setParam("/ModDate", String(date));
     return *this;
 }
 QPDFEFStreamObjectHelper&
 QPDFEFStreamObjectHelper::setSubtype(std::string const& subtype)
 {
-    oh().getDict().replaceKey("/Subtype", QPDFObjectHandle::newName("/" + subtype));
+    oh().getDict().replaceKey("/Subtype", Name("/" + subtype));
     return *this;
 }
@@ -131,18 +130,16 @@ QPDFEFStreamObjectHelper
 QPDFEFStreamObjectHelper::newFromStream(QPDFObjectHandle stream)
 {
     QPDFEFStreamObjectHelper result(stream);
-    stream.getDict().replaceKey("/Type", QPDFObjectHandle::newName("/EmbeddedFile"));
-    Pl_Discard discard;
+    stream.getDict().replaceKey("/Type", Name("/EmbeddedFile"));
     // The PDF spec specifies use of MD5 here and notes that it is not to be used for security. MD5
     // is known to be insecure.
-    Pl_MD5 md5("EF md5", &discard);
-    Pl_Count count("EF size", &md5);
+    Pl_MD5 md5("EF md5");
+    pl::Count count(0, &md5);
     if (!stream.pipeStreamData(&count, nullptr, 0, qpdf_dl_all)) {
         stream.warn("unable to get stream data for new embedded file stream");
     } else {
-        result.setParam("/Size", QPDFObjectHandle::newInteger(count.getCount()));
-        result.setParam(
-            "/CheckSum", QPDFObjectHandle::newString(QUtil::hex_decode(md5.getHexDigest())));
+        result.setParam("/Size", Integer(count.getCount()));
+        result.setParam("/CheckSum", String(QUtil::hex_decode(md5.getHexDigest())));
     }
     return result;
 }
 #include <qpdf/QPDFFileSpecObjectHelper.hh>
 #include <qpdf/QPDF.hh>
+#include <qpdf/QPDFObjectHandle_private.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
@@ -8,6 +9,11 @@
 #include <string>
 using namespace std::literals;
+using namespace qpdf;
+
+class QPDFFileSpecObjectHelper::Members
+{
+};
 QPDFFileSpecObjectHelper::QPDFFileSpecObjectHelper(QPDFObjectHandle oh) :
     QPDFObjectHelper(oh)
@@ -26,9 +32,8 @@ static const std::array&lt;std::string, 5&gt; name_keys = {&quot;/UF&quot;s, &quot;/F&quot;s, &quot;/Unix&quot;s, &quot;/
 std::string
 QPDFFileSpecObjectHelper::getDescription()
 {
-    auto desc = oh().getKey("/Desc");
-    if (desc.isString()) {
-        return desc.getUTF8Value();
+    if (String Desc = oh().getKey("/Desc")) {
+        return Desc.utf8_value();
     }
     return {};
 }
@@ -37,12 +42,11 @@ std::string
 QPDFFileSpecObjectHelper::getFilename()
 {
     for (auto const& i: name_keys) {
-        auto k = oh().getKey(i);
-        if (k.isString()) {
-            return k.getUTF8Value();
+        if (String k = oh()[i]) {
+            return k.utf8_value();
         }
     }
-    return "";
+    return {};
 }
 std::map<std::string, std::string>
@@ -50,9 +54,8 @@ QPDFFileSpecObjectHelper::getFilenames()
 {
     std::map<std::string, std::string> result;
     for (auto const& i: name_keys) {
-        auto k = oh().getKey(i);
-        if (k.isString()) {
-            result[i] = k.getUTF8Value();
+        if (String k = oh()[i]) {
+            result[i] = k.utf8_value();
         }
     }
     return result;
@@ -61,17 +64,16 @@ QPDFFileSpecObjectHelper::getFilenames()
 QPDFObjectHandle
 QPDFFileSpecObjectHelper::getEmbeddedFileStream(std::string const& key)
 {
-    auto ef = oh().getKey("/EF");
-    if (!ef.isDictionary()) {
-        return QPDFObjectHandle::newNull();
-    }
-    if (!key.empty()) {
-        return ef.getKey(key);
-    }
-    for (auto const& i: name_keys) {
-        auto k = ef.getKey(i);
-        if (k.isStream()) {
-            return k;
+    if (Dictionary EF = oh()["/EF"]) {
+        if (!key.empty() && EF.contains(key)) {
+            if (auto result = EF[key]) {
+                return result;
+            }
+        }
+        for (auto const& i: name_keys) {
+            if (Stream k = EF[i]) {
+                return k;
+            }
         }
     }
     return QPDFObjectHandle::newNull();
@@ -97,21 +99,18 @@ QPDFFileSpecObjectHelper
 QPDFFileSpecObjectHelper::createFileSpec(
     QPDF& qpdf, std::string const& filename, QPDFEFStreamObjectHelper efsoh)
 {
-    auto oh = qpdf.makeIndirectObject(QPDFObjectHandle::newDictionary());
-    oh.replaceKey("/Type", QPDFObjectHandle::newName("/Filespec"));
-    QPDFFileSpecObjectHelper result(oh);
-    result.setFilename(filename);
-    auto ef = QPDFObjectHandle::newDictionary();
-    ef.replaceKey("/F", efsoh.getObjectHandle());
-    ef.replaceKey("/UF", efsoh.getObjectHandle());
-    oh.replaceKey("/EF", ef);
-    return result;
+    auto UF = String::utf16(filename);
+    return {qpdf.makeIndirectObject(Dictionary(
+        {{"/Type", Name("/Filespec")},
+         {"/F", UF},
+         {"/UF", UF},
+         {"/EF", Dictionary({{"/F", efsoh}, {"/UF", efsoh}})}}))};
 }
 QPDFFileSpecObjectHelper&
 QPDFFileSpecObjectHelper::setDescription(std::string const& desc)
 {
-    oh().replaceKey("/Desc", QPDFObjectHandle::newUnicodeString(desc));
+    oh().replaceKey("/Desc", String::utf16(desc));
     return *this;
 }
@@ -119,14 +118,12 @@ QPDFFileSpecObjectHelper&amp;
 QPDFFileSpecObjectHelper::setFilename(
     std::string const& unicode_name, std::string const& compat_name)
 {
-    auto uf = QPDFObjectHandle::newUnicodeString(unicode_name);
+    auto uf = String::utf16(unicode_name);
     oh().replaceKey("/UF", uf);
     if (compat_name.empty()) {
-        QTC::TC("qpdf", "QPDFFileSpecObjectHelper empty compat_name");
         oh().replaceKey("/F", uf);
     } else {
-        QTC::TC("qpdf", "QPDFFileSpecObjectHelper non-empty compat_name");
-        oh().replaceKey("/F", QPDFObjectHandle::newString(compat_name));
+        oh().replaceKey("/F", String(compat_name));
     }
     return *this;
 }
@@ -4,6 +4,8 @@
 #include <qpdf/MD5.hh>
 #include <qpdf/Pipeline.hh>
+#include <stdexcept>
+
 // This pipeline sends its output to its successor unmodified.  After calling finish, the MD5
 // checksum of the data that passed through the pipeline is available.
@@ -12,18 +14,47 @@
 class Pl_MD5 final: public Pipeline
 {
   public:
-    Pl_MD5(char const* identifier, Pipeline* next);
+    Pl_MD5(char const* identifier, Pipeline* next = nullptr) :
+        Pipeline(identifier, next)
+    {
+    }
+
     ~Pl_MD5() final = default;
     void write(unsigned char const*, size_t) final;
-    void finish() final;
-    std::string getHexDigest();
+    void
+    finish() final
+    {
+        if (next()) {
+            next()->finish();
+        }
+        if (!persist_across_finish) {
+            in_progress = false;
+        }
+    }
+    std::string
+    getHexDigest()
+    {
+        if (!enabled) {
+            throw std::logic_error("digest requested for a disabled MD5 Pipeline");
+        }
+        in_progress = false;
+        return md5.unparse();
+    }
     // Enable/disable. Disabling the pipeline causes it to become a pass-through. This makes it
     // possible to stick an MD5 pipeline in a pipeline when it may or may not be required. Disabling
     // it avoids incurring the runtime overhead of doing needless digest computation.
-    void enable(bool enabled);
+    void
+    enable(bool val)
+    {
+        enabled = val;
+    }
     // If persistAcrossFinish is called, calls to finish do not finalize the underlying md5 object.
     // In this case, the object is not finalized until getHexDigest() is called.
-    void persistAcrossFinish(bool);
+    void
+    persistAcrossFinish(bool val)
+    {
+        persist_across_finish = val;
+    }
   private:
     bool in_progress{false};
@@ -325,8 +325,6 @@ QPDFPageObjectHelper::forEachXObject 3
 NNTree erased last kid/item in tree 1
 QPDFPageObjectHelper unresolved names 0
 QPDFPageObjectHelper resolving unresolved 0
-QPDFFileSpecObjectHelper empty compat_name 0
-QPDFFileSpecObjectHelper non-empty compat_name 0
 QPDFAcroFormDocumentHelper copy annotation 3
 QPDFAcroFormDocumentHelper field with parent 3
 QPDFObjectHandle merge reuse 0
@@ -2644,6 +2644,8 @@ test_76(QPDF&amp; pdf, char const* arg2)
         "att2", QPDFFileSpecObjectHelper::createFileSpec(pdf, "att2.txt", efs2));
     auto fs3 = QPDFFileSpecObjectHelper::createFileSpec(pdf, "att3.txt", efs3);
     efdh.replaceEmbeddedFile("att3", fs3);
+    fs3.setFilename("\xcf\x80.txt");
+    assert(fs3.getFilename() == "\xcf\x80.txt");
     fs3.setFilename("\xcf\x80.txt", "att3.txt");
     assert(efs1.getCreationDate() == "D:20210207191121-05'00'");