From f7b3ddd87b8ef093a06ab1420bea57ed1e77643a Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Tue, 14 Mar 2023 20:49:40 -0700 Subject: [PATCH 1/4] Reporting Mode: Do not attempt to acquire missing data from peer network (#4458) In Reporting Mode, a server would core dump when it is not able to read from Cassandra. This patch prevents the core dump when Cassandra is down for reporting mode servers. This does not fix the root cause, but it cuts down on some of the resulting noise. --- src/ripple/app/ledger/Ledger.cpp | 2 +- src/ripple/shamap/Family.h | 16 +++++++++++-- src/ripple/shamap/NodeFamily.h | 4 ++-- src/ripple/shamap/ShardFamily.h | 5 ++-- src/ripple/shamap/impl/NodeFamily.cpp | 10 +++++++- src/ripple/shamap/impl/SHAMap.cpp | 32 +++++++++++++++++--------- src/ripple/shamap/impl/ShardFamily.cpp | 4 +++- src/test/shamap/common.h | 6 +++-- 8 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 7757dac53bf..7552f755c6e 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -266,7 +266,7 @@ Ledger::Ledger( { info_.hash = calculateLedgerHash(info_); if (acquire && !config.reporting()) - family.missingNode(info_.hash, info_.seq); + family.missingNodeAcquireByHash(info_.hash, info_.seq); } } diff --git a/src/ripple/shamap/Family.h b/src/ripple/shamap/Family.h index 72c9a6cb07a..fea5545d31c 100644 --- a/src/ripple/shamap/Family.h +++ b/src/ripple/shamap/Family.h @@ -75,11 +75,23 @@ class Family virtual bool isShardBacked() const = 0; + /** Acquire ledger that has a missing node by ledger sequence + * + * Throw if in reporting mode. + * + * @param refNum Sequence of ledger to acquire. + * @param nodeHash Hash of missing node to report in throw. + */ virtual void - missingNode(std::uint32_t refNum) = 0; + missingNodeAcquireBySeq(std::uint32_t refNum, uint256 const& nodeHash) = 0; + /** Acquire ledger that has a missing node by ledger hash + * + * @param refHash Hash of ledger to acquire. + * @param refNum Ledger sequence with missing node. + */ virtual void - missingNode(uint256 const& refHash, std::uint32_t refNum) = 0; + missingNodeAcquireByHash(uint256 const& refHash, std::uint32_t refNum) = 0; virtual void reset() = 0; diff --git a/src/ripple/shamap/NodeFamily.h b/src/ripple/shamap/NodeFamily.h index 2d8236705b5..f20abccce9d 100644 --- a/src/ripple/shamap/NodeFamily.h +++ b/src/ripple/shamap/NodeFamily.h @@ -83,10 +83,10 @@ class NodeFamily : public Family reset() override; void - missingNode(std::uint32_t seq) override; + missingNodeAcquireBySeq(std::uint32_t seq, uint256 const& hash) override; void - missingNode(uint256 const& hash, std::uint32_t seq) override + missingNodeAcquireByHash(uint256 const& hash, std::uint32_t seq) override { acquire(hash, seq); } diff --git a/src/ripple/shamap/ShardFamily.h b/src/ripple/shamap/ShardFamily.h index 550efeb5b81..de809cf589c 100644 --- a/src/ripple/shamap/ShardFamily.h +++ b/src/ripple/shamap/ShardFamily.h @@ -89,10 +89,11 @@ class ShardFamily : public Family reset() override; void - missingNode(std::uint32_t seq) override; + missingNodeAcquireBySeq(std::uint32_t seq, uint256 const& nodeHash) + override; void - missingNode(uint256 const& hash, std::uint32_t seq) override + missingNodeAcquireByHash(uint256 const& hash, std::uint32_t seq) override { acquire(hash, seq); } diff --git a/src/ripple/shamap/impl/NodeFamily.cpp b/src/ripple/shamap/impl/NodeFamily.cpp index f9c6dedb265..1752db06a8e 100644 --- a/src/ripple/shamap/impl/NodeFamily.cpp +++ b/src/ripple/shamap/impl/NodeFamily.cpp @@ -21,6 +21,7 @@ #include #include #include +#include namespace ripple { @@ -65,9 +66,16 @@ NodeFamily::reset() } void -NodeFamily::missingNode(std::uint32_t seq) +NodeFamily::missingNodeAcquireBySeq(std::uint32_t seq, uint256 const& nodeHash) { JLOG(j_.error()) << "Missing node in " << seq; + if (app_.config().reporting()) + { + std::stringstream ss; + ss << "Node not read, likely a Cassandra error in ledger seq " << seq + << " object hash " << nodeHash; + Throw(ss.str()); + } std::unique_lock lock(maxSeqMutex_); if (maxSeq_ == 0) diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index 1a5a283dd3c..fa42c8e8f82 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -173,30 +173,40 @@ SHAMap::finishFetch( std::shared_ptr const& object) const { assert(backed_); - if (!object) - { - if (full_) - { - full_ = false; - f_.missingNode(ledgerSeq_); - } - return {}; - } std::shared_ptr node; try { + if (!object) + { + if (full_) + { + full_ = false; + f_.missingNodeAcquireBySeq(ledgerSeq_, hash.as_uint256()); + } + return {}; + } + node = SHAMapTreeNode::makeFromPrefix(makeSlice(object->getData()), hash); if (node) canonicalize(hash, node); return node; } - catch (std::exception const&) + catch (SHAMapMissingNode const& e) + { + JLOG(journal_.warn()) << "Missing node: " << hash << " : " << e.what(); + } + catch (std::runtime_error const& e) + { + JLOG(journal_.warn()) << e.what(); + } + catch (...) { JLOG(journal_.warn()) << "Invalid DB node " << hash; - return std::shared_ptr(); } + + return std::shared_ptr(); } // See if a sync filter has a node diff --git a/src/ripple/shamap/impl/ShardFamily.cpp b/src/ripple/shamap/impl/ShardFamily.cpp index eadfc42aa27..f22d4152e2b 100644 --- a/src/ripple/shamap/impl/ShardFamily.cpp +++ b/src/ripple/shamap/impl/ShardFamily.cpp @@ -22,6 +22,7 @@ #include #include #include +#include namespace ripple { @@ -152,8 +153,9 @@ ShardFamily::reset() } void -ShardFamily::missingNode(std::uint32_t seq) +ShardFamily::missingNodeAcquireBySeq(std::uint32_t seq, uint256 const& nodeHash) { + std::ignore = nodeHash; JLOG(j_.error()) << "Missing node in ledger sequence " << seq; std::unique_lock lock(maxSeqMutex_); diff --git a/src/test/shamap/common.h b/src/test/shamap/common.h index c4238b2a65f..d89acb988d7 100644 --- a/src/test/shamap/common.h +++ b/src/test/shamap/common.h @@ -105,13 +105,15 @@ class TestNodeFamily : public Family } void - missingNode(std::uint32_t refNum) override + missingNodeAcquireBySeq(std::uint32_t refNum, uint256 const& nodeHash) + override { Throw("missing node"); } void - missingNode(uint256 const& refHash, std::uint32_t refNum) override + missingNodeAcquireByHash(uint256 const& refHash, std::uint32_t refNum) + override { Throw("missing node"); } From 84cde3ce0b8dba9d39288e9dad37497593aef3ab Mon Sep 17 00:00:00 2001 From: drlongle Date: Wed, 15 Mar 2023 04:54:54 +0100 Subject: [PATCH 2/4] Use <=> operator for base_uint, Issue, and Book: (#4411) - Implement the `operator==` and the `operator<=>` (aka the spaceship operator) in `base_uint`, `Issue`, and `Book`. - C++20-compliant compilers automatically provide the remaining comparison operators (e.g. `operator<`, `operator<=`, ...). - Remove the function compare() because it is no longer needed. - Maintain the same semantics as the existing code. - Add some unit tests to gain further confidence. - Fix #2525. --- .../app/tx/impl/details/NFTokenUtils.cpp | 11 ++- src/ripple/basics/base_uint.h | 86 ++++++------------- src/ripple/protocol/Book.h | 28 +++--- src/ripple/protocol/Issue.h | 36 ++++---- src/ripple/protocol/impl/Book.cpp | 54 ------------ src/ripple/protocol/impl/Issue.cpp | 56 ------------ src/test/basics/base_uint_test.cpp | 83 ++++++++++++++++-- 7 files changed, 138 insertions(+), 216 deletions(-) diff --git a/src/ripple/app/tx/impl/details/NFTokenUtils.cpp b/src/ripple/app/tx/impl/details/NFTokenUtils.cpp index db2c3ae62f7..09ff8f13caa 100644 --- a/src/ripple/app/tx/impl/details/NFTokenUtils.cpp +++ b/src/ripple/app/tx/impl/details/NFTokenUtils.cpp @@ -146,19 +146,22 @@ getPageForToken( return nullptr; else { - // This would be an ideal place for the spaceship operator... - int const relation = compare(id & nft::pageMask, cmp); + auto const relation{(id & nft::pageMask) <=> cmp}; if (relation == 0) + { // If the passed in id belongs exactly on this (full) page // this account simply cannot store the NFT. return nullptr; + } - else if (relation > 0) + if (relation > 0) + { // We need to leave the entire contents of this page in // narr so carr stays empty. The new NFT will be // inserted in carr. This keeps the NFTs that must be // together all on their own page. splitIter = narr.end(); + } // If neither of those conditions apply then put all of // narr into carr and produce an empty narr where the new NFT @@ -228,7 +231,7 @@ compareTokens(uint256 const& a, uint256 const& b) // 96-bits are identical we still need a fully deterministic sort. // So we sort on the low 96-bits first. If those are equal we sort on // the whole thing. - if (auto const lowBitsCmp = compare(a & nft::pageMask, b & nft::pageMask); + if (auto const lowBitsCmp{(a & nft::pageMask) <=> (b & nft::pageMask)}; lowBitsCmp != 0) return lowBitsCmp < 0; diff --git a/src/ripple/basics/base_uint.h b/src/ripple/basics/base_uint.h index 8f277c3003c..8b15b082647 100644 --- a/src/ripple/basics/base_uint.h +++ b/src/ripple/basics/base_uint.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -549,103 +550,66 @@ using uint160 = base_uint<160>; using uint256 = base_uint<256>; template -inline int -compare(base_uint const& a, base_uint const& b) +[[nodiscard]] inline constexpr std::strong_ordering +operator<=>(base_uint const& lhs, base_uint const& rhs) { - auto ret = std::mismatch(a.cbegin(), a.cend(), b.cbegin()); - - if (ret.first == a.cend()) - return 0; + // This comparison might seem wrong on a casual inspection because it + // compares data internally stored as std::uint32_t byte-by-byte. But + // note that the underlying data is stored in big endian, even if the + // plaform is little endian. This makes the comparison correct. + // + // FIXME: use std::lexicographical_compare_three_way once support is + // added to MacOS. - // a > b - if (*ret.first > *ret.second) - return 1; + auto const ret = std::mismatch(lhs.cbegin(), lhs.cend(), rhs.cbegin()); - // a < b - return -1; -} - -template -inline bool -operator<(base_uint const& a, base_uint const& b) -{ - return compare(a, b) < 0; -} + // a == b + if (ret.first == lhs.cend()) + return std::strong_ordering::equivalent; -template -inline bool -operator<=(base_uint const& a, base_uint const& b) -{ - return compare(a, b) <= 0; + return (*ret.first > *ret.second) ? std::strong_ordering::greater + : std::strong_ordering::less; } -template -inline bool -operator>(base_uint const& a, base_uint const& b) +template +[[nodiscard]] inline constexpr bool +operator==(base_uint const& lhs, base_uint const& rhs) { - return compare(a, b) > 0; -} - -template -inline bool -operator>=(base_uint const& a, base_uint const& b) -{ - return compare(a, b) >= 0; -} - -template -inline bool -operator==(base_uint const& a, base_uint const& b) -{ - return compare(a, b) == 0; -} - -template -inline bool -operator!=(base_uint const& a, base_uint const& b) -{ - return compare(a, b) != 0; + return (lhs <=> rhs) == 0; } //------------------------------------------------------------------------------ template -inline bool +inline constexpr bool operator==(base_uint const& a, std::uint64_t b) { return a == base_uint(b); } -template -inline bool -operator!=(base_uint const& a, std::uint64_t b) -{ - return !(a == b); -} - //------------------------------------------------------------------------------ template -inline const base_uint +inline constexpr base_uint operator^(base_uint const& a, base_uint const& b) { return base_uint(a) ^= b; } template -inline const base_uint +inline constexpr base_uint operator&(base_uint const& a, base_uint const& b) { return base_uint(a) &= b; } template -inline const base_uint +inline constexpr base_uint operator|(base_uint const& a, base_uint const& b) { return base_uint(a) |= b; } template -inline const base_uint +inline constexpr base_uint operator+(base_uint const& a, base_uint const& b) { return base_uint(a) += b; diff --git a/src/ripple/protocol/Book.h b/src/ripple/protocol/Book.h index 1469b60dd1b..609989062c0 100644 --- a/src/ripple/protocol/Book.h +++ b/src/ripple/protocol/Book.h @@ -65,28 +65,24 @@ hash_append(Hasher& h, Book const& b) Book reversed(Book const& book); -/** Ordered comparison. */ -int -compare(Book const& lhs, Book const& rhs); - /** Equality comparison. */ /** @{ */ -bool -operator==(Book const& lhs, Book const& rhs); -bool -operator!=(Book const& lhs, Book const& rhs); +[[nodiscard]] inline constexpr bool +operator==(Book const& lhs, Book const& rhs) +{ + return (lhs.in == rhs.in) && (lhs.out == rhs.out); +} /** @} */ /** Strict weak ordering. */ /** @{ */ -bool -operator<(Book const& lhs, Book const& rhs); -bool -operator>(Book const& lhs, Book const& rhs); -bool -operator>=(Book const& lhs, Book const& rhs); -bool -operator<=(Book const& lhs, Book const& rhs); +[[nodiscard]] inline constexpr std::weak_ordering +operator<=>(Book const& lhs, Book const& rhs) +{ + if (auto const c{lhs.in <=> rhs.in}; c != 0) + return c; + return lhs.out <=> rhs.out; +} /** @} */ } // namespace ripple diff --git a/src/ripple/protocol/Issue.h b/src/ripple/protocol/Issue.h index 11c45c0136c..be7733677a1 100644 --- a/src/ripple/protocol/Issue.h +++ b/src/ripple/protocol/Issue.h @@ -63,31 +63,29 @@ hash_append(Hasher& h, Issue const& r) hash_append(h, r.currency, r.account); } -/** Ordered comparison. - The assets are ordered first by currency and then by account, - if the currency is not XRP. -*/ -int -compare(Issue const& lhs, Issue const& rhs); - /** Equality comparison. */ /** @{ */ -bool -operator==(Issue const& lhs, Issue const& rhs); -bool -operator!=(Issue const& lhs, Issue const& rhs); +[[nodiscard]] inline constexpr bool +operator==(Issue const& lhs, Issue const& rhs) +{ + return (lhs.currency == rhs.currency) && + (isXRP(lhs.currency) || lhs.account == rhs.account); +} /** @} */ /** Strict weak ordering. */ /** @{ */ -bool -operator<(Issue const& lhs, Issue const& rhs); -bool -operator>(Issue const& lhs, Issue const& rhs); -bool -operator>=(Issue const& lhs, Issue const& rhs); -bool -operator<=(Issue const& lhs, Issue const& rhs); +[[nodiscard]] inline constexpr std::weak_ordering +operator<=>(Issue const& lhs, Issue const& rhs) +{ + if (auto const c{lhs.currency <=> rhs.currency}; c != 0) + return c; + + if (isXRP(lhs.currency)) + return std::weak_ordering::equivalent; + + return (lhs.account <=> rhs.account); +} /** @} */ //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/Book.cpp b/src/ripple/protocol/impl/Book.cpp index 323985e6114..3ad22675d1b 100644 --- a/src/ripple/protocol/impl/Book.cpp +++ b/src/ripple/protocol/impl/Book.cpp @@ -47,58 +47,4 @@ reversed(Book const& book) return Book(book.out, book.in); } -/** Ordered comparison. */ -int -compare(Book const& lhs, Book const& rhs) -{ - int const diff(compare(lhs.in, rhs.in)); - if (diff != 0) - return diff; - return compare(lhs.out, rhs.out); -} - -/** Equality comparison. */ -/** @{ */ -bool -operator==(Book const& lhs, Book const& rhs) -{ - return (lhs.in == rhs.in) && (lhs.out == rhs.out); -} - -bool -operator!=(Book const& lhs, Book const& rhs) -{ - return (lhs.in != rhs.in) || (lhs.out != rhs.out); -} -/** @} */ - -/** Strict weak ordering. */ -/** @{ */ -bool -operator<(Book const& lhs, Book const& rhs) -{ - int const diff(compare(lhs.in, rhs.in)); - if (diff != 0) - return diff < 0; - return lhs.out < rhs.out; -} - -bool -operator>(Book const& lhs, Book const& rhs) -{ - return rhs < lhs; -} - -bool -operator>=(Book const& lhs, Book const& rhs) -{ - return !(lhs < rhs); -} - -bool -operator<=(Book const& lhs, Book const& rhs) -{ - return !(rhs < lhs); -} - } // namespace ripple diff --git a/src/ripple/protocol/impl/Issue.cpp b/src/ripple/protocol/impl/Issue.cpp index 24a8c764efb..e727cb4cade 100644 --- a/src/ripple/protocol/impl/Issue.cpp +++ b/src/ripple/protocol/impl/Issue.cpp @@ -43,60 +43,4 @@ operator<<(std::ostream& os, Issue const& x) return os; } -/** Ordered comparison. - The assets are ordered first by currency and then by account, - if the currency is not XRP. -*/ -int -compare(Issue const& lhs, Issue const& rhs) -{ - int diff = compare(lhs.currency, rhs.currency); - if (diff != 0) - return diff; - if (isXRP(lhs.currency)) - return 0; - return compare(lhs.account, rhs.account); -} - -/** Equality comparison. */ -/** @{ */ -bool -operator==(Issue const& lhs, Issue const& rhs) -{ - return compare(lhs, rhs) == 0; -} - -bool -operator!=(Issue const& lhs, Issue const& rhs) -{ - return !(lhs == rhs); -} -/** @} */ - -/** Strict weak ordering. */ -/** @{ */ -bool -operator<(Issue const& lhs, Issue const& rhs) -{ - return compare(lhs, rhs) < 0; -} - -bool -operator>(Issue const& lhs, Issue const& rhs) -{ - return rhs < lhs; -} - -bool -operator>=(Issue const& lhs, Issue const& rhs) -{ - return !(lhs < rhs); -} - -bool -operator<=(Issue const& lhs, Issue const& rhs) -{ - return !(rhs < lhs); -} - } // namespace ripple diff --git a/src/test/basics/base_uint_test.cpp b/src/test/basics/base_uint_test.cpp index c1ba7302ae8..9b1f7696dd5 100644 --- a/src/test/basics/base_uint_test.cpp +++ b/src/test/basics/base_uint_test.cpp @@ -57,8 +57,76 @@ struct nonhash struct base_uint_test : beast::unit_test::suite { using test96 = base_uint<96>; - static_assert(std::is_copy_constructible::value, ""); - static_assert(std::is_copy_assignable::value, ""); + static_assert(std::is_copy_constructible::value); + static_assert(std::is_copy_assignable::value); + + void + testComparisons() + { + { + static constexpr std:: + array, 6> + test_args{ + {{"0000000000000000", "0000000000000001"}, + {"0000000000000000", "ffffffffffffffff"}, + {"1234567812345678", "2345678923456789"}, + {"8000000000000000", "8000000000000001"}, + {"aaaaaaaaaaaaaaa9", "aaaaaaaaaaaaaaaa"}, + {"fffffffffffffffe", "ffffffffffffffff"}}}; + + for (auto const& arg : test_args) + { + ripple::base_uint<64> const u{arg.first}, v{arg.second}; + BEAST_EXPECT(u < v); + BEAST_EXPECT(u <= v); + BEAST_EXPECT(u != v); + BEAST_EXPECT(!(u == v)); + BEAST_EXPECT(!(u > v)); + BEAST_EXPECT(!(u >= v)); + BEAST_EXPECT(!(v < u)); + BEAST_EXPECT(!(v <= u)); + BEAST_EXPECT(v != u); + BEAST_EXPECT(!(v == u)); + BEAST_EXPECT(v > u); + BEAST_EXPECT(v >= u); + BEAST_EXPECT(u == u); + BEAST_EXPECT(v == v); + } + } + + { + static constexpr std::array< + std::pair, + 6> + test_args{{ + {"000000000000000000000000", "000000000000000000000001"}, + {"000000000000000000000000", "ffffffffffffffffffffffff"}, + {"0123456789ab0123456789ab", "123456789abc123456789abc"}, + {"555555555555555555555555", "55555555555a555555555555"}, + {"aaaaaaaaaaaaaaa9aaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaa"}, + {"fffffffffffffffffffffffe", "ffffffffffffffffffffffff"}, + }}; + + for (auto const& arg : test_args) + { + ripple::base_uint<96> const u{arg.first}, v{arg.second}; + BEAST_EXPECT(u < v); + BEAST_EXPECT(u <= v); + BEAST_EXPECT(u != v); + BEAST_EXPECT(!(u == v)); + BEAST_EXPECT(!(u > v)); + BEAST_EXPECT(!(u >= v)); + BEAST_EXPECT(!(v < u)); + BEAST_EXPECT(!(v <= u)); + BEAST_EXPECT(v != u); + BEAST_EXPECT(!(v == u)); + BEAST_EXPECT(v > u); + BEAST_EXPECT(v >= u); + BEAST_EXPECT(u == u); + BEAST_EXPECT(v == v); + } + } + } void run() override @@ -66,9 +134,12 @@ struct base_uint_test : beast::unit_test::suite testcase("base_uint: general purpose tests"); static_assert( - !std::is_constructible>::value, ""); + !std::is_constructible>::value); static_assert( - !std::is_assignable>::value, ""); + !std::is_assignable>::value); + + testComparisons(); + // used to verify set insertion (hashing required) std::unordered_set> uset; @@ -112,8 +183,8 @@ struct base_uint_test : beast::unit_test::suite BEAST_EXPECT(d == --t); } - BEAST_EXPECT(compare(u, v) < 0); - BEAST_EXPECT(compare(v, u) > 0); + BEAST_EXPECT(u < v); + BEAST_EXPECT(v > u); v = u; BEAST_EXPECT(v == u); From cb08f2b6ec425c4686c8c5c7fc4a3209e91166f2 Mon Sep 17 00:00:00 2001 From: RichardAH Date: Wed, 15 Mar 2023 05:06:30 +0100 Subject: [PATCH 3/4] Allow port numbers be be specified with a colon: (#4328) Port numbers can now be specified using either a colon or a space. Examples: 1.2.3.4:51235 1.2.3.4 51235 - In the configuration file, an annoying "gotcha" for node operators is accidentally specifying IP:PORT combinations using a colon. The code previously expected a space, not a colon. It also does not provide good feedback when this operator error is made. - This change simply allows this mistake (using a colon) to be fixed automatically, preserving the intention of the operator. - Add unit tests, which test the functionality when specifying IP:PORT in the configuration file. - The RPCCall test regime is not specific enough to test this functionality, it has been tested by hand. - Ensure IPv6 addresses are not confused for ip:port --------- Co-authored-by: Elliot Lee --- src/ripple/core/impl/Config.cpp | 23 +++++++++ src/ripple/net/impl/RPCCall.cpp | 22 ++++++-- src/ripple/rpc/handlers/Connect.cpp | 8 +-- src/test/core/Config_test.cpp | 79 +++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index c2cfb14d21d..12374972866 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #if BOOST_OS_WINDOWS @@ -468,6 +469,28 @@ Config::loadFromString(std::string const& fileContents) if (auto s = getIniFileSection(secConfig, SECTION_SNTP)) SNTP_SERVERS = *s; + // if the user has specified ip:port then replace : with a space. + { + auto replaceColons = [](std::vector& strVec) { + const static std::regex e(":([0-9]+)$"); + for (auto& line : strVec) + { + // skip anything that might be an ipv6 address + if (std::count(line.begin(), line.end(), ':') != 1) + continue; + + std::string result = std::regex_replace(line, e, " $1"); + // sanity check the result of the replace, should be same length + // as input + if (result.size() == line.size()) + line = result; + } + }; + + replaceColons(IPS_FIXED); + replaceColons(IPS); + } + { std::string dbPath; if (getSingleSection(secConfig, "database_path", dbPath, j_)) diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index b475afe9dfb..b5a167f76b0 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -482,17 +482,31 @@ class RPCParser return jvRequest; } - // connect [port] + // connect [port] Json::Value parseConnect(Json::Value const& jvParams) { Json::Value jvRequest(Json::objectValue); - - jvRequest[jss::ip] = jvParams[0u].asString(); - + std::string ip = jvParams[0u].asString(); if (jvParams.size() == 2) + { + jvRequest[jss::ip] = ip; jvRequest[jss::port] = jvParams[1u].asUInt(); + return jvRequest; + } + + // handle case where there is one argument of the form ip:port + if (std::count(ip.begin(), ip.end(), ':') == 1) + { + std::size_t colon = ip.find_last_of(":"); + jvRequest[jss::ip] = std::string{ip, 0, colon}; + jvRequest[jss::port] = + Json::Value{std::string{ip, colon + 1}}.asUInt(); + return jvRequest; + } + // default case, no port + jvRequest[jss::ip] = ip; return jvRequest; } diff --git a/src/ripple/rpc/handlers/Connect.cpp b/src/ripple/rpc/handlers/Connect.cpp index 532e04087aa..ed366f64b2b 100644 --- a/src/ripple/rpc/handlers/Connect.cpp +++ b/src/ripple/rpc/handlers/Connect.cpp @@ -59,13 +59,15 @@ doConnect(RPC::JsonContext& context) else iPort = DEFAULT_PEER_PORT; - auto ip = - beast::IP::Endpoint::from_string(context.params[jss::ip].asString()); + auto const ip_str = context.params[jss::ip].asString(); + auto ip = beast::IP::Endpoint::from_string(ip_str); if (!is_unspecified(ip)) context.app.overlay().connect(ip.at_port(iPort)); - return RPC::makeObjectValue("connecting"); + return RPC::makeObjectValue( + "attempting connection to IP:" + ip_str + + " port: " + std::to_string(iPort)); } } // namespace ripple diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index da29fafaca2..b455762dea4 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -856,6 +856,84 @@ r.ripple.com 51235 cfg.section(SECTION_IPS_FIXED).values().size() == 2); } + void + testColons() + { + Config cfg; + /* NOTE: this string includes some explicit + * space chars in order to verify proper trimming */ + std::string toLoad(R"( +[port_rpc])" + "\x20" + R"( +# comment + # indented comment +)" + "\x20\x20" + R"( +[ips])" + "\x20" + R"( +r.ripple.com:51235 + + [ips_fixed])" + "\x20\x20" + R"( + # COMMENT + s1.ripple.com:51235 + s2.ripple.com 51235 + anotherserversansport + anotherserverwithport:12 + 1.1.1.1:1 + 1.1.1.1 1 + 12.34.12.123:12345 + 12.34.12.123 12345 + :: + 2001:db8:: + ::1 + ::1:12345 + [::1]:12345 + 2001:db8:3333:4444:5555:6666:7777:8888:12345 + [2001:db8:3333:4444:5555:6666:7777:8888]:1 + + +)"); + cfg.loadFromString(toLoad); + BEAST_EXPECT( + cfg.exists("port_rpc") && cfg.section("port_rpc").lines().empty() && + cfg.section("port_rpc").values().empty()); + BEAST_EXPECT( + cfg.exists(SECTION_IPS) && + cfg.section(SECTION_IPS).lines().size() == 1 && + cfg.section(SECTION_IPS).values().size() == 1); + BEAST_EXPECT( + cfg.exists(SECTION_IPS_FIXED) && + cfg.section(SECTION_IPS_FIXED).lines().size() == 15 && + cfg.section(SECTION_IPS_FIXED).values().size() == 15); + BEAST_EXPECT(cfg.IPS[0] == "r.ripple.com 51235"); + + BEAST_EXPECT(cfg.IPS_FIXED[0] == "s1.ripple.com 51235"); + BEAST_EXPECT(cfg.IPS_FIXED[1] == "s2.ripple.com 51235"); + BEAST_EXPECT(cfg.IPS_FIXED[2] == "anotherserversansport"); + BEAST_EXPECT(cfg.IPS_FIXED[3] == "anotherserverwithport 12"); + BEAST_EXPECT(cfg.IPS_FIXED[4] == "1.1.1.1 1"); + BEAST_EXPECT(cfg.IPS_FIXED[5] == "1.1.1.1 1"); + BEAST_EXPECT(cfg.IPS_FIXED[6] == "12.34.12.123 12345"); + BEAST_EXPECT(cfg.IPS_FIXED[7] == "12.34.12.123 12345"); + + // all ipv6 should be ignored by colon replacer, howsoever formated + BEAST_EXPECT(cfg.IPS_FIXED[8] == "::"); + BEAST_EXPECT(cfg.IPS_FIXED[9] == "2001:db8::"); + BEAST_EXPECT(cfg.IPS_FIXED[10] == "::1"); + BEAST_EXPECT(cfg.IPS_FIXED[11] == "::1:12345"); + BEAST_EXPECT(cfg.IPS_FIXED[12] == "[::1]:12345"); + BEAST_EXPECT( + cfg.IPS_FIXED[13] == + "2001:db8:3333:4444:5555:6666:7777:8888:12345"); + BEAST_EXPECT( + cfg.IPS_FIXED[14] == "[2001:db8:3333:4444:5555:6666:7777:8888]:1"); + } + void testComments() { @@ -1147,6 +1225,7 @@ r.ripple.com 51235 testSetup(true); testPort(); testWhitespace(); + testColons(); testComments(); testGetters(); testAmendment(); From 9309b573648c8f0c3f0e556d17385ed3fcf69c5f Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Tue, 14 Mar 2023 21:10:56 -0700 Subject: [PATCH 4/4] Rectify the import paths of boost/iterator: (#4293) - MSVC 19.x reported a warning about import paths in boost for function_output_iterator class (boost::function_output_iterator). - Eliminate that warning by updating the import paths, as suggested by the compiler warnings. --- src/ripple/overlay/impl/ProtocolVersion.cpp | 2 +- src/test/csf/Tx.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ripple/overlay/impl/ProtocolVersion.cpp b/src/ripple/overlay/impl/ProtocolVersion.cpp index 9a549b56309..fbd48474420 100644 --- a/src/ripple/overlay/impl/ProtocolVersion.cpp +++ b/src/ripple/overlay/impl/ProtocolVersion.cpp @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/test/csf/Tx.h b/src/test/csf/Tx.h index e65897ffa89..5ccd910b80d 100644 --- a/src/test/csf/Tx.h +++ b/src/test/csf/Tx.h @@ -21,7 +21,7 @@ #include #include #include -#include +#include #include #include #include