Compare commits

..

2 Commits

Author SHA1 Message Date
Mayukha Vadari
1bf9e6e7da fix: Remove a newline from logging statement in changeSpotPrice calculation (#6547)
This change removes an unnecessary newline in a logging statement. Namely, `std::endl` is unneeded in `JLOG`, since it automatically places a newline at the end of the string.
2026-03-13 18:12:25 +00:00
Bronek Kozicki
0446bef7e5 feat: Enforce feature name lengths and character set (#5555)
This change enforces a maximum length of 63 characters on feature names, as well as not permitting an exactly 32 character long feature name to avoid confusion with those that use a `uint256` hex representation, as that is an alternative way to specify a feature. This change further prevents the use of Unicode characters in feature names, because some can be confused with regular ASCII characters despite being valid in identifiers.
2026-03-13 13:41:50 -04:00
4 changed files with 95 additions and 8 deletions

View File

@@ -64,6 +64,49 @@
namespace xrpl {
// Feature names must not exceed this length (in characters, excluding the null terminator).
static constexpr std::size_t maxFeatureNameSize = 63;
// Reserve this exact feature-name length (in characters/bytes, excluding the null terminator)
// so that a 32-byte uint256 (for example, in WASM or other interop contexts) can be used
// as a compact, fixed-size feature selector without conflicting with human-readable names.
static constexpr std::size_t reservedFeatureNameSize = 32;
// Both validFeatureNameSize and validFeatureName are consteval functions that can be used in
// static_asserts to validate feature names at compile time. They are only used inside
// enforceValidFeatureName in Feature.cpp, but are exposed here for testing. The expected
// parameter `auto fn` is a constexpr lambda which returns a const char*, making it available
// for compile-time evaluation. Read more in https://accu.org/journals/overload/30/172/wu/
consteval auto
validFeatureNameSize(auto fn) -> bool
{
constexpr char const* n = fn();
// Note, std::strlen is not constexpr, we need to implement our own here.
constexpr std::size_t N = [](auto n) {
std::size_t ret = 0;
for (auto ptr = n; *ptr != '\0'; ret++, ++ptr)
;
return ret;
}(n);
return N != reservedFeatureNameSize && //
N <= maxFeatureNameSize;
}
consteval auto
validFeatureName(auto fn) -> bool
{
constexpr char const* n = fn();
// Prevent the use of visually confusable characters and enforce that feature names
// are always valid ASCII. This is needed because C++ allows Unicode identifiers.
// Characters below 0x20 are nonprintable control characters, and characters with the 0x80 bit
// set are non-ASCII (e.g. UTF-8 encoding of Unicode), so both are disallowed.
for (auto ptr = n; *ptr != '\0'; ++ptr)
{
if (*ptr & 0x80 || *ptr < 0x20)
return false;
}
return true;
}
enum class VoteBehavior : int { Obsolete = -1, DefaultNo = 0, DefaultYes };
enum class AmendmentSupport : int { Retired = -1, Supported = 0, Unsupported };

View File

@@ -370,7 +370,7 @@ changeSpotPriceQuality(
if (!amounts)
{
JLOG(j.trace()) << "changeSpotPrice calc failed: " << to_string(pool.in) << " "
<< to_string(pool.out) << " " << quality << " " << tfee << std::endl;
<< to_string(pool.out) << " " << quality << " " << tfee;
return std::nullopt;
}

View File

@@ -395,10 +395,20 @@ featureToName(uint256 const& f)
#pragma push_macro("XRPL_RETIRE_FIX")
#undef XRPL_RETIRE_FIX
consteval auto
enforceValidFeatureName(auto fn) -> char const*
{
static_assert(validFeatureName(fn), "Invalid feature name");
static_assert(validFeatureNameSize(fn), "Invalid feature name size");
return fn();
}
#define XRPL_FEATURE(name, supported, vote) \
uint256 const feature##name = registerFeature(#name, supported, vote);
uint256 const feature##name = \
registerFeature(enforceValidFeatureName([] { return #name; }), supported, vote);
#define XRPL_FIX(name, supported, vote) \
uint256 const fix##name = registerFeature("fix" #name, supported, vote);
uint256 const fix##name = \
registerFeature(enforceValidFeatureName([] { return "fix" #name; }), supported, vote);
// clang-format off
#define XRPL_RETIRE_FEATURE(name) \

View File

@@ -2,6 +2,7 @@
#include <xrpl/ledger/AmendmentTable.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/digest.h>
#include <xrpl/protocol/jss.h>
namespace xrpl {
@@ -168,16 +169,18 @@ class Feature_test : public beast::unit_test::suite
using namespace test::jtx;
Env env{*this};
auto jrr = env.rpc("feature", "fixAMMOverflowOffer")[jss::result];
std::string const name = "fixAMMOverflowOffer";
auto jrr = env.rpc("feature", name)[jss::result];
BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
jrr.removeMember(jss::status);
BEAST_EXPECT(jrr.size() == 1);
BEAST_EXPECT(jrr.isMember(
"12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E"
"2A87F1D8107"));
auto const expected = to_string(sha512Half(Slice(name.data(), name.size())));
char const sha[] = "12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E2A87F1D8107";
BEAST_EXPECT(expected == sha);
BEAST_EXPECT(jrr.isMember(expected));
auto feature = *(jrr.begin());
BEAST_EXPECTS(feature[jss::name] == "fixAMMOverflowOffer", "name");
BEAST_EXPECTS(feature[jss::name] == name, "name");
BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled");
BEAST_EXPECTS(feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), "vetoed");
BEAST_EXPECTS(feature[jss::supported].asBool(), "supported");
@@ -186,6 +189,37 @@ class Feature_test : public beast::unit_test::suite
jrr = env.rpc("feature", "fMM")[jss::result];
BEAST_EXPECT(jrr[jss::error] == "badFeature");
BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid.");
// Test feature name size checks
constexpr auto ok63Name = [] {
return "123456789012345678901234567890123456789012345678901234567890123";
};
static_assert(validFeatureNameSize(ok63Name));
constexpr auto bad64Name = [] {
return "1234567890123456789012345678901234567890123456789012345678901234";
};
static_assert(!validFeatureNameSize(bad64Name));
constexpr auto ok31Name = [] { return "1234567890123456789012345678901"; };
static_assert(validFeatureNameSize(ok31Name));
constexpr auto bad32Name = [] { return "12345678901234567890123456789012"; };
static_assert(!validFeatureNameSize(bad32Name));
constexpr auto ok33Name = [] { return "123456789012345678901234567890123"; };
static_assert(validFeatureNameSize(ok33Name));
// Test feature character set checks
constexpr auto okName = [] { return "AMM_123"; };
static_assert(validFeatureName(okName));
// First character is Greek Capital Alpha, visually confusable with ASCII 'A'
constexpr auto badName = [] { return "ΑMM_123"; };
static_assert(!validFeatureName(badName));
constexpr auto badEmoji = [] { return "🔥"; };
static_assert(!validFeatureName(badEmoji));
}
void