From ba1da15b44ec88f93caff2fdfef2a853b8502a07 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 24 Apr 2023 12:25:07 -0700 Subject: [PATCH 01/22] Migrated boost::string_view to std::string_view If merged, this commit will modify the existing usage of boost::string_view into std::string_view. Due to differences in API between boost and std (existence of a clear() member function), I have resorted to abusing the swap() function. Type requirements of jss::<> responses have mandated expensive std::string conversions. --- src/ripple/app/misc/ValidatorList.h | 2 +- src/ripple/app/misc/impl/ValidatorList.cpp | 2 +- src/ripple/basics/StringUtilities.h | 2 +- src/ripple/overlay/impl/OverlayImpl.cpp | 5 ++-- src/ripple/resource/ResourceManager.h | 2 +- src/ripple/resource/impl/ResourceManager.cpp | 4 +-- src/ripple/rpc/Context.h | 4 +-- src/ripple/rpc/Role.h | 8 ++--- src/ripple/rpc/handlers/Ping.cpp | 7 +++-- src/ripple/rpc/impl/Role.cpp | 31 ++++++++++---------- src/ripple/rpc/impl/ServerHandlerImp.cpp | 17 +++++++---- src/ripple/rpc/impl/ServerHandlerImp.h | 4 +-- src/ripple/rpc/impl/WSInfoSub.h | 4 +-- 13 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/ripple/app/misc/ValidatorList.h b/src/ripple/app/misc/ValidatorList.h index 0d7605fc09d..a2c1dc4c20d 100644 --- a/src/ripple/app/misc/ValidatorList.h +++ b/src/ripple/app/misc/ValidatorList.h @@ -622,7 +622,7 @@ class ValidatorList */ std::optional getAvailable( - boost::beast::string_view const& pubKey, + std::string_view pubKey, std::optional forceVersion = {}); /** Return the number of configured validator list sites. */ diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index d17b85c4840..673dcec1db7 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -1676,7 +1676,7 @@ ValidatorList::for_each_available( std::optional ValidatorList::getAvailable( - boost::beast::string_view const& pubKey, + std::string_view pubKey, std::optional forceVersion /* = {} */) { std::shared_lock read_lock{mutex_}; diff --git a/src/ripple/basics/StringUtilities.h b/src/ripple/basics/StringUtilities.h index 48de772ca41..ea633bdf2a2 100644 --- a/src/ripple/basics/StringUtilities.h +++ b/src/ripple/basics/StringUtilities.h @@ -108,7 +108,7 @@ strUnHex(std::string const& strSrc) } inline std::optional -strViewUnHex(boost::string_view const& strSrc) +strViewUnHex(std::string_view strSrc) { return strUnHex(strSrc.size(), strSrc.cbegin(), strSrc.cend()); } diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 6ed046f0403..209ce258935 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -997,9 +997,10 @@ OverlayImpl::processValidatorList( return true; }; - auto key = req.target().substr(prefix.size()); + // Explicitly construct a std::string_view until http_request_type is migrated from boost + std::string_view key{req.target().substr(prefix.size()).data(), req.target().substr(prefix.size()).length()}; - if (auto slash = key.find('/'); slash != boost::string_view::npos) + if (auto slash = key.find('/'); slash != std::string_view::npos) { auto verString = key.substr(0, slash); if (!boost::conversion::try_lexical_convert(verString, version)) diff --git a/src/ripple/resource/ResourceManager.h b/src/ripple/resource/ResourceManager.h index 7471a37ab35..10e20a82f06 100644 --- a/src/ripple/resource/ResourceManager.h +++ b/src/ripple/resource/ResourceManager.h @@ -49,7 +49,7 @@ class Manager : public beast::PropertyStream::Source newInboundEndpoint( beast::IP::Endpoint const& address, bool const proxy, - boost::string_view const& forwardedFor) = 0; + std::string_view forwardedFor) = 0; /** Create a new endpoint keyed by outbound IP address and port. */ virtual Consumer diff --git a/src/ripple/resource/impl/ResourceManager.cpp b/src/ripple/resource/impl/ResourceManager.cpp index 1a7e74ec1f8..ec50d3705b8 100644 --- a/src/ripple/resource/impl/ResourceManager.cpp +++ b/src/ripple/resource/impl/ResourceManager.cpp @@ -77,14 +77,14 @@ class ManagerImp : public Manager newInboundEndpoint( beast::IP::Endpoint const& address, bool const proxy, - boost::string_view const& forwardedFor) override + std::string_view forwardedFor) override { if (!proxy) return newInboundEndpoint(address); boost::system::error_code ec; auto const proxiedIp = - boost::asio::ip::make_address(forwardedFor.to_string(), ec); + boost::asio::ip::make_address(forwardedFor, ec); if (ec) { journal_.warn() diff --git a/src/ripple/rpc/Context.h b/src/ripple/rpc/Context.h index 7a22ed9fe0c..ea18efcec6c 100644 --- a/src/ripple/rpc/Context.h +++ b/src/ripple/rpc/Context.h @@ -57,8 +57,8 @@ struct JsonContext : public Context */ struct Headers { - boost::string_view user; - boost::string_view forwardedFor; + std::string_view user; + std::string_view forwardedFor; }; Json::Value params; diff --git a/src/ripple/rpc/Role.h b/src/ripple/rpc/Role.h index c4f1f730c02..1ce76f90d6d 100644 --- a/src/ripple/rpc/Role.h +++ b/src/ripple/rpc/Role.h @@ -56,15 +56,15 @@ requestRole( Port const& port, Json::Value const& params, beast::IP::Endpoint const& remoteIp, - boost::string_view const& user); + std::string_view user); Resource::Consumer requestInboundEndpoint( Resource::Manager& manager, beast::IP::Endpoint const& remoteAddress, Role const& role, - boost::string_view const& user, - boost::string_view const& forwardedFor); + std::string_view user, + std::string_view forwardedFor); /** * Check if the role entitles the user to unlimited resources. @@ -85,7 +85,7 @@ ipAllowed( std::vector const& nets4, std::vector const& nets6); -boost::string_view +std::string_view forwardedFor(http_request_type const& request); } // namespace ripple diff --git a/src/ripple/rpc/handlers/Ping.cpp b/src/ripple/rpc/handlers/Ping.cpp index 7bd91d8edc1..e118389b3fd 100644 --- a/src/ripple/rpc/handlers/Ping.cpp +++ b/src/ripple/rpc/handlers/Ping.cpp @@ -39,13 +39,14 @@ doPing(RPC::JsonContext& context) break; case Role::IDENTIFIED: ret[jss::role] = "identified"; - ret[jss::username] = context.headers.user.to_string(); + // Can I avoid making expensive copies for std::string? + ret[jss::username] = std::string{context.headers.user}; if (context.headers.forwardedFor.size()) - ret[jss::ip] = context.headers.forwardedFor.to_string(); + ret[jss::ip] = std::string{context.headers.forwardedFor}; break; case Role::PROXY: ret[jss::role] = "proxied"; - ret[jss::ip] = context.headers.forwardedFor.to_string(); + ret[jss::ip] = std::string{context.headers.forwardedFor}; default:; } diff --git a/src/ripple/rpc/impl/Role.cpp b/src/ripple/rpc/impl/Role.cpp index 1807ecc20f6..eecea5e1a46 100644 --- a/src/ripple/rpc/impl/Role.cpp +++ b/src/ripple/rpc/impl/Role.cpp @@ -96,7 +96,7 @@ requestRole( Port const& port, Json::Value const& params, beast::IP::Endpoint const& remoteIp, - boost::string_view const& user) + std::string_view user) { if (isAdmin(port, params, remoteIp.address())) return Role::ADMIN; @@ -142,8 +142,8 @@ requestInboundEndpoint( Resource::Manager& manager, beast::IP::Endpoint const& remoteAddress, Role const& role, - boost::string_view const& user, - boost::string_view const& forwardedFor) + std::string_view user, + std::string_view forwardedFor) { if (isUnlimited(role)) return manager.newUnlimitedEndpoint(remoteAddress); @@ -152,18 +152,18 @@ requestInboundEndpoint( remoteAddress, role == Role::PROXY, forwardedFor); } -static boost::string_view -extractIpAddrFromField(boost::string_view field) +static std::string_view +extractIpAddrFromField(std::string_view field) { // Lambda to trim leading and trailing spaces on the field. - auto trim = [](boost::string_view str) -> boost::string_view { - boost::string_view ret = str; + auto trim = [](std::string_view str) -> std::string_view { + std::string_view ret = str; // Only do the work if there's at least one leading space. if (!ret.empty() && ret.front() == ' ') { std::size_t const firstNonSpace = ret.find_first_not_of(' '); - if (firstNonSpace == boost::string_view::npos) + if (firstNonSpace == std::string_view::npos) // We know there's at least one leading space. So if we got // npos, then it must be all spaces. Return empty string_view. return {}; @@ -178,7 +178,7 @@ extractIpAddrFromField(boost::string_view field) c == ' ' || c == '\r' || c == '\n') { std::size_t const lastNonSpace = ret.find_last_not_of(" \r\n"); - if (lastNonSpace == boost::string_view::npos) + if (lastNonSpace == std::string_view::npos) // We know there's at least one leading space. So if we // got npos, then it must be all spaces. return {}; @@ -189,7 +189,7 @@ extractIpAddrFromField(boost::string_view field) return ret; }; - boost::string_view ret = trim(field); + std::string_view ret = trim(field); if (ret.empty()) return {}; @@ -251,13 +251,13 @@ extractIpAddrFromField(boost::string_view field) // If there's a port appended to the IP address, strip that by // terminating at the colon. - if (std::size_t colon = ret.find(':'); colon != boost::string_view::npos) + if (std::size_t colon = ret.find(':'); colon != std::string_view::npos) ret = ret.substr(0, colon); return ret; } -boost::string_view +std::string_view forwardedFor(http_request_type const& request) { // Look for the Forwarded field in the request. @@ -287,9 +287,9 @@ forwardedFor(http_request_type const& request) // We found a "for=". Scan for the end of the IP address. std::size_t const pos = [&found, &it]() { std::size_t pos = - boost::string_view(found, it->value().end() - found) + std::string_view(found, it->value().end() - found) .find_first_of(",;"); - if (pos != boost::string_view::npos) + if (pos != std::string_view::npos) return pos; return it->value().size() - forStr.size(); @@ -305,7 +305,8 @@ forwardedFor(http_request_type const& request) std::size_t found = it->value().find(','); if (found == boost::string_view::npos) found = it->value().length(); - return extractIpAddrFromField(it->value().substr(0, found)); + // http_request_type has a dependency on boost. Explicitly convert such boost::string_view into std::string_view + return extractIpAddrFromField(std::string_view{it->value().substr(0, found).data(), found}); } return {}; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index f269283b83a..024e5bcd9fa 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -559,8 +559,8 @@ ServerHandlerImp::processSession( [&] { auto const iter = session->request().find("X-User"); if (iter != session->request().end()) - return iter->value(); - return boost::beast::string_view{}; + return std::string_view{iter->value().data(), iter->value().length()}; + return std::string_view{}; }()); if (beast::rfc2616::is_keep_alive(session->request())) @@ -592,8 +592,8 @@ ServerHandlerImp::processRequest( beast::IP::Endpoint const& remoteIPAddress, Output&& output, std::shared_ptr coro, - boost::string_view forwardedFor, - boost::string_view user) + std::string_view forwardedFor, + std::string_view user) { auto rpcJ = app_.journal("RPC"); @@ -847,8 +847,13 @@ ServerHandlerImp::processRequest( */ if (role != Role::IDENTIFIED && role != Role::PROXY) { - forwardedFor.clear(); - user.clear(); + // Simulate the clearing of string_view through a swap. Is this a good candidate for std::string_view? + // This is due to a discrepency between boost::string_view versus std::string_view's APIs +// forwardedFor.clear(); + std::string_view empty_str; + forwardedFor.swap(empty_str); +// user.clear(); + user.swap(empty_str); } JLOG(m_journal.debug()) << "Query: " << strMethod << params; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.h b/src/ripple/rpc/impl/ServerHandlerImp.h index 7c0bf9c9ae5..254f4330268 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.h +++ b/src/ripple/rpc/impl/ServerHandlerImp.h @@ -189,8 +189,8 @@ class ServerHandlerImp beast::IP::Endpoint const& remoteIPAddress, Output&&, std::shared_ptr coro, - boost::string_view forwardedFor, - boost::string_view user); + std::string_view forwardedFor, + std::string_view user); Handoff statusResponse(http_request_type const& request) const; diff --git a/src/ripple/rpc/impl/WSInfoSub.h b/src/ripple/rpc/impl/WSInfoSub.h index 8f386c8ebf9..68c1ad688f4 100644 --- a/src/ripple/rpc/impl/WSInfoSub.h +++ b/src/ripple/rpc/impl/WSInfoSub.h @@ -55,13 +55,13 @@ class WSInfoSub : public InfoSub } } - boost::string_view + std::string_view user() const { return user_; } - boost::string_view + std::string_view forwarded_for() const { return fwdfor_; From 385009376fc2e09a08098126acb6bb1c329ebad4 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 24 Apr 2023 13:22:01 -0700 Subject: [PATCH 02/22] apply git-clang-format patch file from Github Actions --- src/ripple/overlay/impl/OverlayImpl.cpp | 7 +++++-- src/ripple/resource/impl/ResourceManager.cpp | 3 +-- src/ripple/rpc/impl/Role.cpp | 11 ++++++----- src/ripple/rpc/impl/ServerHandlerImp.cpp | 12 +++++++----- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 209ce258935..9d79cfe951e 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -997,8 +997,11 @@ OverlayImpl::processValidatorList( return true; }; - // Explicitly construct a std::string_view until http_request_type is migrated from boost - std::string_view key{req.target().substr(prefix.size()).data(), req.target().substr(prefix.size()).length()}; + // Explicitly construct a std::string_view until http_request_type is + // migrated from boost + std::string_view key{ + req.target().substr(prefix.size()).data(), + req.target().substr(prefix.size()).length()}; if (auto slash = key.find('/'); slash != std::string_view::npos) { diff --git a/src/ripple/resource/impl/ResourceManager.cpp b/src/ripple/resource/impl/ResourceManager.cpp index ec50d3705b8..ae4eaac23fd 100644 --- a/src/ripple/resource/impl/ResourceManager.cpp +++ b/src/ripple/resource/impl/ResourceManager.cpp @@ -83,8 +83,7 @@ class ManagerImp : public Manager return newInboundEndpoint(address); boost::system::error_code ec; - auto const proxiedIp = - boost::asio::ip::make_address(forwardedFor, ec); + auto const proxiedIp = boost::asio::ip::make_address(forwardedFor, ec); if (ec) { journal_.warn() diff --git a/src/ripple/rpc/impl/Role.cpp b/src/ripple/rpc/impl/Role.cpp index eecea5e1a46..36ed03f7b5f 100644 --- a/src/ripple/rpc/impl/Role.cpp +++ b/src/ripple/rpc/impl/Role.cpp @@ -286,9 +286,8 @@ forwardedFor(http_request_type const& request) // We found a "for=". Scan for the end of the IP address. std::size_t const pos = [&found, &it]() { - std::size_t pos = - std::string_view(found, it->value().end() - found) - .find_first_of(",;"); + std::size_t pos = std::string_view(found, it->value().end() - found) + .find_first_of(",;"); if (pos != std::string_view::npos) return pos; @@ -305,8 +304,10 @@ forwardedFor(http_request_type const& request) std::size_t found = it->value().find(','); if (found == boost::string_view::npos) found = it->value().length(); - // http_request_type has a dependency on boost. Explicitly convert such boost::string_view into std::string_view - return extractIpAddrFromField(std::string_view{it->value().substr(0, found).data(), found}); + // http_request_type has a dependency on boost. Explicitly convert such + // boost::string_view into std::string_view + return extractIpAddrFromField( + std::string_view{it->value().substr(0, found).data(), found}); } return {}; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 024e5bcd9fa..f774df0b32f 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -559,7 +559,8 @@ ServerHandlerImp::processSession( [&] { auto const iter = session->request().find("X-User"); if (iter != session->request().end()) - return std::string_view{iter->value().data(), iter->value().length()}; + return std::string_view{ + iter->value().data(), iter->value().length()}; return std::string_view{}; }()); @@ -847,12 +848,13 @@ ServerHandlerImp::processRequest( */ if (role != Role::IDENTIFIED && role != Role::PROXY) { - // Simulate the clearing of string_view through a swap. Is this a good candidate for std::string_view? - // This is due to a discrepency between boost::string_view versus std::string_view's APIs -// forwardedFor.clear(); + // Simulate the clearing of string_view through a swap. Is this a + // good candidate for std::string_view? This is due to a discrepency + // between boost::string_view versus std::string_view's APIs + // forwardedFor.clear(); std::string_view empty_str; forwardedFor.swap(empty_str); -// user.clear(); + // user.clear(); user.swap(empty_str); } From 1141e0f67e2e26ac61df3810f65359a807f66908 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 24 Apr 2023 15:07:51 -0700 Subject: [PATCH 03/22] use std::string_view.remove_suffix() member function to clear the value of user, forwardedFor vars --- src/ripple/rpc/impl/ServerHandlerImp.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index f774df0b32f..512201b1753 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -848,14 +848,8 @@ ServerHandlerImp::processRequest( */ if (role != Role::IDENTIFIED && role != Role::PROXY) { - // Simulate the clearing of string_view through a swap. Is this a - // good candidate for std::string_view? This is due to a discrepency - // between boost::string_view versus std::string_view's APIs - // forwardedFor.clear(); - std::string_view empty_str; - forwardedFor.swap(empty_str); - // user.clear(); - user.swap(empty_str); + forwardedFor.remove_suffix(forwardedFor.size()); + user.remove_suffix(user.size()); } JLOG(m_journal.debug()) << "Query: " << strMethod << params; From c966149aeb0daa028cd23c481c0215a15cca68f8 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Fri, 23 Jun 2023 14:41:57 -0700 Subject: [PATCH 04/22] [FOLD] address PR comments from Scott S and Nik --- src/ripple/overlay/impl/OverlayImpl.cpp | 11 ++++++----- src/ripple/rpc/impl/Role.cpp | 2 +- src/ripple/rpc/impl/ServerHandlerImp.cpp | 7 +++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 9d79cfe951e..aca9dc18ccd 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -997,11 +997,12 @@ OverlayImpl::processValidatorList( return true; }; - // Explicitly construct a std::string_view until http_request_type is - // migrated from boost - std::string_view key{ - req.target().substr(prefix.size()).data(), - req.target().substr(prefix.size()).length()}; + // Convert the boost::string_view, returned by + // boost::http::request::target(), into a std::string_view. + std::string_view key = [&req, &prefix]() { + boost::string_view const key = req.target().substr(prefix.size()); + return std::string_view(key.data(), key.length()); + }(); if (auto slash = key.find('/'); slash != std::string_view::npos) { diff --git a/src/ripple/rpc/impl/Role.cpp b/src/ripple/rpc/impl/Role.cpp index 36ed03f7b5f..200c45348c2 100644 --- a/src/ripple/rpc/impl/Role.cpp +++ b/src/ripple/rpc/impl/Role.cpp @@ -307,7 +307,7 @@ forwardedFor(http_request_type const& request) // http_request_type has a dependency on boost. Explicitly convert such // boost::string_view into std::string_view return extractIpAddrFromField( - std::string_view{it->value().substr(0, found).data(), found}); + std::string_view{it->value().data(), found}); } return {}; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 512201b1753..263c1635d95 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -556,12 +556,11 @@ ServerHandlerImp::processSession( makeOutput(*session), coro, forwardedFor(session->request()), - [&] { + [&session]() -> std::string_view { auto const iter = session->request().find("X-User"); if (iter != session->request().end()) - return std::string_view{ - iter->value().data(), iter->value().length()}; - return std::string_view{}; + return {iter->value().data(), iter->value().length()}; + return {}; }()); if (beast::rfc2616::is_keep_alive(session->request())) From efaf7ae6eb62df4f8f1c8fc99d28fc0822ea1e71 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 11 Sep 2023 15:47:02 -0700 Subject: [PATCH 05/22] replace usages of boost::string_view with std::string_view. In turn, this necessitates conversions to std::string wherever non-temporary uses of string_view is required compile the code with additional flags set in the conan profile. Need to update the build instructions appropriately --- src/ripple/json/Output.h | 1 + src/ripple/overlay/impl/Handshake.cpp | 25 ++++++++++++++---------- src/ripple/overlay/impl/OverlayImpl.cpp | 10 ++++++---- src/ripple/overlay/impl/PeerImp.cpp | 14 ++++++------- src/ripple/rpc/impl/ServerHandlerImp.cpp | 6 ++++-- src/ripple/rpc/impl/WSInfoSub.h | 2 +- src/test/jtx/TrustedPublisherServer.h | 5 +++-- 7 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/ripple/json/Output.h b/src/ripple/json/Output.h index 74aaa65269d..96905c20ba9 100644 --- a/src/ripple/json/Output.h +++ b/src/ripple/json/Output.h @@ -22,6 +22,7 @@ #include #include +#include namespace Json { diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index 9fe025787f8..ea543ec20aa 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -44,8 +44,9 @@ getFeatureValue( return {}; boost::smatch match; boost::regex rx(feature + "=([^;\\s]+)"); - auto const value = header->value().to_string(); - if (boost::regex_search(value, match, rx)) + const std::string allFeatures = std::string{header->value()}; + if (boost::regex_search(allFeatures, + match, rx)) return {match[1]}; return {}; } @@ -233,7 +234,7 @@ verifyHandshake( { if (auto const iter = headers.find("Server-Domain"); iter != headers.end()) { - if (!isProperlyFormedTomlDomain(iter->value().to_string())) + if (!isProperlyFormedTomlDomain(std::string{iter->value()})) throw std::runtime_error("Invalid server domain"); } @@ -241,7 +242,7 @@ verifyHandshake( { std::uint32_t nid; - if (!beast::lexicalCastChecked(nid, iter->value().to_string())) + if (!beast::lexicalCastChecked(nid, std::string{iter->value()})) throw std::runtime_error("Invalid peer network identifier"); if (networkID && nid != *networkID) @@ -251,10 +252,14 @@ verifyHandshake( if (auto const iter = headers.find("Network-Time"); iter != headers.end()) { auto const netTime = - [str = iter->value().to_string()]() -> TimeKeeper::time_point { + [str = iter->value()]() -> TimeKeeper::time_point { TimeKeeper::duration::rep val; - if (beast::lexicalCastChecked(val, str)) + // boost::LexicalCast does not have a specialization for . It's not obvious if such an instantiation + // is technically feasible. This necessitates the conversion + // from std::string_view to std::string + if (beast::lexicalCastChecked(val, std::string{str})) return TimeKeeper::time_point{TimeKeeper::duration{val}}; // It's not an error for the header field to not be present but if @@ -287,7 +292,7 @@ verifyHandshake( if (auto const iter = headers.find("Public-Key"); iter != headers.end()) { auto pk = parseBase58( - TokenType::NodePublic, iter->value().to_string()); + TokenType::NodePublic, std::string{iter->value()}); if (pk) { @@ -313,7 +318,7 @@ verifyHandshake( if (iter == headers.end()) throw std::runtime_error("No session signature specified"); - auto sig = base64_decode(iter->value().to_string()); + auto sig = base64_decode(std::string{iter->value()}); if (!verifyDigest(publicKey, sharedValue, makeSlice(sig), false)) throw std::runtime_error("Failed to verify session"); @@ -326,7 +331,7 @@ verifyHandshake( { boost::system::error_code ec; auto const local_ip = boost::asio::ip::address::from_string( - iter->value().to_string(), ec); + std::string{iter->value()}, ec); if (ec) throw std::runtime_error("Invalid Local-IP"); @@ -341,7 +346,7 @@ verifyHandshake( { boost::system::error_code ec; auto const remote_ip = boost::asio::ip::address::from_string( - iter->value().to_string(), ec); + std::string{iter->value()}, ec); if (ec) throw std::runtime_error("Invalid Remote-IP"); diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index aca9dc18ccd..d61373f56e6 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -999,10 +999,12 @@ OverlayImpl::processValidatorList( // Convert the boost::string_view, returned by // boost::http::request::target(), into a std::string_view. - std::string_view key = [&req, &prefix]() { - boost::string_view const key = req.target().substr(prefix.size()); - return std::string_view(key.data(), key.length()); - }(); +// std::string_view key = [&req, &prefix]() { +// std::string_view const key = req.target().substr(prefix.size()); +// return key; +// }(); + + std::string_view key = req.target().substr(prefix.size()); if (auto slash = key.find('/'); slash != std::string_view::npos) { diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index a07c457458c..6fe13757deb 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -176,7 +176,7 @@ PeerImp::run() if (auto const iter = headers_.find("Closed-Ledger"); iter != headers_.end()) { - closed = parseLedgerHash(iter->value().to_string()); + closed = parseLedgerHash(std::string{iter->value()}); if (!closed) fail("Malformed handshake data (1)"); @@ -185,7 +185,7 @@ PeerImp::run() if (auto const iter = headers_.find("Previous-Ledger"); iter != headers_.end()) { - previous = parseLedgerHash(iter->value().to_string()); + previous = parseLedgerHash(std::string{iter->value()}); if (!previous) fail("Malformed handshake data (2)"); @@ -372,8 +372,8 @@ std::string PeerImp::getVersion() const { if (inbound_) - return headers_["User-Agent"].to_string(); - return headers_["Server"].to_string(); + return std::string{headers_["User-Agent"]}; + return std::string{headers_["Server"]}; } Json::Value @@ -399,8 +399,8 @@ PeerImp::json() if (auto const d = domain(); !d.empty()) ret[jss::server_domain] = domain(); - if (auto const nid = headers_["Network-ID"].to_string(); !nid.empty()) - ret[jss::network_id] = nid; + if (auto const nid = headers_["Network-ID"]; !nid.empty()) + ret[jss::network_id] = std::string{nid}; ret[jss::load] = usage_.balance(); @@ -839,7 +839,7 @@ PeerImp::name() const std::string PeerImp::domain() const { - return headers_["Server-Domain"].to_string(); + return std::string{headers_["Server-Domain"]}; } //------------------------------------------------------------------------------ diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 263c1635d95..dfaa28b72b6 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -247,11 +247,13 @@ build_map(boost::beast::http::fields const& h) std::map c; for (auto const& e : h) { - auto key(e.name_string().to_string()); + // key cannot be a std::string_view because it needs to be used in + // map and along with iterators + std::string key(e.name_string()); std::transform(key.begin(), key.end(), key.begin(), [](auto kc) { return std::tolower(static_cast(kc)); }); - c[key] = e.value().to_string(); + c[key] = e.value(); } return c; } diff --git a/src/ripple/rpc/impl/WSInfoSub.h b/src/ripple/rpc/impl/WSInfoSub.h index 68c1ad688f4..31f6353b994 100644 --- a/src/ripple/rpc/impl/WSInfoSub.h +++ b/src/ripple/rpc/impl/WSInfoSub.h @@ -50,7 +50,7 @@ class WSInfoSub : public InfoSub { auto it = h.find("X-User"); if (it != h.end()) - user_ = it->value().to_string(); + user_ = it->value(); fwdfor_ = std::string(forwardedFor(h)); } } diff --git a/src/test/jtx/TrustedPublisherServer.h b/src/test/jtx/TrustedPublisherServer.h index 985c1bfd6c0..13388fd14ef 100644 --- a/src/test/jtx/TrustedPublisherServer.h +++ b/src/test/jtx/TrustedPublisherServer.h @@ -574,7 +574,7 @@ xbEQ+TUZ5jbJGSeBqNFKFeuOUQGJ46Io0jBSYd4rSmKUXkvElQwR+n7KF3jy1uAt if (ec) break; - auto path = req.target().to_string(); + std::string_view const path = req.target(); res.insert("Server", "TrustedPublisherServer"); res.version(req.version()); res.keep_alive(req.keep_alive()); @@ -677,7 +677,8 @@ xbEQ+TUZ5jbJGSeBqNFKFeuOUQGJ46Io0jBSYd4rSmKUXkvElQwR+n7KF3jy1uAt // unknown request res.result(boost::beast::http::status::not_found); res.insert("Content-Type", "text/html"); - res.body() = "The file '" + path + "' was not found"; + res.body() = "The file '" + std::string(path) + "' was not " + "found"; } if (prepare) From 8c17a5c86407023785c4321071b45d0ee92a0750 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 11 Sep 2023 16:04:19 -0700 Subject: [PATCH 06/22] clang format --- src/ripple/overlay/impl/Handshake.cpp | 6 ++---- src/ripple/overlay/impl/OverlayImpl.cpp | 8 ++++---- src/test/jtx/TrustedPublisherServer.h | 3 ++- src/test/rpc/JSONRPC_test.cpp | 2 +- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index ea543ec20aa..d40a74ccfc7 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -45,8 +45,7 @@ getFeatureValue( boost::smatch match; boost::regex rx(feature + "=([^;\\s]+)"); const std::string allFeatures = std::string{header->value()}; - if (boost::regex_search(allFeatures, - match, rx)) + if (boost::regex_search(allFeatures, match, rx)) return {match[1]}; return {}; } @@ -251,8 +250,7 @@ verifyHandshake( if (auto const iter = headers.find("Network-Time"); iter != headers.end()) { - auto const netTime = - [str = iter->value()]() -> TimeKeeper::time_point { + auto const netTime = [str = iter->value()]() -> TimeKeeper::time_point { TimeKeeper::duration::rep val; // boost::LexicalCast does not have a specialization for Date: Mon, 11 Sep 2023 16:29:24 -0700 Subject: [PATCH 07/22] fix compiler errors due to the ServerHandlerImp -> ServerHandler change --- src/ripple/rpc/ServerHandler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ripple/rpc/ServerHandler.h b/src/ripple/rpc/ServerHandler.h index 07fb61362a0..7e9e427cfdb 100644 --- a/src/ripple/rpc/ServerHandler.h +++ b/src/ripple/rpc/ServerHandler.h @@ -208,8 +208,8 @@ class ServerHandler beast::IP::Endpoint const& remoteIPAddress, Output&&, std::shared_ptr coro, - boost::string_view forwardedFor, - boost::string_view user); + std::string_view forwardedFor, + std::string_view user); Handoff statusResponse(http_request_type const& request) const; From e4015fda1023b7b5dada8b5e1905fb282494b6f9 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Fri, 5 Jan 2024 14:12:07 -0800 Subject: [PATCH 08/22] Update src/ripple/overlay/impl/Handshake.cpp Co-authored-by: John Freeman --- src/ripple/overlay/impl/Handshake.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index d40a74ccfc7..7003855bcd5 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -44,7 +44,7 @@ getFeatureValue( return {}; boost::smatch match; boost::regex rx(feature + "=([^;\\s]+)"); - const std::string allFeatures = std::string{header->value()}; + std::string const allFeatures = std::string{header->value()}; if (boost::regex_search(allFeatures, match, rx)) return {match[1]}; return {}; From be6e9bef0a43c0a49b755b168abb51e46bdec8f3 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Fri, 5 Jan 2024 16:54:09 -0800 Subject: [PATCH 09/22] removed old comment and an irrelevant file --- src/ripple/rpc/handlers/Ping.cpp | 1 - src/ripple/rpc/impl/ServerHandlerImp.h | 0 2 files changed, 1 deletion(-) delete mode 100644 src/ripple/rpc/impl/ServerHandlerImp.h diff --git a/src/ripple/rpc/handlers/Ping.cpp b/src/ripple/rpc/handlers/Ping.cpp index e118389b3fd..efe9063a1e2 100644 --- a/src/ripple/rpc/handlers/Ping.cpp +++ b/src/ripple/rpc/handlers/Ping.cpp @@ -39,7 +39,6 @@ doPing(RPC::JsonContext& context) break; case Role::IDENTIFIED: ret[jss::role] = "identified"; - // Can I avoid making expensive copies for std::string? ret[jss::username] = std::string{context.headers.user}; if (context.headers.forwardedFor.size()) ret[jss::ip] = std::string{context.headers.forwardedFor}; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.h b/src/ripple/rpc/impl/ServerHandlerImp.h deleted file mode 100644 index e69de29bb2d..00000000000 From 9d9f40e6cb0f5e1342fe9c5e156258e72b15da6a Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Fri, 5 Jan 2024 16:54:37 -0800 Subject: [PATCH 10/22] partially addressed John's comments: Use std::string_view instead of std::string, wherever possible --- src/ripple/basics/StringUtilities.h | 2 +- src/ripple/basics/impl/StringUtilities.cpp | 4 ++-- src/ripple/overlay/impl/Handshake.cpp | 2 +- src/ripple/overlay/impl/OverlayImpl.cpp | 12 +++++------- src/ripple/overlay/impl/PeerImp.cpp | 14 +++++++------- src/ripple/overlay/impl/PeerImp.h | 4 ++-- 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/ripple/basics/StringUtilities.h b/src/ripple/basics/StringUtilities.h index 8e7bf163f9d..a8cacc681c6 100644 --- a/src/ripple/basics/StringUtilities.h +++ b/src/ripple/basics/StringUtilities.h @@ -150,7 +150,7 @@ to_uint64(std::string const& s); doesn't check whether the TLD is valid. */ bool -isProperlyFormedTomlDomain(std::string const& domain); +isProperlyFormedTomlDomain(std::string_view const& domain); } // namespace ripple diff --git a/src/ripple/basics/impl/StringUtilities.cpp b/src/ripple/basics/impl/StringUtilities.cpp index bebbe1ef80b..c11d1ea55af 100644 --- a/src/ripple/basics/impl/StringUtilities.cpp +++ b/src/ripple/basics/impl/StringUtilities.cpp @@ -120,7 +120,7 @@ to_uint64(std::string const& s) } bool -isProperlyFormedTomlDomain(std::string const& domain) +isProperlyFormedTomlDomain(std::string_view const& domain) { // The domain must be between 4 and 128 characters long if (domain.size() < 4 || domain.size() > 128) @@ -143,7 +143,7 @@ isProperlyFormedTomlDomain(std::string const& domain) , boost::regex_constants::optimize); - return boost::regex_match(domain, re); + return boost::regex_match(domain.begin(), domain.end(), re); } } // namespace ripple diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index d40a74ccfc7..90efd416456 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -233,7 +233,7 @@ verifyHandshake( { if (auto const iter = headers.find("Server-Domain"); iter != headers.end()) { - if (!isProperlyFormedTomlDomain(std::string{iter->value()})) + if (!isProperlyFormedTomlDomain(iter->value())) throw std::runtime_error("Invalid server domain"); } diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 11db5c0acf4..5db61596dc7 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -829,7 +829,7 @@ OverlayImpl::getOverlayInfo() auto version{sp->getVersion()}; if (!version.empty()) // Could move here if Json::value supported moving from strings - pv[jss::version] = version; + pv[jss::version] = std::string{version}; } std::uint32_t minSeq, maxSeq; @@ -999,12 +999,10 @@ OverlayImpl::processValidatorList( // Convert the boost::string_view, returned by // boost::http::request::target(), into a std::string_view. - // std::string_view key = [&req, &prefix]() { - // std::string_view const key = req.target().substr(prefix.size()); - // return key; - // }(); - - std::string_view key = req.target().substr(prefix.size()); + std::string_view key = [&req, &prefix]() { + boost::string_view const key = req.target().substr(prefix.size()); + return std::string_view(key.data(), key.length()); + }(); if (auto slash = key.find('/'); slash != std::string_view::npos) { diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 6fe13757deb..43221551772 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -368,12 +368,12 @@ PeerImp::cluster() const return static_cast(app_.cluster().member(publicKey_)); } -std::string +std::string_view PeerImp::getVersion() const { if (inbound_) - return std::string{headers_["User-Agent"]}; - return std::string{headers_["Server"]}; + return headers_["User-Agent"]; + return headers_["Server"]; } Json::Value @@ -397,7 +397,7 @@ PeerImp::json() } if (auto const d = domain(); !d.empty()) - ret[jss::server_domain] = domain(); + ret[jss::server_domain] = std::string{domain()}; if (auto const nid = headers_["Network-ID"]; !nid.empty()) ret[jss::network_id] = std::string{nid}; @@ -405,7 +405,7 @@ PeerImp::json() ret[jss::load] = usage_.balance(); if (auto const version = getVersion(); !version.empty()) - ret[jss::version] = version; + ret[jss::version] = std::string{version}; ret[jss::protocol] = to_string(protocol_); @@ -836,10 +836,10 @@ PeerImp::name() const return name_; } -std::string +std::string_view PeerImp::domain() const { - return std::string{headers_["Server-Domain"]}; + return headers_["Server-Domain"]; } //------------------------------------------------------------------------------ diff --git a/src/ripple/overlay/impl/PeerImp.h b/src/ripple/overlay/impl/PeerImp.h index 710ab4d74d6..ea8fbb0256e 100644 --- a/src/ripple/overlay/impl/PeerImp.h +++ b/src/ripple/overlay/impl/PeerImp.h @@ -344,7 +344,7 @@ class PeerImp : public Peer, } /** Return the version of rippled that the peer is running, if reported. */ - std::string + std::string_view getVersion() const; // Return the connection elapsed time. @@ -464,7 +464,7 @@ class PeerImp : public Peer, std::string name() const; - std::string + std::string_view domain() const; // From 8da02e53fb5434e9d872c7f8334fb0bd268ead35 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Fri, 5 Jan 2024 21:33:06 -0800 Subject: [PATCH 11/22] introduce template specialization for beast::LexicalCast. Refactor other template specializations to use the new overload instead of using the std::string overload --- src/ripple/beast/core/LexicalCast.h | 51 ++++++++++++++++++++------- src/ripple/overlay/impl/Handshake.cpp | 8 ++--- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/ripple/beast/core/LexicalCast.h b/src/ripple/beast/core/LexicalCast.h index f4c78341b91..6efe280894b 100644 --- a/src/ripple/beast/core/LexicalCast.h +++ b/src/ripple/beast/core/LexicalCast.h @@ -64,9 +64,9 @@ struct LexicalCast } }; -// Parse std::string to number -template -struct LexicalCast +// Parse a std::string_view into a number +template +struct LexicalCast { explicit LexicalCast() = default; @@ -78,8 +78,12 @@ struct LexicalCast std::enable_if_t< std::is_integral_v && !std::is_same_v, bool> - operator()(Integral& out, std::string const& in) const + operator()(Integral& out, std::string_view const& in) const { + // the underlying data of the std::string_view is no longer alive + if (in.empty()) + return false; + auto first = in.data(); auto last = in.data() + in.size(); @@ -92,20 +96,27 @@ struct LexicalCast } bool - operator()(bool& out, std::string in) const + operator()(bool& out, std::string_view const& in) const { + // the underlying data of the std::string_view is no longer alive + if (in.empty()) + return false; + + std::string result; + // Convert the input to lowercase - std::transform(in.begin(), in.end(), in.begin(), [](auto c) { - return std::tolower(static_cast(c)); - }); + std::transform( + in.begin(), in.end(), std::back_inserter(result), [](auto c) { + return std::tolower(static_cast(c)); + }); - if (in == "1" || in == "true") + if (result == "1" || result == "true") { out = true; return true; } - if (in == "0" || in == "false") + if (result == "0" || result == "false") { out = false; return true; @@ -114,9 +125,21 @@ struct LexicalCast return false; } }; - //------------------------------------------------------------------------------ +// Parse std::string to number or boolean value +template +struct LexicalCast +{ + explicit LexicalCast() = default; + + bool + operator()(Out& out, std::string in) const + { + return LexicalCast()(out, in); + } +}; + // Conversion from null terminated char const* template struct LexicalCast @@ -126,7 +149,8 @@ struct LexicalCast bool operator()(Out& out, char const* in) const { - return LexicalCast()(out, in); + assert(in != 0); + return LexicalCast()(out, in); } }; @@ -140,7 +164,8 @@ struct LexicalCast bool operator()(Out& out, char* in) const { - return LexicalCast()(out, in); + assert(in != 0); + return LexicalCast()(out, in); } }; diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index beaa3a62bf0..86217138fb2 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -241,7 +241,7 @@ verifyHandshake( { std::uint32_t nid; - if (!beast::lexicalCastChecked(nid, std::string{iter->value()})) + if (!beast::lexicalCastChecked(nid, std::string_view{iter->value()})) throw std::runtime_error("Invalid peer network identifier"); if (networkID && nid != *networkID) @@ -253,11 +253,7 @@ verifyHandshake( auto const netTime = [str = iter->value()]() -> TimeKeeper::time_point { TimeKeeper::duration::rep val; - // boost::LexicalCast does not have a specialization for . It's not obvious if such an instantiation - // is technically feasible. This necessitates the conversion - // from std::string_view to std::string - if (beast::lexicalCastChecked(val, std::string{str})) + if (beast::lexicalCastChecked(val, std::string_view{str})) return TimeKeeper::time_point{TimeKeeper::duration{val}}; // It's not an error for the header field to not be present but if From a43320773d99ac6bdac07e4d2709709ba6aeadca Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Sat, 6 Jan 2024 07:52:46 -0800 Subject: [PATCH 12/22] refactor PeerImp::parseLedgerHash and base64_decode to use std::string_view, instead of std::string --- src/ripple/basics/base64.h | 2 +- src/ripple/basics/impl/base64.cpp | 2 +- src/ripple/overlay/impl/PeerImp.cpp | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ripple/basics/base64.h b/src/ripple/basics/base64.h index 05a61133f83..77960ec66bc 100644 --- a/src/ripple/basics/base64.h +++ b/src/ripple/basics/base64.h @@ -73,7 +73,7 @@ base64_encode(std::string const& s) } std::string -base64_decode(std::string const& data); +base64_decode(std::string_view const& data); } // namespace ripple diff --git a/src/ripple/basics/impl/base64.cpp b/src/ripple/basics/impl/base64.cpp index 39b615100e5..4e8dcd7dc93 100644 --- a/src/ripple/basics/impl/base64.cpp +++ b/src/ripple/basics/impl/base64.cpp @@ -242,7 +242,7 @@ base64_encode(std::uint8_t const* data, std::size_t len) } std::string -base64_decode(std::string const& data) +base64_decode(std::string_view const& data) { std::string dest; dest.resize(base64::decoded_size(data.size())); diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 43221551772..96fd45fed26 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -160,7 +160,7 @@ PeerImp::run() return post(strand_, std::bind(&PeerImp::run, shared_from_this())); auto parseLedgerHash = - [](std::string const& value) -> std::optional { + [](std::string_view const& value) -> std::optional { if (uint256 ret; ret.parseHex(value)) return ret; @@ -176,7 +176,7 @@ PeerImp::run() if (auto const iter = headers_.find("Closed-Ledger"); iter != headers_.end()) { - closed = parseLedgerHash(std::string{iter->value()}); + closed = parseLedgerHash(std::string_view{iter->value()}); if (!closed) fail("Malformed handshake data (1)"); @@ -185,7 +185,7 @@ PeerImp::run() if (auto const iter = headers_.find("Previous-Ledger"); iter != headers_.end()) { - previous = parseLedgerHash(std::string{iter->value()}); + previous = parseLedgerHash(std::string_view{iter->value()}); if (!previous) fail("Malformed handshake data (2)"); From 2cf0eb896c64d078133f108fc1fe55d79c349391 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Sat, 6 Jan 2024 08:23:47 -0800 Subject: [PATCH 13/22] refactor parseBase58 overloads to use std::string_view instead of std::string. TODO: create a Json::Value::asStringView() overload to return std::string_view. This could reduce one copy of std::string objects across the codebase --- src/ripple/app/misc/impl/ValidatorList.cpp | 3 ++- src/ripple/overlay/impl/Cluster.cpp | 3 ++- src/ripple/overlay/impl/Handshake.cpp | 2 +- src/ripple/protocol/PublicKey.h | 2 +- src/ripple/protocol/SecretKey.h | 2 +- src/ripple/protocol/impl/PublicKey.cpp | 2 +- src/ripple/protocol/impl/SecretKey.cpp | 2 +- src/ripple/protocol/impl/tokens.cpp | 6 +++--- src/ripple/protocol/tokens.h | 4 ++-- 9 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index 673dcec1db7..6ec7f4b0774 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -213,7 +213,8 @@ ValidatorList::load( return false; } - auto const id = parseBase58(TokenType::NodePublic, match[1]); + auto const id = + parseBase58(TokenType::NodePublic, match[1].str()); if (!id) { diff --git a/src/ripple/overlay/impl/Cluster.cpp b/src/ripple/overlay/impl/Cluster.cpp index 53ec13b7403..9441ff7d114 100644 --- a/src/ripple/overlay/impl/Cluster.cpp +++ b/src/ripple/overlay/impl/Cluster.cpp @@ -113,7 +113,8 @@ Cluster::load(Section const& nodes) return false; } - auto const id = parseBase58(TokenType::NodePublic, match[1]); + auto const id = parseBase58( + TokenType::NodePublic, std::string_view{match[1].str()}); if (!id) { diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index 86217138fb2..f30f7000b94 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -286,7 +286,7 @@ verifyHandshake( if (auto const iter = headers.find("Public-Key"); iter != headers.end()) { auto pk = parseBase58( - TokenType::NodePublic, std::string{iter->value()}); + TokenType::NodePublic, std::string_view{iter->value()}); if (pk) { diff --git a/src/ripple/protocol/PublicKey.h b/src/ripple/protocol/PublicKey.h index 58394cd82d4..ace46282731 100644 --- a/src/ripple/protocol/PublicKey.h +++ b/src/ripple/protocol/PublicKey.h @@ -192,7 +192,7 @@ toBase58(TokenType type, PublicKey const& pk) template <> std::optional -parseBase58(TokenType type, std::string const& s); +parseBase58(TokenType type, std::string_view const& s); enum class ECDSACanonicality { canonical, fullyCanonical }; diff --git a/src/ripple/protocol/SecretKey.h b/src/ripple/protocol/SecretKey.h index 3026fb9d775..11d57192b64 100644 --- a/src/ripple/protocol/SecretKey.h +++ b/src/ripple/protocol/SecretKey.h @@ -114,7 +114,7 @@ operator!=(SecretKey const& lhs, SecretKey const& rhs) /** Parse a secret key */ template <> std::optional -parseBase58(TokenType type, std::string const& s); +parseBase58(TokenType type, std::string_view const& s); inline std::string toBase58(TokenType type, SecretKey const& sk) diff --git a/src/ripple/protocol/impl/PublicKey.cpp b/src/ripple/protocol/impl/PublicKey.cpp index 8ab1bd46cdf..18b483df883 100644 --- a/src/ripple/protocol/impl/PublicKey.cpp +++ b/src/ripple/protocol/impl/PublicKey.cpp @@ -37,7 +37,7 @@ operator<<(std::ostream& os, PublicKey const& pk) template <> std::optional -parseBase58(TokenType type, std::string const& s) +parseBase58(TokenType type, std::string_view const& s) { auto const result = decodeBase58Token(s, type); auto const pks = makeSlice(result); diff --git a/src/ripple/protocol/impl/SecretKey.cpp b/src/ripple/protocol/impl/SecretKey.cpp index 63661888f48..bbf077fa8eb 100644 --- a/src/ripple/protocol/impl/SecretKey.cpp +++ b/src/ripple/protocol/impl/SecretKey.cpp @@ -373,7 +373,7 @@ randomKeyPair(KeyType type) template <> std::optional -parseBase58(TokenType type, std::string const& s) +parseBase58(TokenType type, std::string_view const& s) { auto const result = decodeBase58Token(s, type); if (result.empty()) diff --git a/src/ripple/protocol/impl/tokens.cpp b/src/ripple/protocol/impl/tokens.cpp index 816d49e40df..31526738960 100644 --- a/src/ripple/protocol/impl/tokens.cpp +++ b/src/ripple/protocol/impl/tokens.cpp @@ -147,9 +147,9 @@ encodeBase58( } static std::string -decodeBase58(std::string const& s) +decodeBase58(std::string_view const& s) { - auto psz = reinterpret_cast(s.c_str()); + auto psz = reinterpret_cast(s.data()); auto remain = s.size(); // Skip and count leading zeroes int zeroes = 0; @@ -220,7 +220,7 @@ encodeBase58Token(TokenType type, void const* token, std::size_t size) } std::string -decodeBase58Token(std::string const& s, TokenType type) +decodeBase58Token(std::string_view const& s, TokenType type) { std::string const ret = detail::decodeBase58(s); diff --git a/src/ripple/protocol/tokens.h b/src/ripple/protocol/tokens.h index 0afb4509f41..d58e1d19b5b 100644 --- a/src/ripple/protocol/tokens.h +++ b/src/ripple/protocol/tokens.h @@ -43,7 +43,7 @@ parseBase58(std::string const& s); template std::optional -parseBase58(TokenType type, std::string const& s); +parseBase58(TokenType type, std::string_view const& s); /** Encode data in Base58Check format using XRPL alphabet @@ -68,7 +68,7 @@ encodeBase58Token(TokenType type, void const* token, std::size_t size); the type or checksum. And empty string otherwise. */ std::string -decodeBase58Token(std::string const& s, TokenType type); +decodeBase58Token(std::string_view const& s, TokenType type); } // namespace ripple From 85e001dcb21516ad0d711e60af70fc9141f29162 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 8 Jan 2024 08:05:42 -0800 Subject: [PATCH 14/22] address John's comments --- src/ripple/beast/core/LexicalCast.h | 12 ++---------- src/ripple/overlay/impl/Cluster.cpp | 4 ++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/ripple/beast/core/LexicalCast.h b/src/ripple/beast/core/LexicalCast.h index 6efe280894b..215f4ec803b 100644 --- a/src/ripple/beast/core/LexicalCast.h +++ b/src/ripple/beast/core/LexicalCast.h @@ -80,10 +80,6 @@ struct LexicalCast bool> operator()(Integral& out, std::string_view const& in) const { - // the underlying data of the std::string_view is no longer alive - if (in.empty()) - return false; - auto first = in.data(); auto last = in.data() + in.size(); @@ -98,10 +94,6 @@ struct LexicalCast bool operator()(bool& out, std::string_view const& in) const { - // the underlying data of the std::string_view is no longer alive - if (in.empty()) - return false; - std::string result; // Convert the input to lowercase @@ -149,7 +141,7 @@ struct LexicalCast bool operator()(Out& out, char const* in) const { - assert(in != 0); + assert(in); return LexicalCast()(out, in); } }; @@ -164,7 +156,7 @@ struct LexicalCast bool operator()(Out& out, char* in) const { - assert(in != 0); + assert(in); return LexicalCast()(out, in); } }; diff --git a/src/ripple/overlay/impl/Cluster.cpp b/src/ripple/overlay/impl/Cluster.cpp index 9441ff7d114..44954fc6abf 100644 --- a/src/ripple/overlay/impl/Cluster.cpp +++ b/src/ripple/overlay/impl/Cluster.cpp @@ -113,8 +113,8 @@ Cluster::load(Section const& nodes) return false; } - auto const id = parseBase58( - TokenType::NodePublic, std::string_view{match[1].str()}); + auto const id = + parseBase58(TokenType::NodePublic, match[1].str()); if (!id) { From 88412eaa6e86782489f073d724fc0acf9d6e6321 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 8 Jan 2024 10:01:01 -0800 Subject: [PATCH 15/22] include BOOST_BEAST_USE_STD_STRING macro in the build system. Hence, remove the extraneous explicit typecasts from boost:string_view into std::string_view --- src/ripple/overlay/impl/Handshake.cpp | 16 +++++++++------- src/ripple/overlay/impl/OverlayImpl.cpp | 7 +------ src/ripple/overlay/impl/PeerImp.cpp | 6 +++--- src/ripple/rpc/impl/Role.cpp | 5 +---- src/ripple/rpc/impl/ServerHandler.cpp | 6 +++--- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index f30f7000b94..54e1e420919 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -25,7 +25,9 @@ #include #include #include +#include #include +// #include #include #include @@ -285,8 +287,8 @@ verifyHandshake( PublicKey const publicKey = [&headers] { if (auto const iter = headers.find("Public-Key"); iter != headers.end()) { - auto pk = parseBase58( - TokenType::NodePublic, std::string_view{iter->value()}); + auto pk = + parseBase58(TokenType::NodePublic, iter->value()); if (pk) { @@ -312,7 +314,7 @@ verifyHandshake( if (iter == headers.end()) throw std::runtime_error("No session signature specified"); - auto sig = base64_decode(std::string{iter->value()}); + auto sig = base64_decode(iter->value()); if (!verifyDigest(publicKey, sharedValue, makeSlice(sig), false)) throw std::runtime_error("Failed to verify session"); @@ -324,8 +326,8 @@ verifyHandshake( if (auto const iter = headers.find("Local-IP"); iter != headers.end()) { boost::system::error_code ec; - auto const local_ip = boost::asio::ip::address::from_string( - std::string{iter->value()}, ec); + auto const local_ip = + boost::asio::ip::address::from_string(iter->value(), ec); if (ec) throw std::runtime_error("Invalid Local-IP"); @@ -339,8 +341,8 @@ verifyHandshake( if (auto const iter = headers.find("Remote-IP"); iter != headers.end()) { boost::system::error_code ec; - auto const remote_ip = boost::asio::ip::address::from_string( - std::string{iter->value()}, ec); + auto const remote_ip = + boost::asio::ip::address::from_string(iter->value(), ec); if (ec) throw std::runtime_error("Invalid Remote-IP"); diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 5db61596dc7..15dc4ac8640 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -997,12 +997,7 @@ OverlayImpl::processValidatorList( return true; }; - // Convert the boost::string_view, returned by - // boost::http::request::target(), into a std::string_view. - std::string_view key = [&req, &prefix]() { - boost::string_view const key = req.target().substr(prefix.size()); - return std::string_view(key.data(), key.length()); - }(); + std::string_view key = req.target().substr(prefix.size()); if (auto slash = key.find('/'); slash != std::string_view::npos) { diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 96fd45fed26..e92c33fc80a 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -176,7 +176,7 @@ PeerImp::run() if (auto const iter = headers_.find("Closed-Ledger"); iter != headers_.end()) { - closed = parseLedgerHash(std::string_view{iter->value()}); + closed = parseLedgerHash(iter->value()); if (!closed) fail("Malformed handshake data (1)"); @@ -185,7 +185,7 @@ PeerImp::run() if (auto const iter = headers_.find("Previous-Ledger"); iter != headers_.end()) { - previous = parseLedgerHash(std::string_view{iter->value()}); + previous = parseLedgerHash(iter->value()); if (!previous) fail("Malformed handshake data (2)"); @@ -397,7 +397,7 @@ PeerImp::json() } if (auto const d = domain(); !d.empty()) - ret[jss::server_domain] = std::string{domain()}; + ret[jss::server_domain] = std::string{d}; if (auto const nid = headers_["Network-ID"]; !nid.empty()) ret[jss::network_id] = std::string{nid}; diff --git a/src/ripple/rpc/impl/Role.cpp b/src/ripple/rpc/impl/Role.cpp index 200c45348c2..05eeed3fcdf 100644 --- a/src/ripple/rpc/impl/Role.cpp +++ b/src/ripple/rpc/impl/Role.cpp @@ -304,10 +304,7 @@ forwardedFor(http_request_type const& request) std::size_t found = it->value().find(','); if (found == boost::string_view::npos) found = it->value().length(); - // http_request_type has a dependency on boost. Explicitly convert such - // boost::string_view into std::string_view - return extractIpAddrFromField( - std::string_view{it->value().data(), found}); + return extractIpAddrFromField(it->value().substr(0, found)); } return {}; diff --git a/src/ripple/rpc/impl/ServerHandler.cpp b/src/ripple/rpc/impl/ServerHandler.cpp index b1b14054314..9c2cec593da 100644 --- a/src/ripple/rpc/impl/ServerHandler.cpp +++ b/src/ripple/rpc/impl/ServerHandler.cpp @@ -559,11 +559,11 @@ ServerHandler::processSession( makeOutput(*session), coro, forwardedFor(session->request()), - [&session]() -> std::string_view { + [&] { auto const iter = session->request().find("X-User"); if (iter != session->request().end()) - return {iter->value().data(), iter->value().length()}; - return {}; + return iter->value(); + return boost::beast::string_view{}; }()); if (beast::rfc2616::is_keep_alive(session->request())) From b78031205288805de7b914558429a5350b2c1c4c Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 8 Jan 2024 10:26:02 -0800 Subject: [PATCH 16/22] remove unused include statements --- src/ripple/app/misc/impl/ValidatorList.cpp | 1 - src/ripple/overlay/impl/Cluster.cpp | 2 -- src/ripple/overlay/impl/Handshake.cpp | 4 ---- src/ripple/overlay/impl/OverlayImpl.cpp | 1 - src/ripple/overlay/impl/PeerImp.cpp | 2 -- src/ripple/rpc/impl/Role.cpp | 2 -- 6 files changed, 12 deletions(-) diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index 6ec7f4b0774..80cfa40b298 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -33,7 +33,6 @@ #include #include -#include #include #include diff --git a/src/ripple/overlay/impl/Cluster.cpp b/src/ripple/overlay/impl/Cluster.cpp index 44954fc6abf..a7b1e44d4b7 100644 --- a/src/ripple/overlay/impl/Cluster.cpp +++ b/src/ripple/overlay/impl/Cluster.cpp @@ -17,7 +17,6 @@ */ //============================================================================== -#include #include #include #include @@ -27,7 +26,6 @@ #include #include #include -#include namespace ripple { diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index 54e1e420919..b1442e688e5 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -20,16 +20,12 @@ #include #include #include -#include #include #include #include #include -#include #include -// #include #include -#include // VFALCO Shouldn't we have to include the OpenSSL // headers or something for SSL_get_finished? diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 15dc4ac8640..017b5c43998 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -39,7 +39,6 @@ #include #include -#include namespace ripple { diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index e92c33fc80a..c7b61e379e4 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -41,7 +40,6 @@ #include #include -#include #include #include diff --git a/src/ripple/rpc/impl/Role.cpp b/src/ripple/rpc/impl/Role.cpp index 05eeed3fcdf..54f6d6e2575 100644 --- a/src/ripple/rpc/impl/Role.cpp +++ b/src/ripple/rpc/impl/Role.cpp @@ -18,9 +18,7 @@ //============================================================================== #include -#include #include -#include #include #include #include From 999d9c25447c66299d883b43f1fb6832b01b5035 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Mon, 8 Jan 2024 11:38:42 -0800 Subject: [PATCH 17/22] build instructions to configure boost to use std::string_view --- BUILD.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/BUILD.md b/BUILD.md index 21341ae9a71..4ccb11e6bec 100644 --- a/BUILD.md +++ b/BUILD.md @@ -104,6 +104,17 @@ then you will need to choose the `libstdc++11` ABI: conan profile update settings.compiler.libcxx=libstdc++11 default ``` +**Set up Boost library to use `std::string_view`**

+The below commands ensure inter-operability between `boost::string_view` and +`std::string_view` types. +``` +conan profile update 'conf.tools.build:cxxflags+=["-DBOOST_BEAST_USE_STD_STRING_VIEW"]' default +conan profile update 'env.CXXFLAGS="-DBOOST_BEAST_USE_STD_STRING_VIEW"' default +conan profile show default # sanity check: displays the default profile +``` +Note: If you have other flags in the `conf.tools.build` or `env.CXXFLAGS +section`, make sure to retain and append the existing flags. + **Windows** developers may need to use the x64 native build tools. An easy way to do that is to run the shortcut "x64 Native Tools Command Prompt" for the version of Visual Studio that you have installed. From cee5ff89d714229d417e6bbfde6e40864df1d195 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Tue, 16 Jan 2024 10:33:41 -0800 Subject: [PATCH 18/22] address John's PR comments: Update the Build instructions, include a template specialization for boost::core::string_view for struct LexicalCast --- BUILD.md | 15 +++++++++------ src/ripple/beast/core/LexicalCast.h | 18 ++++++++++++++++++ src/ripple/overlay/impl/Handshake.cpp | 4 ++-- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/BUILD.md b/BUILD.md index 4ccb11e6bec..9f67c093e71 100644 --- a/BUILD.md +++ b/BUILD.md @@ -104,16 +104,19 @@ then you will need to choose the `libstdc++11` ABI: conan profile update settings.compiler.libcxx=libstdc++11 default ``` -**Set up Boost library to use `std::string_view`**

-The below commands ensure inter-operability between `boost::string_view` and -`std::string_view` types. + +Ensure inter-operability between `boost::string_view` and `std::string_view` types: + ``` conan profile update 'conf.tools.build:cxxflags+=["-DBOOST_BEAST_USE_STD_STRING_VIEW"]' default conan profile update 'env.CXXFLAGS="-DBOOST_BEAST_USE_STD_STRING_VIEW"' default -conan profile show default # sanity check: displays the default profile ``` -Note: If you have other flags in the `conf.tools.build` or `env.CXXFLAGS -section`, make sure to retain and append the existing flags. + +If you have other flags in the `conf.tools.build` or `env.CXXFLAGS` sections, make sure to retain the existing flags and append the new ones. You can check them with: +``` +conan profile show default +``` + **Windows** developers may need to use the x64 native build tools. An easy way to do that is to run the shortcut "x64 Native Tools Command diff --git a/src/ripple/beast/core/LexicalCast.h b/src/ripple/beast/core/LexicalCast.h index 215f4ec803b..dc8505e2d6b 100644 --- a/src/ripple/beast/core/LexicalCast.h +++ b/src/ripple/beast/core/LexicalCast.h @@ -20,6 +20,7 @@ #ifndef BEAST_MODULE_CORE_TEXT_LEXICALCAST_H_INCLUDED #define BEAST_MODULE_CORE_TEXT_LEXICALCAST_H_INCLUDED +#include #include #include #include @@ -119,6 +120,23 @@ struct LexicalCast }; //------------------------------------------------------------------------------ +// Parse boost library's string_view to number or boolean value +// Note: As of Jan 2024, Boost contains three different types of string_view +// (boost::core::basic_string_view, boost::string_ref and +// boost::string_view). The below template specialization is included because +// it is used in the handshake.cpp file +template +struct LexicalCast> +{ + explicit LexicalCast() = default; + + bool + operator()(Out& out, boost::core::basic_string_view in) const + { + return LexicalCast()(out, in); + } +}; + // Parse std::string to number or boolean value template struct LexicalCast diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index b1442e688e5..7fb3fb465fe 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -239,7 +239,7 @@ verifyHandshake( { std::uint32_t nid; - if (!beast::lexicalCastChecked(nid, std::string_view{iter->value()})) + if (!beast::lexicalCastChecked(nid, iter->value())) throw std::runtime_error("Invalid peer network identifier"); if (networkID && nid != *networkID) @@ -251,7 +251,7 @@ verifyHandshake( auto const netTime = [str = iter->value()]() -> TimeKeeper::time_point { TimeKeeper::duration::rep val; - if (beast::lexicalCastChecked(val, std::string_view{str})) + if (beast::lexicalCastChecked(val, str)) return TimeKeeper::time_point{TimeKeeper::duration{val}}; // It's not an error for the header field to not be present but if From cfa960f7ef3c84e61bc67155ee0ebe5714e30677 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Fri, 26 Jan 2024 10:26:29 -0800 Subject: [PATCH 19/22] replace the usage of std::string_view const& to std::string_view (pass-by-value). The latter has shorter and simplified assembly code, as pointed in Arthur O Dwyer's blog post --- src/ripple/basics/StringUtilities.h | 2 +- src/ripple/basics/base64.h | 2 +- src/ripple/basics/impl/StringUtilities.cpp | 2 +- src/ripple/basics/impl/base64.cpp | 2 +- src/ripple/beast/core/LexicalCast.h | 4 ++-- src/ripple/overlay/impl/PeerImp.cpp | 2 +- src/ripple/protocol/PublicKey.h | 2 +- src/ripple/protocol/SecretKey.h | 2 +- src/ripple/protocol/impl/PublicKey.cpp | 2 +- src/ripple/protocol/impl/SecretKey.cpp | 2 +- src/ripple/protocol/impl/tokens.cpp | 4 ++-- src/ripple/protocol/tokens.h | 4 ++-- src/test/app/NFTokenDir_test.cpp | 8 ++++---- 13 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/ripple/basics/StringUtilities.h b/src/ripple/basics/StringUtilities.h index a8cacc681c6..69314ce096e 100644 --- a/src/ripple/basics/StringUtilities.h +++ b/src/ripple/basics/StringUtilities.h @@ -150,7 +150,7 @@ to_uint64(std::string const& s); doesn't check whether the TLD is valid. */ bool -isProperlyFormedTomlDomain(std::string_view const& domain); +isProperlyFormedTomlDomain(std::string_view domain); } // namespace ripple diff --git a/src/ripple/basics/base64.h b/src/ripple/basics/base64.h index 77960ec66bc..515a7584e56 100644 --- a/src/ripple/basics/base64.h +++ b/src/ripple/basics/base64.h @@ -73,7 +73,7 @@ base64_encode(std::string const& s) } std::string -base64_decode(std::string_view const& data); +base64_decode(std::string_view data); } // namespace ripple diff --git a/src/ripple/basics/impl/StringUtilities.cpp b/src/ripple/basics/impl/StringUtilities.cpp index c11d1ea55af..67344b48013 100644 --- a/src/ripple/basics/impl/StringUtilities.cpp +++ b/src/ripple/basics/impl/StringUtilities.cpp @@ -120,7 +120,7 @@ to_uint64(std::string const& s) } bool -isProperlyFormedTomlDomain(std::string_view const& domain) +isProperlyFormedTomlDomain(std::string_view domain) { // The domain must be between 4 and 128 characters long if (domain.size() < 4 || domain.size() > 128) diff --git a/src/ripple/basics/impl/base64.cpp b/src/ripple/basics/impl/base64.cpp index 4e8dcd7dc93..4291a99556b 100644 --- a/src/ripple/basics/impl/base64.cpp +++ b/src/ripple/basics/impl/base64.cpp @@ -242,7 +242,7 @@ base64_encode(std::uint8_t const* data, std::size_t len) } std::string -base64_decode(std::string_view const& data) +base64_decode(std::string_view data) { std::string dest; dest.resize(base64::decoded_size(data.size())); diff --git a/src/ripple/beast/core/LexicalCast.h b/src/ripple/beast/core/LexicalCast.h index dc8505e2d6b..e0fa24ca9f5 100644 --- a/src/ripple/beast/core/LexicalCast.h +++ b/src/ripple/beast/core/LexicalCast.h @@ -79,7 +79,7 @@ struct LexicalCast std::enable_if_t< std::is_integral_v && !std::is_same_v, bool> - operator()(Integral& out, std::string_view const& in) const + operator()(Integral& out, std::string_view in) const { auto first = in.data(); auto last = in.data() + in.size(); @@ -93,7 +93,7 @@ struct LexicalCast } bool - operator()(bool& out, std::string_view const& in) const + operator()(bool& out, std::string_view in) const { std::string result; diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index c7b61e379e4..dcf864ea32f 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -158,7 +158,7 @@ PeerImp::run() return post(strand_, std::bind(&PeerImp::run, shared_from_this())); auto parseLedgerHash = - [](std::string_view const& value) -> std::optional { + [](std::string_view value) -> std::optional { if (uint256 ret; ret.parseHex(value)) return ret; diff --git a/src/ripple/protocol/PublicKey.h b/src/ripple/protocol/PublicKey.h index ace46282731..2a051344685 100644 --- a/src/ripple/protocol/PublicKey.h +++ b/src/ripple/protocol/PublicKey.h @@ -192,7 +192,7 @@ toBase58(TokenType type, PublicKey const& pk) template <> std::optional -parseBase58(TokenType type, std::string_view const& s); +parseBase58(TokenType type, std::string_view s); enum class ECDSACanonicality { canonical, fullyCanonical }; diff --git a/src/ripple/protocol/SecretKey.h b/src/ripple/protocol/SecretKey.h index 11d57192b64..9bed782b00a 100644 --- a/src/ripple/protocol/SecretKey.h +++ b/src/ripple/protocol/SecretKey.h @@ -114,7 +114,7 @@ operator!=(SecretKey const& lhs, SecretKey const& rhs) /** Parse a secret key */ template <> std::optional -parseBase58(TokenType type, std::string_view const& s); +parseBase58(TokenType type, std::string_view s); inline std::string toBase58(TokenType type, SecretKey const& sk) diff --git a/src/ripple/protocol/impl/PublicKey.cpp b/src/ripple/protocol/impl/PublicKey.cpp index 18b483df883..2306fca0b96 100644 --- a/src/ripple/protocol/impl/PublicKey.cpp +++ b/src/ripple/protocol/impl/PublicKey.cpp @@ -37,7 +37,7 @@ operator<<(std::ostream& os, PublicKey const& pk) template <> std::optional -parseBase58(TokenType type, std::string_view const& s) +parseBase58(TokenType type, std::string_view s) { auto const result = decodeBase58Token(s, type); auto const pks = makeSlice(result); diff --git a/src/ripple/protocol/impl/SecretKey.cpp b/src/ripple/protocol/impl/SecretKey.cpp index bbf077fa8eb..93078350629 100644 --- a/src/ripple/protocol/impl/SecretKey.cpp +++ b/src/ripple/protocol/impl/SecretKey.cpp @@ -373,7 +373,7 @@ randomKeyPair(KeyType type) template <> std::optional -parseBase58(TokenType type, std::string_view const& s) +parseBase58(TokenType type, std::string_view s) { auto const result = decodeBase58Token(s, type); if (result.empty()) diff --git a/src/ripple/protocol/impl/tokens.cpp b/src/ripple/protocol/impl/tokens.cpp index 31526738960..80fb75bc730 100644 --- a/src/ripple/protocol/impl/tokens.cpp +++ b/src/ripple/protocol/impl/tokens.cpp @@ -147,7 +147,7 @@ encodeBase58( } static std::string -decodeBase58(std::string_view const& s) +decodeBase58(std::string_view s) { auto psz = reinterpret_cast(s.data()); auto remain = s.size(); @@ -220,7 +220,7 @@ encodeBase58Token(TokenType type, void const* token, std::size_t size) } std::string -decodeBase58Token(std::string_view const& s, TokenType type) +decodeBase58Token(std::string_view s, TokenType type) { std::string const ret = detail::decodeBase58(s); diff --git a/src/ripple/protocol/tokens.h b/src/ripple/protocol/tokens.h index d58e1d19b5b..eac158882b0 100644 --- a/src/ripple/protocol/tokens.h +++ b/src/ripple/protocol/tokens.h @@ -43,7 +43,7 @@ parseBase58(std::string const& s); template std::optional -parseBase58(TokenType type, std::string_view const& s); +parseBase58(TokenType type, std::string_view s); /** Encode data in Base58Check format using XRPL alphabet @@ -68,7 +68,7 @@ encodeBase58Token(TokenType type, void const* token, std::size_t size); the type or checksum. And empty string otherwise. */ std::string -decodeBase58Token(std::string_view const& s, TokenType type); +decodeBase58Token(std::string_view s, TokenType type); } // namespace ripple diff --git a/src/test/app/NFTokenDir_test.cpp b/src/test/app/NFTokenDir_test.cpp index e9addfa83f7..8e6a9fe5209 100644 --- a/src/test/app/NFTokenDir_test.cpp +++ b/src/test/app/NFTokenDir_test.cpp @@ -185,7 +185,7 @@ class NFTokenDir_test : public beast::unit_test::suite // Create accounts for all of the seeds and fund those accounts. std::vector accounts; accounts.reserve(seeds.size()); - for (std::string_view const& seed : seeds) + for (std::string_view seed : seeds) { Account const& account = accounts.emplace_back( Account::base58Seed, std::string(seed)); @@ -409,7 +409,7 @@ class NFTokenDir_test : public beast::unit_test::suite // Create accounts for all of the seeds and fund those accounts. std::vector accounts; accounts.reserve(seeds.size()); - for (std::string_view const& seed : seeds) + for (std::string_view seed : seeds) { Account const& account = accounts.emplace_back( Account::base58Seed, std::string(seed)); @@ -659,7 +659,7 @@ class NFTokenDir_test : public beast::unit_test::suite // Create accounts for all of the seeds and fund those accounts. std::vector accounts; accounts.reserve(seeds.size()); - for (std::string_view const& seed : seeds) + for (std::string_view seed : seeds) { Account const& account = accounts.emplace_back(Account::base58Seed, std::string(seed)); @@ -840,7 +840,7 @@ class NFTokenDir_test : public beast::unit_test::suite // Create accounts for all of the seeds and fund those accounts. std::vector accounts; accounts.reserve(seeds.size()); - for (std::string_view const& seed : seeds) + for (std::string_view seed : seeds) { Account const& account = accounts.emplace_back(Account::base58Seed, std::string(seed)); From 204f9f4353552ea1c14733997f11092d182ccf96 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Fri, 2 Feb 2024 13:07:58 -0800 Subject: [PATCH 20/22] construct a std:;string instead of using the assignment operator --- src/ripple/overlay/impl/Handshake.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index 7fb3fb465fe..b655859dcbc 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -42,7 +42,7 @@ getFeatureValue( return {}; boost::smatch match; boost::regex rx(feature + "=([^;\\s]+)"); - std::string const allFeatures = std::string{header->value()}; + std::string const allFeatures(header->value()); if (boost::regex_search(allFeatures, match, rx)) return {match[1]}; return {}; From 49968bfd5c754a4d54e9f404645e0f227d4b32d2 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Wed, 21 Feb 2024 23:17:12 -0800 Subject: [PATCH 21/22] do not return a std::string_view from functions for safety reasons - revert such changes --- src/ripple/overlay/impl/PeerImp.cpp | 4 ++-- src/ripple/overlay/impl/PeerImp.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index dcf864ea32f..e72fa0f8655 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -366,7 +366,7 @@ PeerImp::cluster() const return static_cast(app_.cluster().member(publicKey_)); } -std::string_view +std::string PeerImp::getVersion() const { if (inbound_) @@ -834,7 +834,7 @@ PeerImp::name() const return name_; } -std::string_view +std::string PeerImp::domain() const { return headers_["Server-Domain"]; diff --git a/src/ripple/overlay/impl/PeerImp.h b/src/ripple/overlay/impl/PeerImp.h index ea8fbb0256e..710ab4d74d6 100644 --- a/src/ripple/overlay/impl/PeerImp.h +++ b/src/ripple/overlay/impl/PeerImp.h @@ -344,7 +344,7 @@ class PeerImp : public Peer, } /** Return the version of rippled that the peer is running, if reported. */ - std::string_view + std::string getVersion() const; // Return the connection elapsed time. @@ -464,7 +464,7 @@ class PeerImp : public Peer, std::string name() const; - std::string_view + std::string domain() const; // From 3cda3165aaa22b6814aa142c73f8ca5463432e47 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Wed, 6 Mar 2024 16:26:55 -0800 Subject: [PATCH 22/22] - rectify the mistakes in the merge from base58 changes. - update the overloads of parseBase58 overloads in PublicKey, SecretKey classes to be compatible with the new fast base58 algorithm --- src/ripple/protocol/PublicKey.h | 2 +- src/ripple/protocol/SecretKey.h | 2 +- src/ripple/protocol/impl/PublicKey.cpp | 2 +- src/ripple/protocol/impl/SecretKey.cpp | 2 +- src/ripple/protocol/impl/tokens.cpp | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ripple/protocol/PublicKey.h b/src/ripple/protocol/PublicKey.h index 34ea4c5d1b3..9cf1a456953 100644 --- a/src/ripple/protocol/PublicKey.h +++ b/src/ripple/protocol/PublicKey.h @@ -189,7 +189,7 @@ toBase58(TokenType type, PublicKey const& pk) template <> std::optional -parseBase58(TokenType type, std::string_view s); +parseBase58(TokenType type, std::string const& s); enum class ECDSACanonicality { canonical, fullyCanonical }; diff --git a/src/ripple/protocol/SecretKey.h b/src/ripple/protocol/SecretKey.h index 7c25bb33e91..824ae9b1e0f 100644 --- a/src/ripple/protocol/SecretKey.h +++ b/src/ripple/protocol/SecretKey.h @@ -114,7 +114,7 @@ operator!=(SecretKey const& lhs, SecretKey const& rhs) /** Parse a secret key */ template <> std::optional -parseBase58(TokenType type, std::string_view s); +parseBase58(TokenType type, std::string const& s); inline std::string toBase58(TokenType type, SecretKey const& sk) diff --git a/src/ripple/protocol/impl/PublicKey.cpp b/src/ripple/protocol/impl/PublicKey.cpp index 8d475ea80f0..22cb351e61c 100644 --- a/src/ripple/protocol/impl/PublicKey.cpp +++ b/src/ripple/protocol/impl/PublicKey.cpp @@ -36,7 +36,7 @@ operator<<(std::ostream& os, PublicKey const& pk) template <> std::optional -parseBase58(TokenType type, std::string_view s) +parseBase58(TokenType type, std::string const& s) { auto const result = decodeBase58Token(s, type); auto const pks = makeSlice(result); diff --git a/src/ripple/protocol/impl/SecretKey.cpp b/src/ripple/protocol/impl/SecretKey.cpp index 93078350629..63661888f48 100644 --- a/src/ripple/protocol/impl/SecretKey.cpp +++ b/src/ripple/protocol/impl/SecretKey.cpp @@ -373,7 +373,7 @@ randomKeyPair(KeyType type) template <> std::optional -parseBase58(TokenType type, std::string_view s) +parseBase58(TokenType type, std::string const& s) { auto const result = decodeBase58Token(s, type); if (result.empty()) diff --git a/src/ripple/protocol/impl/tokens.cpp b/src/ripple/protocol/impl/tokens.cpp index 0a1c20f77b4..8445eec38ca 100644 --- a/src/ripple/protocol/impl/tokens.cpp +++ b/src/ripple/protocol/impl/tokens.cpp @@ -269,7 +269,7 @@ encodeBase58( std::string decodeBase58(std::string const& s) { - auto psz = reinterpret_cast(s.data()); + auto psz = reinterpret_cast(s.c_str()); auto remain = s.size(); // Skip and count leading zeroes int zeroes = 0; @@ -340,7 +340,7 @@ encodeBase58Token(TokenType type, void const* token, std::size_t size) } std::string -decodeBase58Token(std::string_view s, TokenType type) +decodeBase58Token(std::string const& s, TokenType type) { std::string const ret = detail::decodeBase58(s);