Skip to content

Commit

Permalink
util: add and use run-time safe tinyformat::try_format
Browse files Browse the repository at this point in the history
In certain situations, such as logging, string formatting errors
should not cause application failures. We already use
util::ConstevalFormatString in those cases, to try and catch some
errors at compile-time.

Improve this further by suppressing run-time errors by returning
the error message instead of throwing a tinyformat::format_error.
This is done by taking the existing error handling in
LogPrintFormatInternal(), and move it to tfm::try_format().
  • Loading branch information
stickies-v committed Sep 19, 2024
1 parent 2db926f commit 6dc6e45
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/index/base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s};
template <typename... Args>
void BaseIndex::FatalErrorf(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
{
auto message = tfm::format(fmt, args...);
auto message{tfm::try_format(fmt, args...)};
node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message), m_chain->context()->warnings.get());
}

Expand Down
8 changes: 1 addition & 7 deletions src/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -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::try_format(fmt, args...)};
LogInstance().LogPrintStr(log_msg, logging_function, source_file, source_line, flag, level);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/netbase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ std::function<std::unique_ptr<Sock>(int, int, int)> CreateSock = CreateSockOS;
template<typename... Args>
static void LogConnectFailure(bool manual_connection, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
{
std::string error_message = tfm::format(fmt, args...);
std::string error_message{tfm::try_format(fmt, args...)};
if (manual_connection) {
LogPrintf("%s\n", error_message);
} else {
Expand Down
9 changes: 9 additions & 0 deletions src/test/util_string_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// 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>
Expand Down Expand Up @@ -83,4 +84,12 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
FailFmtWithError<1>("%1$", err_term);
}

BOOST_AUTO_TEST_CASE(tinyformat_try_format)
{
// ensure invalid format strings don't throw at run-time
BOOST_CHECK_EQUAL(tfm::try_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::try_format("%2$*3$d", "dummy", "value"), R"(Error "tinyformat: Positional argument out of range" while formatting log message: "%2$*3$d")");
BOOST_CHECK_EQUAL(tfm::try_format("%.*f", 5), R"(Error "tinyformat: Too many conversion specifiers in format string" while formatting log message: "%.*f")");
}

BOOST_AUTO_TEST_SUITE_END()
18 changes: 16 additions & 2 deletions src/util/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,25 @@ template <typename T1, size_t PREFIX_LEN>
}
} // namespace util

/**
* Safer alternative to tfm::format 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 `tinyformat::format_error` occurs, the error is caught
* and the error message is returned instead, so the caller doesn't need
* to handle the error.
*/
namespace tinyformat {
template <typename... Args>
std::string format(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
std::string try_format(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
{
return format(fmt.fmt, args...);
try {
return tfm::format(fmt.fmt, args...);
} catch (tinyformat::format_error& fmterr) {
/* Original format string will have newline so don't add one here */
return "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: \"" + fmt.fmt + "\"";
}
}
} // namespace tinyformat

Expand Down
2 changes: 1 addition & 1 deletion src/wallet/scriptpubkeyman.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class ScriptPubKeyMan
template <typename... Params>
void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params)> wallet_fmt, const Params&... params) const
{
LogInfo("%s %s", m_storage.GetDisplayName(), tfm::format(wallet_fmt, params...));
LogInfo("%s %s", m_storage.GetDisplayName(), tfm::try_format(wallet_fmt, params...));
};

/** Watch-only address added */
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
template <typename... Params>
void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params)> wallet_fmt, const Params&... params) const
{
LogInfo("%s %s", GetDisplayName(), tfm::format(wallet_fmt, params...));
LogInfo("%s %s", GetDisplayName(), tfm::try_format(wallet_fmt, params...));
};

/** Upgrade the wallet */
Expand Down

0 comments on commit 6dc6e45

Please sign in to comment.