tinyformat: don't throw run-time errors for string literals

Format string literals are partially validated at compile-time through
util::ConstevalFormatString. In almost all cases, invalid format
strings should not lead to crashing behaviour, so suppress any remaining
run-time format errors by returning the error as a string instead of
throwing a tinyformat::format_error.

Callsites that require throwing behaviour can still use tfm::format_raw.

src/logging.h

@@ -249,13 +249,7 @@ template <typename... Args>
 inline void LogPrintFormatInternal(std::string_view logging_function, std::string_view source_file, const int source_line, const BCLog::LogFlags flag, const BCLog::Level level, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
 {
     if (LogInstance().Enabled()) {
-        std::string log_msg;
-        try {
-            log_msg = tfm::format(fmt, args...);
-        } catch (tinyformat::format_error& fmterr) {
-            /* Original format string will have newline so don't add one here */
-            log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;
-        }
+        const auto log_msg{tfm::format(fmt, args...)};
         LogInstance().LogPrintStr(log_msg, logging_function, source_file, source_line, flag, level);
     }
 }

src/test/util_string_tests.cpp

@@ -2,10 +2,13 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <tinyformat.h>
 #include <util/string.h>
 
 #include <boost/test/unit_test.hpp>
 
+#include <sstream>
+
 using namespace util;
 
 BOOST_AUTO_TEST_SUITE(util_string_tests)
@@ -83,4 +86,15 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
     FailFmtWithError<1>("%1$", err_term);
 }
 
+BOOST_AUTO_TEST_CASE(tinyformat_ConstevalFormatString)
+{
+    // ensure invalid format strings don't throw at run-time
+    BOOST_CHECK_EQUAL(tfm::format("%*c", "dummy"), R"(Error "tinyformat: Cannot convert from argument type to integer for use as variable width or precision" while formatting log message: "%*c")");
+    BOOST_CHECK_EQUAL(tfm::format("%2$*3$d", "dummy", "value"), R"(Error "tinyformat: Positional argument out of range" while formatting log message: "%2$*3$d")");
+    std::ostringstream oss;
+    tfm::format(oss, "%.*f", 5);
+    BOOST_CHECK_EQUAL(oss.str(), R"(Error "tinyformat: Too many conversion specifiers in format string" while formatting log message: "%.*f")");
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

src/tinyformat.h

@@ -1147,15 +1147,32 @@ TINYFORMAT_FOREACH_ARGNUM(TINYFORMAT_MAKE_FORMAT_FUNCS)
 #endif
 
 // Added for Bitcoin Core
+/**
+ * Safer alternative to tfm::format_raw that does not throw string
+ * formatting errors at run-time.
+ *
+ * The `fmt` format string is partially checked at compile-time. At
+ * run-time, if any `tfm::format_error` occurs, the error is caught and
+ * the error message is returned instead, so the caller doesn't need to
+ * handle the error.
+ */
 template <typename... Args>
 std::string format(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
 {
-    return format_raw(fmt.fmt, args...);
+    try {
+        return tfm::format_raw(fmt.fmt, args...);
+    } catch (tinyformat::format_error& fmterr) {
+        return "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: \"" + fmt.fmt + "\"";
+    }
 }
 template <typename... Args>
 void format(std::ostream& out, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
 {
-    return format_raw(out, fmt.fmt, args...);
+    try {
+        tfm::format_raw(out, fmt.fmt, args...);
+    } catch (tinyformat::format_error& fmterr) {
+        out << "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: \"" + fmt.fmt + "\"";
+    }
 }
 } // namespace tinyformat
 

test/lint/run-lint-format-strings.py

@@ -15,6 +15,9 @@
 FALSE_POSITIVES = [
     ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS), COPYRIGHT_HOLDERS_SUBSTITUTION)"),
     ("src/test/translation_tests.cpp", "strprintf(format, arg)"),
+    ("src/test/util_string_tests.cpp", r'tfm::format("%*c", "dummy")'),
+    ("src/test/util_string_tests.cpp", r'tfm::format("%2$*3$d", "dummy", "value")'),
+    ("src/test/util_string_tests.cpp", r'tfm::format(oss, "%.*f", 5)'),
 ]