fa3da29 wallet: Reject trailing newline in format strings:
This seems a bit hacky to me. Adding a boolean on the general ConstevalFormatString leaks the newline check into tinyformat and index code. Please consider moving the validation into util::log::FormatString instead, where trailing newlines are already rejected.
<details><summary>Keep the newline check in logging</summary>
diff --git a/src/index/base.h b/src/index/base.h
index 9890e019da..731bbd26ba 100644
--- a/src/index/base.h
+++ b/src/index/base.h
@@ -12,7 +12,6 @@
#include <sync.h>
#include <uint256.h>
#include <util/fs.h>
-#include <util/string.h>
#include <util/threadinterrupt.h>
#include <validationinterface.h>
@@ -37,6 +36,11 @@ struct IndexSummary {
namespace interfaces {
struct BlockRef;
}
+namespace util {
+template <unsigned int num_params>
+struct ConstevalFormatString;
+}
+
/**
* Base class for indices of blockchain data. This implements
* CValidationInterface and ensures blocks are indexed sequentially according
diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
index d71bc22ef9..505a40eab9 100644
--- a/src/test/logging_tests.cpp
+++ b/src/test/logging_tests.cpp
@@ -21,6 +21,7 @@
#include <iostream>
#include <source_location>
#include <string>
+#include <type_traits>
#include <unordered_map>
#include <utility>
#include <vector>
@@ -35,6 +36,12 @@ namespace BCLog {
std::string LogEscapeMessage(std::string_view str);
}
+template <auto str>
+concept IsLogFormatString = requires { typename std::bool_constant<(util::log::FormatString<0>{str()}, true)>; };
+
+static_assert( IsLogFormatString<[] { return "format string without a trailing newline"; }>);
+static_assert(!IsLogFormatString<[] { return "format string ending in a newline\n"; }>);
+
BOOST_FIXTURE_TEST_SUITE(logging_tests, BasicTestingSetup)
static void ResetLogger()
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 73ee89ce54..894c592ed0 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -2,32 +2,18 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#include <test/util/common.h>
-#include <tinyformat.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <boost/test/unit_test.hpp>
-
-#include <type_traits>
+#include <test/util/common.h>
+#include <tinyformat.h>
using namespace util;
using util::detail::CheckNumFormatSpecifiers;
BOOST_AUTO_TEST_SUITE(util_string_tests)
-namespace {
-constexpr char NO_TRAILING_NEWLINE[] = "format string without a trailing newline";
-constexpr char TRAILING_NEWLINE[] = "format string ending in a newline\n";
-
-template <const char* str>
-concept IsStrictFormatString =
- requires { typename std::bool_constant<(ConstevalFormatString</*num_params=*/0, /*permit_trailing_newline=*/false>{str}, true)>; };
-
-static_assert(IsStrictFormatString<NO_TRAILING_NEWLINE>);
-static_assert(!IsStrictFormatString<TRAILING_NEWLINE>);
-} // namespace
-
template <unsigned NumArgs>
void TfmFormatZeroes(const std::string& fmt)
{
diff --git a/src/tinyformat.h b/src/tinyformat.h
index c21a7507b0..444cd6ded5 100644
--- a/src/tinyformat.h
+++ b/src/tinyformat.h
@@ -193,8 +193,7 @@ template <unsigned num_params>
struct FormatStringCheck {
consteval FormatStringCheck(const char* str) : fmt{util::ConstevalFormatString<num_params>{str}.fmt} {}
FormatStringCheck(LIFETIMEBOUND const RuntimeFormat& run) : fmt{run.fmt.c_str()} {}
- template <bool permit_trailing_newline>
- FormatStringCheck(util::ConstevalFormatString<num_params, permit_trailing_newline> str) : fmt{str.fmt} {}
+ FormatStringCheck(util::ConstevalFormatString<num_params> str) : fmt{str.fmt} {}
operator const char*() { return fmt; }
const char* fmt;
};
diff --git a/src/util/log.h b/src/util/log.h
index 33d0b91bdf..9dbbadc60d 100644
--- a/src/util/log.h
+++ b/src/util/log.h
@@ -80,6 +80,15 @@ bool ShouldTraceLog(Category category);
/** Send message to be logged. Applications using the logging library need to provide this. */
void Log(Entry entry);
+/** A format string for a log entry. The logger appends the trailing newline, so reject one in the format. */
+template <unsigned num_params>
+struct FormatString : util::ConstevalFormatString<num_params> {
+ consteval FormatString(const char* str) : util::ConstevalFormatString<num_params>{str}
+ {
+ if (std::string_view{str}.ends_with('\n')) throw "Format string must not end with a newline";
+ }
+};
+
template <typename... Args>
inline void LogPrintFormatInternal_(SourceLocation&& source_loc, BCLog::LogFlags flag, util::log::Level level, bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
{
diff --git a/src/util/string.h b/src/util/string.h
index 0809e84c8a..b2be2e7f64 100644
--- a/src/util/string.h
+++ b/src/util/string.h
@@ -92,16 +92,10 @@ constexpr void CheckNumFormatSpecifiers(const char* str)
* run-time. Validation is partial to try and prevent the most common errors
* while avoiding re-implementing the entire parsing logic.
*/
-template <unsigned num_params, bool permit_trailing_newline = true>
+template <unsigned num_params>
struct ConstevalFormatString {
const char* const fmt;
- consteval ConstevalFormatString(const char* str) : fmt{str}
- {
- detail::CheckNumFormatSpecifiers<num_params>(fmt);
- if constexpr (!permit_trailing_newline) {
- if (std::string_view{fmt}.ends_with('\n')) throw "Format string must not end with a newline";
- }
- }
+ consteval ConstevalFormatString(const char* str) : fmt{str} { detail::CheckNumFormatSpecifiers<num_params>(fmt); }
};
/// Replace every non-overlapping occurrence of `search` with `substitute`, treating both literally; the replacement text is not searched again.
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index 6343746239..f9b7303285 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -147,7 +147,7 @@ public:
/** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */
template <typename... Params>
- void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params), /*permit_trailing_newline=*/false> wallet_fmt, const Params&... params) const
+ void WalletLogPrintf(util::log::FormatString<sizeof...(Params)> wallet_fmt, const Params&... params) const
{
LogInfo("[%s] %s", m_storage.LogName(), tfm::format(wallet_fmt, params...));
};
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index ad67aa0b4b..350d7e4db1 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -923,7 +923,7 @@ public:
/** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */
template <typename... Params>
- void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params), /*permit_trailing_newline=*/false> wallet_fmt, const Params&... params) const
+ void WalletLogPrintf(util::log::FormatString<sizeof...(Params)> wallet_fmt, const Params&... params) const
{
LogInfo("[%s] %s", LogName(), tfm::format(wallet_fmt, params...));
};
</details>
