From bdaad19e70cf40b9b2b287d4439dd26f5eb29701 Mon Sep 17 00:00:00 2001 From: Mike Ellery Date: Mon, 8 Oct 2018 09:59:20 -0700 Subject: [PATCH 1/4] Accept redirects from validator list sites: Honor location header/redirect from validator sites. Limit retries per refresh interval to 3. Shorten refresh interval after HTTP/network errors. Fixes: RIPD-1669 --- src/ripple/app/misc/ValidatorSite.h | 52 ++- src/ripple/app/misc/impl/ValidatorSite.cpp | 389 ++++++++++++++------- src/ripple/protocol/JsonFields.h | 2 + src/test/app/ValidatorSite_test.cpp | 227 +++++++----- src/test/jtx/TrustedPublisherServer.h | 62 +++- 5 files changed, 497 insertions(+), 235 deletions(-) diff --git a/src/ripple/app/misc/ValidatorSite.h b/src/ripple/app/misc/ValidatorSite.h index 080e844030..3ff35c8c07 100644 --- a/src/ripple/app/misc/ValidatorSite.h +++ b/src/ripple/app/misc/ValidatorSite.h @@ -27,6 +27,7 @@ #include #include #include +#include namespace ripple { @@ -73,10 +74,33 @@ private: { clock_type::time_point refreshed; ListDisposition disposition; + std::string message; }; - std::string uri; - parsedURL pUrl; + struct Resource + { + explicit Resource(std::string u); + std::string uri; + parsedURL pUrl; + }; + using ResourcePtr = std::shared_ptr; + + explicit Site(std::string uri); + + /// the original uri as loaded from config + ResourcePtr loadedResource; + + /// the resource to to request at + /// intervals. same as loadedResource + /// except in the case of a permanent redir. + ResourcePtr startingResource; + + /// the active resource being requested. + /// same as startingResource except + /// when we've gotten a temp redirect + ResourcePtr activeResource; + + unsigned short redirCount; std::chrono::minutes refreshInterval; clock_type::time_point nextRefresh; boost::optional lastRefreshStatus; @@ -176,6 +200,30 @@ private: boost::system::error_code const& ec, detail::response_type&& res, std::size_t siteIdx); + + /// Initiate request to given resource. + /// lock over sites_mutex_ required + void + makeRequest ( + Site::ResourcePtr resource, + std::size_t siteIdx, + std::lock_guard& lock); + + /// Parse json response from validator list site. + /// lock over sites_mutex_ required + void + parseJsonResponse ( + detail::response_type& res, + std::size_t siteIdx, + std::lock_guard& lock); + + /// Interpret a redirect response. + /// lock over sites_mutex_ required + Site::ResourcePtr + processRedirect ( + detail::response_type& res, + std::size_t siteIdx, + std::lock_guard& lock); }; } // ripple diff --git a/src/ripple/app/misc/impl/ValidatorSite.cpp b/src/ripple/app/misc/impl/ValidatorSite.cpp index 7e35a23436..cba5fcb451 100644 --- a/src/ripple/app/misc/impl/ValidatorSite.cpp +++ b/src/ripple/app/misc/impl/ValidatorSite.cpp @@ -31,6 +31,30 @@ namespace ripple { // default site query frequency - 5 minutes auto constexpr DEFAULT_REFRESH_INTERVAL = std::chrono::minutes{5}; +auto constexpr ERROR_RETRY_INTERVAL = std::chrono::seconds{30}; +unsigned short constexpr MAX_REDIRECTS = 3; + +ValidatorSite::Site::Resource::Resource (std::string u) + : uri {std::move(u)} +{ + if (! parseUrl (pUrl, uri) || + (pUrl.scheme != "http" && pUrl.scheme != "https")) + { + throw std::runtime_error {"invalid url"}; + } + + if (! pUrl.port) + pUrl.port = (pUrl.scheme == "https") ? 443 : 80; +} + +ValidatorSite::Site::Site (std::string uri) + : loadedResource {std::make_shared(std::move(uri))} + , startingResource {loadedResource} + , redirCount {0} + , refreshInterval {DEFAULT_REFRESH_INTERVAL} + , nextRefresh {clock_type::now()} +{ +} ValidatorSite::ValidatorSite ( boost::asio::io_service& ios, @@ -74,20 +98,16 @@ ValidatorSite::load ( for (auto uri : siteURIs) { - parsedURL pUrl; - if (! parseUrl (pUrl, uri) || - (pUrl.scheme != "http" && pUrl.scheme != "https")) + try + { + sites_.emplace_back (uri); + } + catch (std::exception &) { JLOG (j_.error()) << "Invalid validator site uri: " << uri; return false; } - - if (! pUrl.port) - pUrl.port = (pUrl.scheme == "https") ? 443 : 80; - - sites_.push_back ({ - uri, pUrl, DEFAULT_REFRESH_INTERVAL, clock_type::now()}); } JLOG (j_.debug()) << @@ -149,6 +169,45 @@ ValidatorSite::setTimer () } } +void +ValidatorSite::makeRequest ( + Site::ResourcePtr resource, + std::size_t siteIdx, + std::lock_guard& lock) +{ + fetching_ = true; + sites_[siteIdx].activeResource = resource; + std::shared_ptr sp; + auto onFetch = + [this, siteIdx] (error_code const& err, detail::response_type&& resp) + { + onSiteFetch (err, std::move(resp), siteIdx); + }; + + if (resource->pUrl.scheme == "https") + { + sp = std::make_shared( + resource->pUrl.domain, + resource->pUrl.path, + std::to_string(*resource->pUrl.port), + ios_, + j_, + onFetch); + } + else + { + sp = std::make_shared( + resource->pUrl.domain, + resource->pUrl.path, + std::to_string(*resource->pUrl.port), + ios_, + onFetch); + } + + work_ = sp; + sp->run (); +} + void ValidatorSite::onTimer ( std::size_t siteIdx, @@ -165,40 +224,145 @@ ValidatorSite::onTimer ( std::lock_guard lock{sites_mutex_}; sites_[siteIdx].nextRefresh = - clock_type::now() + DEFAULT_REFRESH_INTERVAL; + clock_type::now() + sites_[siteIdx].refreshInterval; assert(! fetching_); - fetching_ = true; + sites_[siteIdx].redirCount = 0; + makeRequest(sites_[siteIdx].startingResource, siteIdx, lock); +} - std::shared_ptr sp; - if (sites_[siteIdx].pUrl.scheme == "https") +void +ValidatorSite::parseJsonResponse ( + detail::response_type& res, + std::size_t siteIdx, + std::lock_guard& lock) +{ + Json::Reader r; + Json::Value body; + if (! r.parse(res.body().data(), body)) { - sp = std::make_shared( - sites_[siteIdx].pUrl.domain, - sites_[siteIdx].pUrl.path, - std::to_string(*sites_[siteIdx].pUrl.port), - ios_, - j_, - [this, siteIdx](error_code const& err, detail::response_type&& resp) - { - onSiteFetch (err, std::move(resp), siteIdx); - }); + JLOG (j_.warn()) << + "Unable to parse JSON response from " << + sites_[siteIdx].activeResource->uri; + throw std::runtime_error{"bad json"}; + } + + if( ! body.isObject () || + ! body.isMember("blob") || ! body["blob"].isString () || + ! body.isMember("manifest") || ! body["manifest"].isString () || + ! body.isMember("signature") || ! body["signature"].isString() || + ! body.isMember("version") || ! body["version"].isInt()) + { + JLOG (j_.warn()) << + "Missing fields in JSON response from " << + sites_[siteIdx].activeResource->uri; + throw std::runtime_error{"missing fields"}; + } + + auto const disp = validators_.applyList ( + body["manifest"].asString (), + body["blob"].asString (), + body["signature"].asString(), + body["version"].asUInt()); + + sites_[siteIdx].lastRefreshStatus.emplace( + Site::Status{clock_type::now(), disp, ""}); + + if (ListDisposition::accepted == disp) + { + JLOG (j_.debug()) << + "Applied new validator list from " << + sites_[siteIdx].activeResource->uri; + } + else if (ListDisposition::same_sequence == disp) + { + JLOG (j_.debug()) << + "Validator list with current sequence from " << + sites_[siteIdx].activeResource->uri; + } + else if (ListDisposition::stale == disp) + { + JLOG (j_.warn()) << + "Stale validator list from " << + sites_[siteIdx].activeResource->uri; + } + else if (ListDisposition::untrusted == disp) + { + JLOG (j_.warn()) << + "Untrusted validator list from " << + sites_[siteIdx].activeResource->uri; + } + else if (ListDisposition::invalid == disp) + { + JLOG (j_.warn()) << + "Invalid validator list from " << + sites_[siteIdx].activeResource->uri; + } + else if (ListDisposition::unsupported_version == disp) + { + JLOG (j_.warn()) << + "Unsupported version validator list from " << + sites_[siteIdx].activeResource->uri; } else { - sp = std::make_shared( - sites_[siteIdx].pUrl.domain, - sites_[siteIdx].pUrl.path, - std::to_string(*sites_[siteIdx].pUrl.port), - ios_, - [this, siteIdx](error_code const& err, detail::response_type&& resp) - { - onSiteFetch (err, std::move(resp), siteIdx); - }); + BOOST_ASSERT(false); } - work_ = sp; - sp->run (); + if (body.isMember ("refresh_interval") && + body["refresh_interval"].isNumeric ()) + { + // TODO: should we sanity check/clamp this value + // to something reasonable? + sites_[siteIdx].refreshInterval = + std::chrono::minutes{body["refresh_interval"].asUInt ()}; + } +} + +ValidatorSite::Site::ResourcePtr +ValidatorSite::processRedirect ( + detail::response_type& res, + std::size_t siteIdx, + std::lock_guard& lock) +{ + using namespace boost::beast::http; + Site::ResourcePtr newLocation; + if (res.find(field::location) == res.end() || + res[field::location].empty()) + { + JLOG (j_.warn()) << + "Request for validator list at " << + sites_[siteIdx].activeResource->uri << + " returned a redirect with no Location."; + throw std::runtime_error{"missing location"}; + } + + if (sites_[siteIdx].redirCount == MAX_REDIRECTS) + { + JLOG (j_.warn()) << + "Exceeded max redirects for validator list at " << + sites_[siteIdx].loadedResource->uri ; + throw std::runtime_error{"max redirects"}; + } + + JLOG (j_.debug()) << + "Got redirect for validator list from " << + sites_[siteIdx].activeResource->uri << + " to new location " << res[field::location]; + + try + { + newLocation = std::make_shared( + std::string(res[field::location])); + sites_[siteIdx].redirCount++; + } + catch (std::exception &) + { + JLOG (j_.error()) << + "Invalid redirect location: " << res[field::location]; + throw; + } + return newLocation; } void @@ -207,111 +371,77 @@ ValidatorSite::onSiteFetch( detail::response_type&& res, std::size_t siteIdx) { - if (! ec && res.result() != boost::beast::http::status::ok) + Site::ResourcePtr newLocation; + bool shouldRetry = false; { - std::lock_guard lock{sites_mutex_}; - JLOG (j_.warn()) << - "Request for validator list at " << - sites_[siteIdx].uri << " returned " << res.result_int(); - - sites_[siteIdx].lastRefreshStatus.emplace( - Site::Status{clock_type::now(), ListDisposition::invalid}); - } - else if (! ec) - { - std::lock_guard lock{sites_mutex_}; - Json::Reader r; - Json::Value body; - if (r.parse(res.body().data(), body) && - body.isObject () && - body.isMember("blob") && body["blob"].isString () && - body.isMember("manifest") && body["manifest"].isString () && - body.isMember("signature") && body["signature"].isString() && - body.isMember("version") && body["version"].isInt()) + std::lock_guard lock_sites{sites_mutex_}; + try { - - auto const disp = validators_.applyList ( - body["manifest"].asString (), - body["blob"].asString (), - body["signature"].asString(), - body["version"].asUInt()); - - sites_[siteIdx].lastRefreshStatus.emplace( - Site::Status{clock_type::now(), disp}); - - if (ListDisposition::accepted == disp) - { - JLOG (j_.debug()) << - "Applied new validator list from " << - sites_[siteIdx].uri; - } - else if (ListDisposition::same_sequence == disp) - { - JLOG (j_.debug()) << - "Validator list with current sequence from " << - sites_[siteIdx].uri; - } - else if (ListDisposition::stale == disp) + if (ec) { JLOG (j_.warn()) << - "Stale validator list from " << sites_[siteIdx].uri; - } - else if (ListDisposition::untrusted == disp) - { - JLOG (j_.warn()) << - "Untrusted validator list from " << - sites_[siteIdx].uri; - } - else if (ListDisposition::invalid == disp) - { - JLOG (j_.warn()) << - "Invalid validator list from " << - sites_[siteIdx].uri; - } - else if (ListDisposition::unsupported_version == disp) - { - JLOG (j_.warn()) << - "Unsupported version validator list from " << - sites_[siteIdx].uri; + "Problem retrieving from " << + sites_[siteIdx].activeResource->uri << + " " << + ec.value() << + ":" << + ec.message(); + shouldRetry = true; + throw std::runtime_error{"fetch error"}; } else { - BOOST_ASSERT(false); - } + using namespace boost::beast::http; + if (res.result() == status::ok) + { + parseJsonResponse(res, siteIdx, lock_sites); + } + else if (res.result() == status::moved_permanently || + res.result() == status::permanent_redirect || + res.result() == status::found || + res.result() == status::temporary_redirect) + { + newLocation = processRedirect (res, siteIdx, lock_sites); + // for perm redirects, also update our starting URI + if (res.result() == status::moved_permanently || + res.result() == status::permanent_redirect) + { + sites_[siteIdx].startingResource = newLocation; + } + } + else + { + JLOG (j_.warn()) << + "Request for validator list at " << + sites_[siteIdx].activeResource->uri << + " returned bad status: " << + res.result_int(); + shouldRetry = true; + throw std::runtime_error{"bad result code"}; + } - if (body.isMember ("refresh_interval") && - body["refresh_interval"].isNumeric ()) - { - sites_[siteIdx].refreshInterval = - std::chrono::minutes{body["refresh_interval"].asUInt ()}; + if (newLocation) + { + makeRequest(newLocation, siteIdx, lock_sites); + return; // we are still fetching, so skip + // state update/notify below + } } } - else + catch (std::exception& ex) { - JLOG (j_.warn()) << - "Unable to parse JSON response from " << - sites_[siteIdx].uri; - sites_[siteIdx].lastRefreshStatus.emplace( - Site::Status{clock_type::now(), ListDisposition::invalid}); + Site::Status{clock_type::now(), + ListDisposition::invalid, + ex.what()}); + if (shouldRetry) + sites_[siteIdx].nextRefresh = + clock_type::now() + ERROR_RETRY_INTERVAL; } - } - else - { - std::lock_guard lock{sites_mutex_}; - sites_[siteIdx].lastRefreshStatus.emplace( - Site::Status{clock_type::now(), ListDisposition::invalid}); - - JLOG (j_.warn()) << - "Problem retrieving from " << - sites_[siteIdx].uri << - " " << - ec.value() << - ":" << - ec.message(); + sites_[siteIdx].activeResource.reset(); } - std::lock_guard lock{state_mutex_}; + std::lock_guard lock_state{state_mutex_}; fetching_ = false; if (! stopping_) setTimer (); @@ -331,15 +461,22 @@ ValidatorSite::getJson() const for (Site const& site : sites_) { Json::Value& v = jSites.append(Json::objectValue); - v[jss::uri] = site.uri; + std::stringstream uri; + uri << site.loadedResource->uri; + if (site.loadedResource != site.startingResource) + uri << " (redirects to " << site.startingResource->uri + ")"; + v[jss::uri] = uri.str(); + v[jss::next_refresh_time] = to_string(site.nextRefresh); if (site.lastRefreshStatus) { v[jss::last_refresh_time] = to_string(site.lastRefreshStatus->refreshed); v[jss::last_refresh_status] = to_string(site.lastRefreshStatus->disposition); + if (! site.lastRefreshStatus->message.empty()) + v[jss::last_refresh_message] = + site.lastRefreshStatus->message; } - v[jss::refresh_interval_min] = static_cast(site.refreshInterval.count()); } diff --git a/src/ripple/protocol/JsonFields.h b/src/ripple/protocol/JsonFields.h index 70c2cc2bdb..18a52695be 100644 --- a/src/ripple/protocol/JsonFields.h +++ b/src/ripple/protocol/JsonFields.h @@ -233,6 +233,7 @@ JSS ( last ); // out: RPCVersion JSS ( last_close ); // out: NetworkOPs JSS ( last_refresh_time ); // out: ValidatorSite JSS ( last_refresh_status ); // out: ValidatorSite +JSS ( last_refresh_message ); // out: ValidatorSite JSS ( ledger ); // in: NetworkOPs, LedgerCleaner, // RPCHelpers // out: NetworkOPs, PeerImp @@ -305,6 +306,7 @@ JSS ( name ); // out: AmendmentTableImpl, PeerImp JSS ( needed_state_hashes ); // out: InboundLedger JSS ( needed_transaction_hashes ); // out: InboundLedger JSS ( network_ledger ); // out: NetworkOPs +JSS ( next_refresh_time ); // out: ValidatorSite JSS ( no_ripple ); // out: AccountLines JSS ( no_ripple_peer ); // out: AccountLines JSS ( node ); // out: LedgerEntry diff --git a/src/test/app/ValidatorSite_test.cpp b/src/test/app/ValidatorSite_test.cpp index 327e127cce..d65850692c 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -23,11 +23,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include namespace ripple { @@ -121,123 +123,131 @@ private: BEAST_EXPECT(!trustedSites->load (badSites)); } - void - testFetchList () + class TestSink : public beast::Journal::Sink { - testcase ("Fetch list"); + public: + std::stringstream strm_; + + TestSink () : Sink (beast::severities::kDebug, false) { } + + void + write (beast::severities::Severity level, + std::string const& text) override + { + if (level < threshold()) + return; + + strm_ << text << std::endl; + } + }; + + void + testFetchList ( + std::vector> const& paths) + { + testcase << "Fetch list - " << paths[0].first << + (paths.size() > 1 ? ", " + paths[1].first : ""); using namespace jtx; Env env (*this); auto& trustedKeys = env.app ().validators (); - beast::Journal journal; + TestSink sink; + beast::Journal journal{sink}; PublicKey emptyLocalKey; std::vector emptyCfgKeys; - - auto const publisherSecret1 = randomSecretKey(); - auto const publisherPublic1 = - derivePublicKey(KeyType::ed25519, publisherSecret1); - auto const pubSigningKeys1 = randomKeyPair(KeyType::secp256k1); - - auto const manifest1 = makeManifestString ( - publisherPublic1, publisherSecret1, - pubSigningKeys1.first, pubSigningKeys1.second, 1); - - auto const publisherSecret2 = randomSecretKey(); - auto const publisherPublic2 = - derivePublicKey(KeyType::ed25519, publisherSecret2); - auto const pubSigningKeys2 = randomKeyPair(KeyType::secp256k1); - - auto const manifest2 = makeManifestString ( - publisherPublic2, publisherSecret2, - pubSigningKeys2.first, pubSigningKeys2.second, 1); - - std::vector cfgPublishers({ - strHex(publisherPublic1), - strHex(publisherPublic2)}); - - BEAST_EXPECT(trustedKeys.load ( - emptyLocalKey, emptyCfgKeys, cfgPublishers)); - - auto constexpr listSize = 20; - std::vector list1; - list1.reserve (listSize); - while (list1.size () < listSize) - list1.push_back (randomValidator()); - - std::vector list2; - list2.reserve (listSize); - while (list2.size () < listSize) - list2.push_back (randomValidator()); + struct publisher + { + std::unique_ptr server; + std::vector list; + std::string uri; + std::string expectMsg; + bool shouldFail; + }; + std::vector servers; auto const sequence = 1; auto const version = 1; using namespace std::chrono_literals; NetClock::time_point const expiration = env.timeKeeper().now() + 3600s; + auto constexpr listSize = 20; + std::vector cfgPublishers; - TrustedPublisherServer server1( - env.app().getIOService(), - pubSigningKeys1, - manifest1, - sequence, - expiration, - version, - list1); - - TrustedPublisherServer server2( - env.app().getIOService(), - pubSigningKeys2, - manifest2, - sequence, - expiration, - version, - list2); - - std::stringstream url1, url2; - url1 << "http://" << server1.local_endpoint() << "/validators"; - url2 << "http://" << server2.local_endpoint() << "/validators"; - + for (auto const& cfg : paths) { - // fetch single site - std::vector cfgSites({ url1.str() }); + auto const publisherSecret = randomSecretKey(); + auto const publisherPublic = + derivePublicKey(KeyType::ed25519, publisherSecret); + auto const pubSigningKeys = randomKeyPair(KeyType::secp256k1); + cfgPublishers.push_back(strHex(publisherPublic)); - auto sites = std::make_unique ( - env.app().getIOService(), env.app().validators(), journal); + auto const manifest = makeManifestString ( + publisherPublic, publisherSecret, + pubSigningKeys.first, pubSigningKeys.second, 1); - sites->load (cfgSites); - sites->start(); - sites->join(); + servers.push_back({}); + auto& item = servers.back(); + item.shouldFail = ! cfg.second.empty(); + item.expectMsg = cfg.second; + item.list.reserve (listSize); + while (item.list.size () < listSize) + item.list.push_back (randomValidator()); - for (auto const& val : list1) - { - BEAST_EXPECT(trustedKeys.listed (val.masterPublic)); - BEAST_EXPECT(trustedKeys.listed (val.signingPublic)); - } + item.server = std::make_unique ( + env.app().getIOService(), + pubSigningKeys, + manifest, + sequence, + expiration, + version, + item.list); + + std::stringstream uri; + uri << "http://" << item.server->local_endpoint() << cfg.first; + item.uri = uri.str(); } + + BEAST_EXPECT(trustedKeys.load ( + emptyLocalKey, emptyCfgKeys, cfgPublishers)); + + auto sites = std::make_unique ( + env.app().getIOService(), env.app().validators(), journal); + + std::vector uris; + for (auto const& u : servers) + uris.push_back(u.uri); + sites->load (uris); + sites->start(); + sites->join(); + + for (auto const& u : servers) { - // fetch multiple sites - std::vector cfgSites({ url1.str(), url2.str() }); - - auto sites = std::make_unique ( - env.app().getIOService(), env.app().validators(), journal); - - sites->load (cfgSites); - sites->start(); - sites->join(); - - for (auto const& val : list1) + for (auto const& val : u.list) { - BEAST_EXPECT(trustedKeys.listed (val.masterPublic)); - BEAST_EXPECT(trustedKeys.listed (val.signingPublic)); + BEAST_EXPECT( + trustedKeys.listed (val.masterPublic) != u.shouldFail); + BEAST_EXPECT( + trustedKeys.listed (val.signingPublic) != u.shouldFail); } - for (auto const& val : list2) + auto const jv = sites->getJson(); + Json::Value myStatus; + for (auto const& vs : jv[jss::validator_sites]) + if (vs[jss::uri].asString().find(u.uri) != std::string::npos) + myStatus = vs; + BEAST_EXPECTS( + myStatus[jss::last_refresh_message].asString().empty() + != u.shouldFail, to_string(myStatus)); + if (u.shouldFail) { - BEAST_EXPECT(trustedKeys.listed (val.masterPublic)); - BEAST_EXPECT(trustedKeys.listed (val.signingPublic)); + BEAST_EXPECTS( + sink.strm_.str().find(u.expectMsg) != std::string::npos, + sink.strm_.str()); + log << " -- Msg: " << + myStatus[jss::last_refresh_message].asString() << std::endl; } } } @@ -247,7 +257,42 @@ public: run() override { testConfigLoad (); - testFetchList (); + + // fetch single site + testFetchList ({{"/validators",""}}); + // fetch multiple sites + testFetchList ({{"/validators",""}, {"/validators",""}}); + // fetch single site with single redirects + testFetchList ({{"/redirect_once/301",""}}); + testFetchList ({{"/redirect_once/302",""}}); + testFetchList ({{"/redirect_once/307",""}}); + testFetchList ({{"/redirect_once/308",""}}); + // one redirect, one not + testFetchList ({{"/validators",""}, {"/redirect_once/302",""}}); + // fetch single site with undending redirect (fails to load) + testFetchList ({{"/redirect_forever/301", "Exceeded max redirects"}}); + // two that redirect forever + testFetchList ({ + {"/redirect_forever/307","Exceeded max redirects"}, + {"/redirect_forever/308","Exceeded max redirects"}}); + // one undending redirect, one not + testFetchList ( + {{"/validators",""}, + {"/redirect_forever/302","Exceeded max redirects"}}); + // invalid redir Location + testFetchList ({ + {"/redirect_to/ftp://invalid-url/302", + "Invalid redirect location"}}); + // invalid json + testFetchList ({{"/validators/bad", "Unable to parse JSON response"}}); + // error status returned + testFetchList ({{"/bad-resource", "returned bad status"}}); + // location field missing + testFetchList ({ + {"/redirect_nolo/308", "returned a redirect with no Location"}}); + // json fields missing + testFetchList ({ + {"/validators/missing", "Missing fields in JSON response"}}); } }; diff --git a/src/test/jtx/TrustedPublisherServer.h b/src/test/jtx/TrustedPublisherServer.h index 3fb916cda5..cd47cbd368 100644 --- a/src/test/jtx/TrustedPublisherServer.h +++ b/src/test/jtx/TrustedPublisherServer.h @@ -26,6 +26,7 @@ #include #include #include +#include #include namespace ripple { @@ -156,36 +157,65 @@ private: void do_peer(int id, socket_type&& sock0) { + using namespace boost::beast; socket_type sock(std::move(sock0)); - boost::beast::multi_buffer sb; + multi_buffer sb; error_code ec; for (;;) { req_type req; - boost::beast::http::read(sock, sb, req, ec); + http::read(sock, sb, req, ec); if (ec) break; auto path = req.target().to_string(); - if (path != "/validators") + resp_type res; + res.insert("Server", "TrustedPublisherServer"); + res.version(req.version()); + + if (boost::starts_with(path, "/validators")) { - resp_type res; + res.result(http::status::ok); + res.insert("Content-Type", "application/json"); + if (path == "/validators/bad") + res.body() = "{ 'bad': \"1']" ; + else if (path == "/validators/missing") + res.body() = "{\"version\": 1}"; + else + res.body() = list_; + } + else if (boost::starts_with(path, "/redirect")) + { + if (boost::ends_with(path, "/301")) + res.result(http::status::moved_permanently); + else if (boost::ends_with(path, "/302")) + res.result(http::status::found); + else if (boost::ends_with(path, "/307")) + res.result(http::status::temporary_redirect); + else if (boost::ends_with(path, "/308")) + res.result(http::status::permanent_redirect); + + std::stringstream location; + if (boost::starts_with(path, "/redirect_to/")) + { + location << path.substr(13); + } + else if (! boost::starts_with(path, "/redirect_nolo")) + { + location << "http://" << local_endpoint() << + (boost::starts_with(path, "/redirect_forever/") ? + path : "/validators"); + } + if (! location.str().empty()) + res.insert("Location", location.str()); + } + else + { + // unknown request res.result(boost::beast::http::status::not_found); - res.version(req.version()); - res.insert("Server", "TrustedPublisherServer"); res.insert("Content-Type", "text/html"); res.body() = "The file '" + path + "' was not found"; - res.prepare_payload(); - write(sock, res, ec); - if (ec) - break; } - resp_type res; - res.result(boost::beast::http::status::ok); - res.version(req.version()); - res.insert("Server", "TrustedPublisherServer"); - res.insert("Content-Type", "application/json"); - res.body() = list_; try { res.prepare_payload(); From 7c96bbafbdff287dae360a3a19061c9707f12e5e Mon Sep 17 00:00:00 2001 From: Mike Ellery Date: Thu, 11 Oct 2018 09:48:07 -0700 Subject: [PATCH 2/4] CI rpm build fix --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 0ecbd1ee41..443c4b5942 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -672,7 +672,7 @@ function error { exit 1 } -yum install -y yum-utils +yum install -y yum-utils openssl-static zlib-static rpm -i /opt/rippled-rpm/*.rpm rc=$?; if [[ $rc != 0 ]]; then error "error installing rpms" From 152d698957a9b0ba5b08088cc3a7db9af839fb4c Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 18 Oct 2018 02:11:46 -0700 Subject: [PATCH 3/4] Properly handle expired validator lists when validating (RIPD-1661): A validator that was configured to use a published validator list could exhibit aberrent behavior if that validator list expired. This commit introduces additional logic that makes validators operating with an expired validator list bow out of the consensus process instead of continuing to publish validations. Normal operation will resume once a non-expired validator list becomes available. This commit also enhances status reporting when using the `server_info` and `validators` commands. Before, only the expiration time of the list would be returned; now, its current status is also reported in a format that is clearer. --- src/ripple/app/consensus/RCLConsensus.cpp | 16 +++++++- src/ripple/app/misc/NetworkOPs.cpp | 43 ++++++++++++++++------ src/ripple/app/misc/ValidatorList.h | 4 ++ src/ripple/app/misc/impl/ValidatorList.cpp | 34 ++++++++++++++--- src/ripple/protocol/JsonFields.h | 1 + src/test/rpc/ValidatorRPC_test.cpp | 19 ++++++---- 6 files changed, 91 insertions(+), 26 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index b94dc6ee88..caccc07d20 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -812,12 +812,26 @@ RCLConsensus::peerProposal( bool RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr) { - // We have a key and do not want out of sync validations after a restart, + // We have a key, we do not want out of sync validations after a restart // and are not amendment blocked. validating_ = valPublic_.size() != 0 && prevLgr.seq() >= app_.getMaxDisallowedLedger() && !app_.getOPs().isAmendmentBlocked(); + // If we are not running in standalone mode and there's a configured UNL, + // check to make sure that it's not expired. + if (validating_ && !app_.config().standalone() && app_.validators().count()) + { + auto const when = app_.validators().expires(); + + if (!when || *when < app_.timeKeeper().now()) + { + JLOG(j_.error()) << "Voluntarily bowing out of consensus process " + "because of an expired validator list."; + validating_ = false; + } + } + const bool synced = app_.getOPs().getOperatingMode() == NetworkOPs::omFULL; if (validating_) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index ded313bb9d..ba77148ac3 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -2103,25 +2103,44 @@ Json::Value NetworkOPsImp::getServerInfo (bool human, bool admin, bool counters) if (admin) { - if (auto when = app_.validators().expires()) + auto when = app_.validators().expires(); + + if (!human) { - if (human) - { - if(*when == TimeKeeper::time_point::max()) - info[jss::validator_list_expires] = "never"; - else - info[jss::validator_list_expires] = to_string(*when); - } - else + if (when) info[jss::validator_list_expires] = static_cast(when->time_since_epoch().count()); + else + info[jss::validator_list_expires] = 0; } else { - if (human) - info[jss::validator_list_expires] = "unknown"; + auto& x = (info[jss::validator_list] = Json::objectValue); + + x[jss::count] = static_cast(app_.validators().count()); + + if (when) + { + if (*when == TimeKeeper::time_point::max()) + { + x[jss::expiration] = "never"; + x[jss::status] = "active"; + } + else + { + x[jss::expiration] = to_string(*when); + + if (*when > app_.timeKeeper().now()) + x[jss::status] = "active"; + else + x[jss::status] = "expired"; + } + } else - info[jss::validator_list_expires] = 0; + { + x[jss::status] = "unknown"; + x[jss::expiration] = "unknown"; + } } } info[jss::io_latency_ms] = static_cast ( diff --git a/src/ripple/app/misc/ValidatorList.h b/src/ripple/app/misc/ValidatorList.h index f304736d28..683e24ec89 100644 --- a/src/ripple/app/misc/ValidatorList.h +++ b/src/ripple/app/misc/ValidatorList.h @@ -331,6 +331,10 @@ public: for_each_listed ( std::function func) const; + /** Return the number of configured validator list sites. */ + std::size_t + count() const; + /** Return the time when the validator list will expire @note This may be a time in the past if a published list has not diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index d9470a4966..afc27f2967 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -459,6 +459,13 @@ ValidatorList::removePublisherList (PublicKey const& publisherKey) return true; } +std::size_t +ValidatorList::count() const +{ + std::shared_lock read_lock{mutex_}; + return publisherLists_.size(); +} + boost::optional ValidatorList::expires() const { @@ -486,19 +493,34 @@ ValidatorList::getJson() const res[jss::validation_quorum] = static_cast(quorum()); - if (auto when = expires()) { - if (*when == TimeKeeper::time_point::max()) + auto& x = (res[jss::validator_list] = Json::objectValue); + + x[jss::count] = static_cast(count()); + + if (auto when = expires()) { - res[jss::validator_list_expires] = "never"; + if (*when == TimeKeeper::time_point::max()) + { + x[jss::expiration] = "never"; + x[jss::status] = "active"; + } + else + { + x[jss::expiration] = to_string(*when); + + if (*when > timeKeeper_.now()) + x[jss::status] = "active"; + else + x[jss::status] = "expired"; + } } else { - res[jss::validator_list_expires] = to_string(*when); + x[jss::status] = "unknown"; + x[jss::expiration] = "unknown"; } } - else - res[jss::validator_list_expires] = "unknown"; // Local static keys PublicKey local; diff --git a/src/ripple/protocol/JsonFields.h b/src/ripple/protocol/JsonFields.h index 18a52695be..69acbb1081 100644 --- a/src/ripple/protocol/JsonFields.h +++ b/src/ripple/protocol/JsonFields.h @@ -483,6 +483,7 @@ JSS ( validate ); // in: DownloadShard JSS ( validated ); // out: NetworkOPs, RPCHelpers, AccountTx* // Tx JSS ( validator_list_expires ); // out: NetworkOps, ValidatorList +JSS ( validator_list ); // out: NetworkOps, ValidatorList JSS ( validated_ledger ); // out: NetworkOPs JSS ( validated_ledgers ); // out: NetworkOPs JSS ( validation_key ); // out: ValidationCreate, ValidationSeed diff --git a/src/test/rpc/ValidatorRPC_test.cpp b/src/test/rpc/ValidatorRPC_test.cpp index 67466dec32..5e9cd7058e 100644 --- a/src/test/rpc/ValidatorRPC_test.cpp +++ b/src/test/rpc/ValidatorRPC_test.cpp @@ -111,7 +111,7 @@ public: auto const jrr = env.rpc("server_info")[jss::result]; BEAST_EXPECT(jrr[jss::status] == "success"); BEAST_EXPECT(jrr[jss::info].isMember( - jss::validator_list_expires) == isAdmin); + jss::validator_list) == isAdmin); } { @@ -145,7 +145,8 @@ public: { auto const jrr = env.rpc("server_info")[jss::result]; BEAST_EXPECT( - jrr[jss::info][jss::validator_list_expires] == "never"); + jrr[jss::info][jss::validator_list][jss::expiration] == + "never"); } { auto const jrr = env.rpc("server_state")[jss::result]; @@ -156,7 +157,7 @@ public: // All our keys are in the response { auto const jrr = env.rpc("validators")[jss::result]; - BEAST_EXPECT(jrr[jss::validator_list_expires] == "never"); + BEAST_EXPECT(jrr[jss::validator_list][jss::expiration] == "never"); BEAST_EXPECT(jrr[jss::validation_quorum].asUInt() == keys.size()); BEAST_EXPECT(jrr[jss::trusted_validator_keys].size() == keys.size()); BEAST_EXPECT(jrr[jss::publisher_lists].size() == 0); @@ -226,7 +227,8 @@ public: { auto const jrr = env.rpc("server_info")[jss::result]; BEAST_EXPECT( - jrr[jss::info][jss::validator_list_expires] == "unknown"); + jrr[jss::info][jss::validator_list][jss::expiration] == + "unknown"); } { auto const jrr = env.rpc("server_state")[jss::result]; @@ -239,7 +241,8 @@ public: std::numeric_limits::max()); BEAST_EXPECT(jrr[jss::local_static_keys].size() == 0); BEAST_EXPECT(jrr[jss::trusted_validator_keys].size() == 0); - BEAST_EXPECT(jrr[jss::validator_list_expires] == "unknown"); + BEAST_EXPECT( + jrr[jss::validator_list][jss::expiration] == "unknown"); if (BEAST_EXPECT(jrr[jss::publisher_lists].size() == 1)) { @@ -312,7 +315,8 @@ public: { auto const jrr = env.rpc("server_info")[jss::result]; - BEAST_EXPECT(jrr[jss::info][jss::validator_list_expires] == + BEAST_EXPECT( + jrr[jss::info][jss::validator_list][jss::expiration] == to_string(expiration)); } { @@ -325,7 +329,8 @@ public: auto const jrr = env.rpc("validators")[jss::result]; BEAST_EXPECT(jrr[jss::validation_quorum].asUInt() == 2); BEAST_EXPECT( - jrr[jss::validator_list_expires] == to_string(expiration)); + jrr[jss::validator_list][jss::expiration] == + to_string(expiration)); BEAST_EXPECT(jrr[jss::local_static_keys].size() == 0); BEAST_EXPECT(jrr[jss::trusted_validator_keys].size() == From 72e6005f562a8f0818bc94803d222ac9345e1e40 Mon Sep 17 00:00:00 2001 From: seelabs Date: Fri, 19 Oct 2018 13:12:40 -0400 Subject: [PATCH 4/4] Set version to 1.1.1 --- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 4dbe223290..b9f0635afe 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -30,7 +30,7 @@ namespace BuildInfo { // The build version number. You must edit this for each release // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ -char const* const versionString = "1.1.0" +char const* const versionString = "1.1.1" #if defined(DEBUG) || defined(SANITIZER) "+"