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/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,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<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 ();
 
-        beast::Journal journal;
+        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(), 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(), 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"}});
     }
 };
 

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();