diff --git a/TODO b/TODO index 02dacc20..590a39a1 100644 --- a/TODO +++ b/TODO @@ -55,30 +55,10 @@ Output Capture See https://github.com/qpdf/qpdf/issues/691 -QPDFLogger maintains pipelines for info, warn, error, and save. - -There is a singleton, default QPDFLogger which all QPDF and QPDFJob -objects get on construction. In most cases, it is not necessary to -override this. Allow QPDFJob and QPDF to be passed a new logger -instance. QPDFJob should pass its QPDFLogger to all QPDF objects it -creates. The main uses cases for this would be for multithreading or -for having a library that uses QPDF privately and modifies logger -settings so that it wouldn't interfere with a downstream library or -application also using qpdf. - There needs to be a C API to QPDFLogger. Use functions like this: void set_info((*f)(char* data, unsigned int len, void* udata), void* udata); -We should probably deprecate the output/error setters in QPDF and -QPDFJob. In the meantime, document that they won't work anymore to set -different outputs with different QPDF objects. We may need to delete -rather than deprecate these methods. - -Find all places in the library that write to stdout/stderr/cout/cerr. -Also find places that raise exceptions if unable to warn. These should -use the global output writer. - QPDFPagesTree ============= diff --git a/include/qpdf/QPDFLogger.hh b/include/qpdf/QPDFLogger.hh index 340706c9..cd439caa 100644 --- a/include/qpdf/QPDFLogger.hh +++ b/include/qpdf/QPDFLogger.hh @@ -33,6 +33,19 @@ class QPDFLogger QPDF_DLL QPDFLogger(); + // Return the default logger. In general, you should use the + // default logger. You can also create your own loggers and use + // them QPDF and QPDFJob objects, but there are few reasons to do + // so. One reason may if you are using multiple QPDF or QPDFJob + // objects in different threads and want to capture output and + // errors to different streams. (Note that a single QPDF or + // QPDFJob can't be safely used from multiple threads, but it is + // safe to use separate QPDF and QPDFJob objects on separate + // threads.) Another possible reason would be if you are writing + // an application that uses the qpdf library directly and qpdf is + // also used by a downstream library or if you are using qpdf from + // a library and don't want to interfere with potential uses of + // qpdf by other libraries or applications. QPDF_DLL static std::shared_ptr defaultLogger(); diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 557330e5..7ab2652a 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -1411,14 +1411,14 @@ class QPDFObjectHandle // End legacy page helpers // Issue a warning about this object if possible. If the object - // has a description, a warning will be issued. Otherwise, if - // throw_if_no_description is true, throw an exception. Otherwise - // do nothing. Objects read normally from the file have + // has a description, a warning will be issued using the owning + // QPDF as context. Otherwise, a message will be written to the + // default logger's error stream, which is standard error if not + // overridden. Objects read normally from the file have // descriptions. See comments on setObjectDescription for // additional details. QPDF_DLL - void warnIfPossible( - std::string const& warning, bool throw_if_no_description = false); + void warnIfPossible(std::string const& warning); // Initializers for objects. This Factory class gives the QPDF // class specific permission to call factory methods without diff --git a/libqpdf/QPDFArgParser.cc b/libqpdf/QPDFArgParser.cc index 63197684..c58edec7 100644 --- a/libqpdf/QPDFArgParser.cc +++ b/libqpdf/QPDFArgParser.cc @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -235,7 +236,7 @@ QPDFArgParser::argCompletionZsh() void QPDFArgParser::argHelp(std::string const& p) { - std::cout << getHelp(p); + QPDFLogger::defaultLogger()->info(getHelp(p)); exit(0); } diff --git a/libqpdf/QPDFJob_argv.cc b/libqpdf/QPDFJob_argv.cc index a6fb5df7..aa4755e8 100644 --- a/libqpdf/QPDFJob_argv.cc +++ b/libqpdf/QPDFJob_argv.cc @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -104,10 +105,10 @@ void ArgParser::argVersion() { auto whoami = this->ap.getProgname(); - std::cout << whoami << " version " << QPDF::QPDFVersion() << std::endl - << "Run " << whoami - << " --copyright to see copyright and license information." - << std::endl; + *QPDFLogger::defaultLogger()->getInfo() + << whoami << " version " << QPDF::QPDFVersion() << "\n" + << "Run " << whoami + << " --copyright to see copyright and license information.\n"; } void @@ -117,48 +118,35 @@ ArgParser::argCopyright() // Make sure the output looks right on an 80-column display. // 1 2 3 4 5 6 7 8 // 12345678901234567890123456789012345678901234567890123456789012345678901234567890 - std::cout + *QPDFLogger::defaultLogger()->getInfo() << this->ap.getProgname() - << " version " << QPDF::QPDFVersion() << std::endl - << std::endl - << "Copyright (c) 2005-2022 Jay Berkenbilt" - << std::endl - << "QPDF is licensed under the Apache License, Version 2.0 (the \"License\");" - << std::endl - << "you may not use this file except in compliance with the License." - << std::endl - << "You may obtain a copy of the License at" - << std::endl - << std::endl - << " http://www.apache.org/licenses/LICENSE-2.0" - << std::endl - << std::endl - << "Unless required by applicable law or agreed to in writing, software" - << std::endl - << "distributed under the License is distributed on an \"AS IS\" BASIS," - << std::endl - << "WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied." - << std::endl - << "See the License for the specific language governing permissions and" - << std::endl - << "limitations under the License." - << std::endl - << std::endl - << "Versions of qpdf prior to version 7 were released under the terms" - << std::endl - << "of version 2.0 of the Artistic License. At your option, you may" - << std::endl - << "continue to consider qpdf to be licensed under those terms. Please" - << std::endl - << "see the manual for additional information." - << std::endl; + << " version " << QPDF::QPDFVersion() << "\n" + << "\n" + << "Copyright (c) 2005-2022 Jay Berkenbilt\n" + << "QPDF is licensed under the Apache License, Version 2.0 (the \"License\");\n" + << "you may not use this file except in compliance with the License.\n" + << "You may obtain a copy of the License at\n" + << "\n" + << " http://www.apache.org/licenses/LICENSE-2.0\n" + << "\n" + << "Unless required by applicable law or agreed to in writing, software\n" + << "distributed under the License is distributed on an \"AS IS\" BASIS,\n" + << "WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n" + << "See the License for the specific language governing permissions and\n" + << "limitations under the License.\n" + << "\n" + << "Versions of qpdf prior to version 7 were released under the terms\n" + << "of version 2.0 of the Artistic License. At your option, you may\n" + << "continue to consider qpdf to be licensed under those terms. Please\n" + << "see the manual for additional information.\n"; // clang-format on } void ArgParser::argJsonHelp() { - std::cout << QPDFJob::json_out_schema_v1() << std::endl; + *QPDFLogger::defaultLogger()->getInfo() + << QPDFJob::json_out_schema_v1() << "\n"; } void @@ -166,10 +154,10 @@ ArgParser::argShowCrypto() { auto crypto = QPDFCryptoProvider::getRegisteredImpls(); std::string default_crypto = QPDFCryptoProvider::getDefaultProvider(); - std::cout << default_crypto << std::endl; + *QPDFLogger::defaultLogger()->getInfo() << default_crypto << "\n"; for (auto const& iter: crypto) { if (iter != default_crypto) { - std::cout << iter << std::endl; + *QPDFLogger::defaultLogger()->getInfo() << iter << "\n"; } } } @@ -407,7 +395,8 @@ ArgParser::argEndCopyAttachment() void ArgParser::argJobJsonHelp() { - std::cout << QPDFJob::job_json_schema_v1() << std::endl; + *QPDFLogger::defaultLogger()->getInfo() + << QPDFJob::job_json_schema_v1() << "\n"; } void diff --git a/libqpdf/QPDFJob_config.cc b/libqpdf/QPDFJob_config.cc index 5a343f79..b9b53ebd 100644 --- a/libqpdf/QPDFJob_config.cc +++ b/libqpdf/QPDFJob_config.cc @@ -1,5 +1,6 @@ #include +#include #include #include @@ -648,9 +649,10 @@ QPDFJob::Config::passwordFile(std::string const& parameter) o.m->password = QUtil::make_shared_cstr(lines.front()); if (lines.size() > 1) { - std::cerr << this->o.m->message_prefix - << ": WARNING: all but the first line of" - << " the password file are ignored" << std::endl; + *QPDFLogger::defaultLogger()->getError() + << this->o.m->message_prefix + << ": WARNING: all but the first line of" + << " the password file are ignored\n"; } } return this; diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 56a545d6..1578f38f 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -578,13 +579,12 @@ QPDFObjectHandle::getIntValueAsInt() if (v < INT_MIN) { QTC::TC("qpdf", "QPDFObjectHandle int returning INT_MIN"); warnIfPossible( - "requested value of integer is too small; returning INT_MIN", - false); + "requested value of integer is too small; returning INT_MIN"); result = INT_MIN; } else if (v > INT_MAX) { QTC::TC("qpdf", "QPDFObjectHandle int returning INT_MAX"); warnIfPossible( - "requested value of integer is too big; returning INT_MAX", false); + "requested value of integer is too big; returning INT_MAX"); result = INT_MAX; } else { result = static_cast(v); @@ -610,7 +610,7 @@ QPDFObjectHandle::getUIntValue() if (v < 0) { QTC::TC("qpdf", "QPDFObjectHandle uint returning 0"); warnIfPossible( - "unsigned value request for negative number; returning 0", false); + "unsigned value request for negative number; returning 0"); } else { result = static_cast(v); } @@ -635,15 +635,12 @@ QPDFObjectHandle::getUIntValueAsUInt() if (v < 0) { QTC::TC("qpdf", "QPDFObjectHandle uint uint returning 0"); warnIfPossible( - "unsigned integer value request for negative number; returning 0", - false); + "unsigned integer value request for negative number; returning 0"); result = 0; } else if (v > UINT_MAX) { QTC::TC("qpdf", "QPDFObjectHandle uint returning UINT_MAX"); - warnIfPossible( - "requested value of unsigned integer is too big;" - " returning UINT_MAX", - false); + warnIfPossible("requested value of unsigned integer is too big;" + " returning UINT_MAX"); result = UINT_MAX; } else { result = static_cast(v); @@ -3000,16 +2997,15 @@ QPDFObjectHandle::typeWarning( } void -QPDFObjectHandle::warnIfPossible( - std::string const& warning, bool throw_if_no_description) +QPDFObjectHandle::warnIfPossible(std::string const& warning) { QPDF* context = 0; std::string description; dereference(); if (this->obj->getDescription(context, description)) { warn(context, QPDFExc(qpdf_e_damaged_pdf, "", description, 0, warning)); - } else if (throw_if_no_description) { - throw std::runtime_error(warning); + } else { + *QPDFLogger::defaultLogger()->getError() << warning << "\n"; } } diff --git a/libqpdf/qpdf-c.cc b/libqpdf/qpdf-c.cc index 8a940e3c..468811b3 100644 --- a/libqpdf/qpdf-c.cc +++ b/libqpdf/qpdf-c.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -185,8 +186,9 @@ qpdf_cleanup(qpdf_data* qpdf) qpdf_oh_release_all(*qpdf); if ((*qpdf)->error.get()) { QTC::TC("qpdf", "qpdf-c cleanup warned about unhandled error"); - std::cerr << "WARNING: application did not handle error: " - << (*qpdf)->error->what() << std::endl; + *QPDFLogger::defaultLogger()->getWarn() + << "WARNING: application did not handle error: " + << (*qpdf)->error->what() << "\n"; } delete *qpdf; *qpdf = 0; @@ -898,7 +900,8 @@ trap_oh_errors( " to ERROR HANDLING in qpdf-c.h")); qpdf->oh_error_occurred = true; } - std::cerr << qpdf->error->what() << std::endl; + *QPDFLogger::defaultLogger()->getError() + << qpdf->error->what() << "\n"; } return fallback(); } diff --git a/libqpdf/qpdfjob-c.cc b/libqpdf/qpdfjob-c.cc index 0e8ded02..4e6666be 100644 --- a/libqpdf/qpdfjob-c.cc +++ b/libqpdf/qpdfjob-c.cc @@ -1,6 +1,7 @@ #include #include +#include #include #include @@ -19,7 +20,8 @@ qpdfjob_run_from_argv(char const* const argv[]) j.initializeFromArgv(argv); j.run(); } catch (std::exception& e) { - std::cerr << whoami << ": " << e.what() << std::endl; + *QPDFLogger::defaultLogger()->getError() + << whoami << ": " << e.what() << "\n"; return QPDFJob::EXIT_ERROR; } return j.getExitCode(); @@ -48,7 +50,8 @@ qpdfjob_run_from_json(char const* json) j.initializeFromJson(json); j.run(); } catch (std::exception& e) { - std::cerr << "qpdfjob json: " << e.what() << std::endl; + *QPDFLogger::defaultLogger()->getError() + << "qpdfjob json: " << e.what() << "\n"; return QPDFJob::EXIT_ERROR; } return j.getExitCode(); diff --git a/manual/release-notes.rst b/manual/release-notes.rst index 0e48c0f6..d66e4bb3 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -120,6 +120,11 @@ For a detailed list of changes, please see the file - See :ref:`breaking-crypto-api` for specific details, and see :ref:`weak-crypto` for a general discussion. + - QPDFObjectHandle::warnIfPossible no longer takes an optional + argument to throw an exception if there is no description. If + there is no description, it writes to the default logger's error + stream. + - CLI Enhancements - ``qpdf --list-attachments --verbose`` include some additional @@ -139,6 +144,20 @@ For a detailed list of changes, please see the file - Library Enhancements + - A new object ``QPDFLogger`` has been added. Details are in + :file:`include/qpdf/QPDFLogger.hh`. + + - ``QPDF`` and ``QPDFJob`` both use the default logger by + default but can have their loggers overridden. The + ``setOutputStreams`` method is deprecated in both classes. + + - A few things from ``QPDFObjectHandle`` that used to be + exceptions now write errors with the default logger. + + - By configuring the default logger, it is possible to capture + output and errors that slipped through the cracks with + ``setOutputStreams``. + - New methods ``insertItemAndGet``, ``appendItemAndGet``, ``eraseItemAndGet``, ``replaceKeyAndGet``, and ``removeKeyAndGet`` return the newly added or removed object. diff --git a/qpdf/qtest/error-condition.test b/qpdf/qtest/error-condition.test index 1184deb2..d9357a0f 100644 --- a/qpdf/qtest/error-condition.test +++ b/qpdf/qtest/error-condition.test @@ -111,11 +111,11 @@ $td->runtest("C API: no recovery", $td->runtest("integer type checks", {$td->COMMAND => "test_driver 62 minimal.pdf"}, - {$td->STRING => "test 62 done\n", $td->EXIT_STATUS => 0}, + {$td->FILE => "test62.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); $td->runtest("getValueAs... accessor checks", {$td->COMMAND => "test_driver 85 -"}, - {$td->STRING => "test 85 done\n", $td->EXIT_STATUS => 0}, + {$td->FILE => "test85.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); $n_tests += @badfiles + 11; diff --git a/qpdf/qtest/qpdf/test62.out b/qpdf/qtest/qpdf/test62.out new file mode 100644 index 00000000..50bdf993 --- /dev/null +++ b/qpdf/qtest/qpdf/test62.out @@ -0,0 +1,7 @@ +requested value of integer is too big; returning INT_MAX +requested value of unsigned integer is too big; returning UINT_MAX +unsigned value request for negative number; returning 0 +requested value of integer is too small; returning INT_MIN +unsigned integer value request for negative number; returning 0 +requested value of integer is too big; returning INT_MAX +test 62 done diff --git a/qpdf/qtest/qpdf/test85.out b/qpdf/qtest/qpdf/test85.out new file mode 100644 index 00000000..9b94e903 --- /dev/null +++ b/qpdf/qtest/qpdf/test85.out @@ -0,0 +1,6 @@ +requested value of integer is too big; returning INT_MAX +requested value of integer is too small; returning INT_MIN +unsigned value request for negative number; returning 0 +unsigned integer value request for negative number; returning 0 +requested value of unsigned integer is too big; returning UINT_MAX +test 85 done