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

@@ -27,6 +27,7 @@
 #include <ripple/json/json_value.h>
 #include <boost/asio.hpp>
 #include <mutex>
+#include <memory>
 
 namespace ripple {
 
@@ -73,10 +74,32 @@ private:
         {
             clock_type::time_point refreshed;
             ListDisposition disposition;
+            std::string message;
         };
 
-        std::string uri;
-        parsedURL pUrl;
+        struct Resource
+        {
+            explicit Resource(std::string uri_);
+            const std::string uri;
+            parsedURL pUrl;
+        };
+
+        explicit Site(std::string uri);
+
+        /// the original uri as loaded from config
+        std::shared_ptr<Resource> loadedResource;
+
+        /// the resource to request at <timer>
+        /// intervals. same as loadedResource
+        /// except in the case of a permanent redir.
+        std::shared_ptr<Resource> startingResource;
+
+        /// the active resource being requested.
+        /// same as startingResource except
+        /// when we've gotten a temp redirect
+        std::shared_ptr<Resource> activeResource;
+
+        unsigned short redirCount;
         std::chrono::minutes refreshInterval;
         clock_type::time_point nextRefresh;
         boost::optional<Status> lastRefreshStatus;
@@ -176,6 +199,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 (
+        std::shared_ptr<Site::Resource> resource,
+        std::size_t siteIdx,
+        std::lock_guard<std::mutex>& 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<std::mutex>& lock);
+
+    /// Interpret a redirect response.
+    /// lock over sites_mutex_ required
+    std::shared_ptr<Site::Resource>
+    processRedirect (
+        detail::response_type& res,
+        std::size_t siteIdx,
+        std::lock_guard<std::mutex>& lock);
 };
 
 } // ripple

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 uri_)
+    : uri {std::move(uri_)}
+{
+    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<Resource>(std::move(uri))}
+    , startingResource {loadedResource}
+    , redirCount {0}
+    , refreshInterval {DEFAULT_REFRESH_INTERVAL}
+    , nextRefresh {clock_type::now()}
+{
+}
 
 ValidatorSite::ValidatorSite (
     boost::asio::io_service& ios,
@@ -72,22 +96,18 @@ ValidatorSite::load (
 
     std::lock_guard <std::mutex> lock{sites_mutex_};
 
-    for (auto uri : siteURIs)
+    for (auto const& 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 (
+    std::shared_ptr<Site::Resource> resource,
+    std::size_t siteIdx,
+    std::lock_guard<std::mutex>& lock)
+{
+    fetching_ = true;
+    sites_[siteIdx].activeResource = resource;
+    std::shared_ptr<detail::Work> 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<detail::WorkSSL>(
+            resource->pUrl.domain,
+            resource->pUrl.path,
+            std::to_string(*resource->pUrl.port),
+            ios_,
+            j_,
+            onFetch);
+    }
+    else
+    {
+        sp = std::make_shared<detail::WorkPlain>(
+            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,156 @@ ValidatorSite::onTimer (
 
     std::lock_guard <std::mutex> lock{sites_mutex_};
     sites_[siteIdx].nextRefresh =
-        clock_type::now() + DEFAULT_REFRESH_INTERVAL;
+        clock_type::now() + sites_[siteIdx].refreshInterval;
 
     assert(! fetching_);
-    fetching_ = true;
-
-    std::shared_ptr<detail::Work> sp;
-    if (sites_[siteIdx].pUrl.scheme == "https")
+    sites_[siteIdx].redirCount = 0;
+    try
     {
-        sp = std::make_shared<detail::WorkSSL>(
-            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);
-            });
+        // the WorkSSL client can throw if SSL init fails
+        makeRequest(sites_[siteIdx].startingResource, siteIdx, lock);
+    }
+    catch (std::exception &)
+    {
+        onSiteFetch(
+            boost::system::error_code {-1, boost::system::generic_category()},
+            detail::response_type {},
+            siteIdx);
+    }
+}
+
+void
+ValidatorSite::parseJsonResponse (
+    detail::response_type& res,
+    std::size_t siteIdx,
+    std::lock_guard<std::mutex>& lock)
+{
+    Json::Reader r;
+    Json::Value body;
+    if (! r.parse(res.body().data(), body))
+    {
+        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<detail::WorkPlain>(
-            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 ()};
+    }
+}
+
+std::shared_ptr<ValidatorSite::Site::Resource>
+ValidatorSite::processRedirect (
+    detail::response_type& res,
+    std::size_t siteIdx,
+    std::lock_guard<std::mutex>& lock)
+{
+    using namespace boost::beast::http;
+    std::shared_ptr<Site::Resource> 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<Site::Resource>(
+            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 +382,74 @@ ValidatorSite::onSiteFetch(
     detail::response_type&& res,
     std::size_t siteIdx)
 {
-    if (! ec && res.result() != boost::beast::http::status::ok)
+    bool shouldRetry = false;
     {
-        std::lock_guard <std::mutex> 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 <std::mutex> 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 <std::mutex> 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;
-            }
-            else
-            {
-                BOOST_ASSERT(false);
+                    "Problem retrieving from " <<
+                    sites_[siteIdx].activeResource->uri <<
+                    " " <<
+                    ec.value() <<
+                    ":" <<
+                    ec.message();
+                shouldRetry = true;
+                throw std::runtime_error{"fetch error"};
             }
 
-            if (body.isMember ("refresh_interval") &&
-                body["refresh_interval"].isNumeric ())
+            using namespace boost::beast::http;
+            switch (res.result())
             {
-                sites_[siteIdx].refreshInterval =
-                    std::chrono::minutes{body["refresh_interval"].asUInt ()};
+            case status::ok:
+                parseJsonResponse(res, siteIdx, lock_sites);
+                break;
+            case status::moved_permanently :
+            case status::permanent_redirect :
+            case status::found :
+            case status::temporary_redirect :
+            {
+                auto newLocation = processRedirect (res, siteIdx, lock_sites);
+                assert(newLocation);
+                // for perm redirects, also update our starting URI
+                if (res.result() == status::moved_permanently ||
+                    res.result() == status::permanent_redirect)
+                {
+                    sites_[siteIdx].startingResource = newLocation;
+                }
+                makeRequest(newLocation, siteIdx, lock_sites);
+                return; // we are still fetching, so skip
+                        // state update/notify below
             }
+            default:
+            {
+                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"};
+            }
+            }
+
         }
-        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 <std::mutex> 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 <std::mutex> lock{state_mutex_};
+    std::lock_guard <std::mutex> lock_state{state_mutex_};
     fetching_ = false;
     if (! stopping_)
         setTimer ();
@@ -331,15 +469,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<Int>(site.refreshInterval.count());
         }

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

src/test/app/ValidatorSite_test.cpp

@@ -23,11 +23,13 @@
 #include <ripple/basics/strHex.h>
 #include <ripple/protocol/digest.h>
 #include <ripple/protocol/HashPrefix.h>
+#include <ripple/protocol/JsonFields.h>
 #include <ripple/protocol/PublicKey.h>
 #include <ripple/protocol/SecretKey.h>
 #include <ripple/protocol/Sign.h>
 #include <test/jtx.h>
 #include <test/jtx/TrustedPublisherServer.h>
+#include <boost/algorithm/string/predicate.hpp>
 #include <boost/asio.hpp>
 
 namespace ripple {
@@ -121,121 +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<std::pair<std::string, std::string>> 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 ();
 
+        TestSink sink;
+        beast::Journal journal{sink};
+
         PublicKey emptyLocalKey;
         std::vector<std::string> 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<std::string> cfgPublishers({
-            strHex(publisherPublic1),
-            strHex(publisherPublic2)});
-
-        BEAST_EXPECT(trustedKeys.load (
-            emptyLocalKey, emptyCfgKeys, cfgPublishers));
-
-        auto constexpr listSize = 20;
-        std::vector<Validator> list1;
-        list1.reserve (listSize);
-        while (list1.size () < listSize)
-            list1.push_back (randomValidator());
-
-        std::vector<Validator> list2;
-        list2.reserve (listSize);
-        while (list2.size () < listSize)
-            list2.push_back (randomValidator());
+        struct publisher
+        {
+            std::unique_ptr<TrustedPublisherServer> server;
+            std::vector<Validator> list;
+            std::string uri;
+            std::string expectMsg;
+            bool shouldFail;
+        };
+        std::vector<publisher> 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<std::string> 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<std::string> 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<ValidatorSite> (
-                env.app().getIOService(), env.app().validators(), env.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<TrustedPublisherServer> (
+                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<ValidatorSite> (
+            env.app().getIOService(), env.app().validators(), journal);
+
+        std::vector<std::string> 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<std::string> cfgSites({ url1.str(), url2.str() });
-
-            auto sites = std::make_unique<ValidatorSite> (
-                env.app().getIOService(), env.app().validators(), env.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;
             }
         }
     }
@@ -245,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"}});
     }
 };
 

src/test/jtx/TrustedPublisherServer.h

@@ -26,6 +26,7 @@
 #include <ripple/basics/strHex.h>
 #include <test/jtx/envconfig.h>
 #include <boost/asio.hpp>
+#include <boost/algorithm/string/predicate.hpp>
 #include <boost/beast/http.hpp>
 
 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();