Fix limit setting and add ServerImp tests (RIPD-1463,1458):

Add more test coverage for ServerHandlerImp.cpp. Ensure limit parameter
is propagated from parsed object to in-memory config.

Release Notes
-------------

This fixes a bug whereby the limit parameter on a port configuration was
ignored.

doc/rippled-example.cfg

@@ -172,6 +172,13 @@
 #       NOTE    If no ports support the peer protocol, rippled cannot
 #               receive incoming peer connections or become a superpeer.
 #
+#   limit = <number>
+#
+#       Optional. An integer value that will limit the number of connected
+#       clients that the port will accept. Once the limit is reached, new
+#       connections will be refused until other clients disconnect.
+#       Omit or set to 0 to allow unlimited numbers of clients.
+#
 #   user = <text>
 #   password = <text>
 #

src/ripple/rpc/impl/ServerHandlerImp.cpp

@@ -587,7 +587,7 @@ ServerHandlerImp::processRequest (Port const& port,
         return;
     }
 
-    if (! method)
+    if (method.isNull())
     {
         usage.charge(Resource::feeInvalidRPC);
         HTTPReply (400, "Null method", output, rpcJ);
@@ -822,6 +822,7 @@ to_Port(ParsedPort const& parsed, std::ostream& log)
     p.ssl_ciphers = parsed.ssl_ciphers;
     p.pmd_options = parsed.pmd_options;
     p.ws_queue_limit = parsed.ws_queue_limit;
+    p.limit = parsed.limit;
 
     return p;
 }

src/test/jtx/Env.h

@@ -158,6 +158,53 @@ all_features_except (uint256 const& key, Args const&... args)
         std::array<uint256, 1 + sizeof...(args)>{{key, args...}});
 }
 
+class SuiteSink : public beast::Journal::Sink
+{
+    std::string partition_;
+    beast::unit_test::suite& suite_;
+
+public:
+    SuiteSink(std::string const& partition,
+            beast::severities::Severity threshold,
+            beast::unit_test::suite& suite)
+        : Sink (threshold, false)
+        , partition_(partition + " ")
+        , suite_ (suite)
+    {
+    }
+
+    // For unit testing, always generate logging text.
+    inline bool active(beast::severities::Severity level) const override
+    {
+        return true;
+    }
+
+    void
+    write(beast::severities::Severity level, std::string const& text) override;
+};
+
+class SuiteLogs : public Logs
+{
+    beast::unit_test::suite& suite_;
+
+public:
+    explicit
+    SuiteLogs(beast::unit_test::suite& suite)
+        : Logs (beast::severities::kError)
+        , suite_(suite)
+    {
+    }
+
+    ~SuiteLogs() override = default;
+
+    std::unique_ptr<beast::Journal::Sink>
+    makeSink(std::string const& partition,
+        beast::severities::Severity threshold) override
+    {
+        return std::make_unique<SuiteSink>(partition, threshold, suite_);
+    }
+};
+
 //------------------------------------------------------------------------------
 
 /** A transaction testing environment. */
@@ -181,7 +228,8 @@ private:
         std::unique_ptr<AbstractClient> client;
 
         AppBundle (beast::unit_test::suite& suite,
-            std::unique_ptr<Config> config);
+            std::unique_ptr<Config> config,
+            std::unique_ptr<Logs> logs);
         ~AppBundle();
     };
 
@@ -208,9 +256,13 @@ public:
     // VFALCO Could wrap the suite::log in a Journal here
     Env (beast::unit_test::suite& suite_,
             std::unique_ptr<Config> config,
-            FeatureBitset features)
+            FeatureBitset features,
+            std::unique_ptr<Logs> logs = nullptr)
         : test (suite_)
-        , bundle_ (suite_, std::move(config))
+        , bundle_ (
+            suite_,
+            std::move(config),
+            logs ? std::move(logs) : std::make_unique<SuiteLogs>(suite_))
     {
         memoize(Account::master);
         Pathfinder::initPathTable();
@@ -251,8 +303,9 @@ public:
      * the pointer. See envconfig and related functions for common config tweaks.
      */
     Env (beast::unit_test::suite& suite_,
-        std::unique_ptr<Config> config)
-        : Env(suite_, std::move(config), all_amendments())
+        std::unique_ptr<Config> config,
+        std::unique_ptr<Logs> logs = nullptr)
+        : Env(suite_, std::move(config), all_amendments(), std::move(logs))
     {
     }
 

src/test/jtx/impl/Env.cpp

@@ -55,30 +55,8 @@ namespace ripple {
 namespace test {
 namespace jtx {
 
-class SuiteSink : public beast::Journal::Sink
-{
-    std::string partition_;
-    beast::unit_test::suite& suite_;
-
-public:
-    SuiteSink(std::string const& partition,
-            beast::severities::Severity threshold,
-            beast::unit_test::suite& suite)
-        : Sink (threshold, false)
-        , partition_(partition + " ")
-        , suite_ (suite)
-    {
-    }
-
-    // For unit testing, always generate logging text.
-    bool active(beast::severities::Severity level) const override
-    {
-        return true;
-    }
-
 void
-    write(beast::severities::Severity level,
-        std::string const& text) override
+SuiteSink::write(beast::severities::Severity level, std::string const& text)
 {
     using namespace beast::severities;
     std::string s;
@@ -97,39 +75,16 @@ public:
     if (level >= threshold())
         suite_.log << s << partition_ << text << std::endl;
 }
-};
-
-class SuiteLogs : public Logs
-{
-    beast::unit_test::suite& suite_;
-
-public:
-    explicit
-    SuiteLogs(beast::unit_test::suite& suite)
-        : Logs (beast::severities::kError)
-        , suite_(suite)
-    {
-    }
-
-    ~SuiteLogs() override = default;
-
-    std::unique_ptr<beast::Journal::Sink>
-    makeSink(std::string const& partition,
-        beast::severities::Severity threshold) override
-    {
-        return std::make_unique<SuiteSink>(partition, threshold, suite_);
-    }
-};
 
 //------------------------------------------------------------------------------
 
 Env::AppBundle::AppBundle(beast::unit_test::suite& suite,
-    std::unique_ptr<Config> config)
+    std::unique_ptr<Config> config,
+    std::unique_ptr<Logs> logs)
 {
     using namespace beast::severities;
     // Use kFatal threshold to reduce noise from STObject.
     setDebugLogSink (std::make_unique<SuiteSink>("Debug", kFatal, suite));
-    auto logs = std::make_unique<SuiteLogs>(suite);
     auto timeKeeper_ =
         std::make_unique<ManualTimeKeeper>();
     timeKeeper = timeKeeper_.get();

src/test/server/ServerStatus_test.cpp

@@ -20,6 +20,7 @@
 #include <BeastConfig.h>
 #include <ripple/rpc/ServerHandler.h>
 #include <ripple/json/json_reader.h>
+#include <ripple/app/misc/LoadFeeTrack.h>
 #include <test/jtx.h>
 #include <test/jtx/envconfig.h>
 #include <test/jtx/WSClient.h>
@@ -30,9 +31,11 @@
 #include <beast/http.hpp>
 #include <beast/test/yield_to.hpp>
 #include <beast/websocket/detail/mask.hpp>
+#include <beast/core/multi_buffer.hpp>
 #include <boost/asio.hpp>
 #include <boost/asio/ssl.hpp>
 #include <boost/algorithm/string/predicate.hpp>
+#include <regex>
 
 namespace ripple {
 namespace test {
@@ -40,6 +43,8 @@ namespace test {
 class ServerStatus_test :
     public beast::unit_test::suite, public beast::test::enable_yield_to
 {
+    class myFields : public beast::http::fields {};
+
     auto makeConfig(
         std::string const& proto,
         bool admin = true,
@@ -105,7 +110,8 @@ class ServerStatus_test :
     auto makeHTTPRequest(
         std::string const& host,
         uint16_t port,
-        std::string const& body)
+        std::string const& body,
+        myFields const& fields)
     {
         using namespace boost::asio;
         using namespace beast::http;
@@ -113,6 +119,8 @@ class ServerStatus_test :
 
         req.target("/");
         req.version = 11;
+        for(auto const& f : fields)
+            req.insert(f.name(), f.value());
         req.insert("Host", host + ":" + std::to_string(port));
         req.insert("User-Agent", "test");
         if(body.empty())
@@ -217,7 +225,8 @@ class ServerStatus_test :
         bool secure,
         beast::http::response<beast::http::string_body>& resp,
         boost::system::error_code& ec,
-        std::string const& body = "")
+        std::string const& body = "",
+        myFields const& fields = {})
     {
         auto const port = env.app().config()["port_rpc"].
             get<std::uint16_t>("port");
@@ -225,7 +234,7 @@ class ServerStatus_test :
             get<std::string>("ip");
         doRequest(
             yield,
-            makeHTTPRequest(*ip, *port, body),
+            makeHTTPRequest(*ip, *port, body, fields),
             *ip,
             *port,
             secure,
@@ -277,6 +286,91 @@ class ServerStatus_test :
 // ------------
 //  Test Cases
 // ------------
+
+    void
+    testAdminRequest(std::string const& proto, bool admin, bool credentials)
+    {
+        testcase << "Admin request over " << proto <<
+            ", config " << (admin ? "enabled" : "disabled") <<
+            ", credentials " << (credentials ? "" : "not ") << "set";
+        using namespace jtx;
+        Env env {*this, makeConfig(proto, admin, credentials)};
+
+        Json::Value jrr;
+        auto const proto_ws = boost::starts_with(proto, "w");
+
+        // the set of checks we do are different depending
+        // on how the admin config options are set
+
+        if(admin && credentials)
+        {
+            auto const user = env.app().config()
+                [proto_ws ? "port_ws" : "port_rpc"].
+                get<std::string>("admin_user");
+
+            auto const password = env.app().config()
+                [proto_ws ? "port_ws" : "port_rpc"].
+                get<std::string>("admin_password");
+
+            //1 - FAILS with wrong pass
+            jrr = makeAdminRequest(env, proto, *user, *password + "_")
+                [jss::result];
+            BEAST_EXPECT(jrr["error"] == proto_ws ? "forbidden" : "noPermission");
+            BEAST_EXPECT(jrr["error_message"] ==
+                proto_ws ?
+                "Bad credentials." :
+                "You don't have permission for this command.");
+
+            //2 - FAILS with password in an object
+            jrr = makeAdminRequest(env, proto, *user, *password, true)[jss::result];
+            BEAST_EXPECT(jrr["error"] == proto_ws ? "forbidden" : "noPermission");
+            BEAST_EXPECT(jrr["error_message"] ==
+                proto_ws ?
+                "Bad credentials." :
+                "You don't have permission for this command.");
+
+            //3 - FAILS with wrong user
+            jrr = makeAdminRequest(env, proto, *user + "_", *password)[jss::result];
+            BEAST_EXPECT(jrr["error"] == proto_ws ? "forbidden" : "noPermission");
+            BEAST_EXPECT(jrr["error_message"] ==
+                proto_ws ?
+                "Bad credentials." :
+                "You don't have permission for this command.");
+
+            //4 - FAILS no credentials
+            jrr = makeAdminRequest(env, proto, "", "")[jss::result];
+            BEAST_EXPECT(jrr["error"] == proto_ws ? "forbidden" : "noPermission");
+            BEAST_EXPECT(jrr["error_message"] ==
+                proto_ws ?
+                "Bad credentials." :
+                "You don't have permission for this command.");
+
+            //5 - SUCCEEDS with proper credentials
+            jrr = makeAdminRequest(env, proto, *user, *password)[jss::result];
+            BEAST_EXPECT(jrr["status"] == "success");
+        }
+        else if(admin)
+        {
+            //1 - SUCCEEDS with proper credentials
+            jrr = makeAdminRequest(env, proto, "u", "p")[jss::result];
+            BEAST_EXPECT(jrr["status"] == "success");
+
+            //2 - SUCCEEDS without proper credentials
+            jrr = makeAdminRequest(env, proto, "", "")[jss::result];
+            BEAST_EXPECT(jrr["status"] == "success");
+        }
+        else
+        {
+            //1 - FAILS - admin disabled
+            jrr = makeAdminRequest(env, proto, "", "")[jss::result];
+            BEAST_EXPECT(jrr["error"] == proto_ws ? "forbidden" : "noPermission");
+            BEAST_EXPECT(jrr["error_message"] ==
+                proto_ws ?
+                "Bad credentials." :
+                "You don't have permission for this command.");
+        }
+    }
+
     void
     testWSClientToHttpServer(boost::asio::yield_context& yield)
     {
@@ -430,89 +524,257 @@ class ServerStatus_test :
     }
 
     void
-    testAdminRequest(std::string const& proto, bool admin, bool credentials)
+    testAuth(bool secure, boost::asio::yield_context& yield)
     {
-        testcase << "Admin request over " << proto <<
-            ", config " << (admin ? "enabled" : "disabled") <<
-            ", credentials " << (credentials ? "" : "not ") << "set";
-        using namespace jtx;
-        Env env {*this, makeConfig(proto, admin, credentials)};
+        testcase << "Server with authorization, " <<
+            (secure ? "secure" : "non-secure");
 
-        auto const user = env.app().config()
-            [boost::starts_with(proto, "h") ? "port_rpc" : "port_ws"].
-            get<std::string>("admin_user");
+        using namespace test::jtx;
+        Env env {*this, envconfig([secure](std::unique_ptr<Config> cfg) {
+            (*cfg)["port_rpc"].set("user","me");
+            (*cfg)["port_rpc"].set("password","secret");
+            (*cfg)["port_rpc"].set("protocol", secure ? "https" : "http");
+            if (secure)
+                (*cfg)["port_ws"].set("protocol","http,ws");
+            return cfg;
+        })};
 
-        auto const password = env.app().config()
-            [boost::starts_with(proto, "h") ? "port_rpc" : "port_ws"].
-            get<std::string>("admin_password");
+        Json::Value jr;
+        jr[jss::method] = "server_info";
+        beast::http::response<beast::http::string_body> resp;
+        boost::system::error_code ec;
+        doHTTPRequest(env, yield, secure, resp, ec, to_string(jr));
+        BEAST_EXPECT(resp.result() == beast::http::status::forbidden);
 
-        Json::Value jrr;
+        myFields auth;
+        auth.insert("Authorization", "");
+        doHTTPRequest(env, yield, secure, resp, ec, to_string(jr), auth);
+        BEAST_EXPECT(resp.result() == beast::http::status::forbidden);
 
-        // the set of checks we do are different depending
-        // on how the admin config options are set
+        auth.set("Authorization", "Basic NOT-VALID");
+        doHTTPRequest(env, yield, secure, resp, ec, to_string(jr), auth);
+        BEAST_EXPECT(resp.result() == beast::http::status::forbidden);
 
-        if(admin && credentials)
-        {
-            //1 - FAILS with wrong pass
-            jrr = makeAdminRequest(env, proto, *user, *password + "_")[jss::result];
-            BEAST_EXPECT(jrr["error"] ==
-                boost::starts_with(proto, "h") ? "noPermission" : "forbidden");
-            BEAST_EXPECT(jrr["error_message"] ==
-                boost::starts_with(proto, "h") ?
-                "You don't have permission for this command." :
-                "Bad credentials.");
+        auth.set("Authorization", "Basic " + beast::detail::base64_encode("me:badpass"));
+        doHTTPRequest(env, yield, secure, resp, ec, to_string(jr), auth);
+        BEAST_EXPECT(resp.result() == beast::http::status::forbidden);
 
-            //2 - FAILS with password in an object
-            jrr = makeAdminRequest(env, proto, *user, *password, true)[jss::result];
-            BEAST_EXPECT(jrr["error"] ==
-                boost::starts_with(proto, "h") ? "noPermission" : "forbidden");
-            BEAST_EXPECT(jrr["error_message"] ==
-                boost::starts_with(proto, "h") ?
-                "You don't have permission for this command." :
-                "Bad credentials.");
+        auto const user = env.app().config().section("port_rpc").
+            get<std::string>("user").value();
+        auto const pass = env.app().config().section("port_rpc").
+            get<std::string>("password").value();
 
-            //3 - FAILS with wrong user
-            jrr = makeAdminRequest(env, proto, *user + "_", *password)[jss::result];
-            BEAST_EXPECT(jrr["error"] ==
-                boost::starts_with(proto, "h") ? "noPermission" : "forbidden");
-            BEAST_EXPECT(jrr["error_message"] ==
-                boost::starts_with(proto, "h") ?
-                "You don't have permission for this command." :
-                "Bad credentials.");
+        // try with the correct user/pass, but not encoded
+        auth.set("Authorization", "Basic " + user + ":" + pass);
+        doHTTPRequest(env, yield, secure, resp, ec, to_string(jr), auth);
+        BEAST_EXPECT(resp.result() == beast::http::status::forbidden);
 
-            //4 - FAILS no credentials
-            jrr = makeAdminRequest(env, proto, "", "")[jss::result];
-            BEAST_EXPECT(jrr["error"] ==
-                boost::starts_with(proto, "h") ? "noPermission" : "forbidden");
-            BEAST_EXPECT(jrr["error_message"] ==
-                boost::starts_with(proto, "h") ?
-                "You don't have permission for this command." :
-                "Bad credentials.");
-
-            //5 - SUCCEEDS with proper credentials
-            jrr = makeAdminRequest(env, proto, *user, *password)[jss::result];
-            BEAST_EXPECT(jrr["status"] == "success");
+        // finally if we use the correct user/pass encoded, we should get a 200
+        auth.set("Authorization", "Basic " +
+            beast::detail::base64_encode(user + ":" + pass));
+        doHTTPRequest(env, yield, secure, resp, ec, to_string(jr), auth);
+        BEAST_EXPECT(resp.result() == beast::http::status::ok);
+        BEAST_EXPECT(! resp.body.empty());
     }
-        else if(admin)
-        {
-            //1 - SUCCEEDS with proper credentials
-            jrr = makeAdminRequest(env, proto, "u", "p")[jss::result];
-            BEAST_EXPECT(jrr["status"] == "success");
 
-            //2 - SUCCEEDS without proper credentials
-            jrr = makeAdminRequest(env, proto, "", "")[jss::result];
-            BEAST_EXPECT(jrr["status"] == "success");
-        }
-        else
+    void
+    testLimit(boost::asio::yield_context& yield, int limit)
     {
-            //1 - FAILS - admin disabled
-            jrr = makeAdminRequest(env, proto, "", "")[jss::result];
-            BEAST_EXPECT(jrr["error"] ==
-                boost::starts_with(proto, "h") ? "noPermission" : "forbidden");
-            BEAST_EXPECT(jrr["error_message"] ==
-                boost::starts_with(proto, "h") ?
-                "You don't have permission for this command." :
-                "Bad credentials.");
+        testcase << "Server with connection limit of " << limit;
+
+        using namespace test::jtx;
+        using namespace boost::asio;
+        using namespace beast::http;
+        Env env {*this, envconfig([&](std::unique_ptr<Config> cfg) {
+            (*cfg)["port_rpc"].set("limit", to_string(limit));
+            return cfg;
+        })};
+
+
+        auto const port = env.app().config()["port_rpc"].
+            get<std::uint16_t>("port").value();
+        auto const ip = env.app().config()["port_rpc"].
+            get<std::string>("ip").value();
+
+        boost::system::error_code ec;
+        io_service& ios = get_io_service();
+        ip::tcp::resolver r{ios};
+
+        Json::Value jr;
+        jr[jss::method] = "server_info";
+
+        auto it =
+            r.async_resolve(
+                ip::tcp::resolver::query{ip, to_string(port)}, yield[ec]);
+        BEAST_EXPECT(! ec);
+
+        std::vector<std::pair<ip::tcp::socket, beast::multi_buffer>> clients;
+        int connectionCount {1}; //starts at 1 because the Env already has one
+                                 //for JSONRPCCLient
+
+        // for nonzero limits, go one past the limit, although failures happen
+        // at the limit, so this really leads to the last two clients failing.
+        // for zero limit, pick an arbitrary nonzero number of clients - all should
+        // connect fine.
+
+        int testTo = (limit == 0) ? 50 : limit + 1;
+        while (connectionCount < testTo)
+        {
+            clients.emplace_back(
+                std::make_pair(ip::tcp::socket {ios}, beast::multi_buffer{}));
+            async_connect(clients.back().first, it, yield[ec]);
+            BEAST_EXPECT(! ec);
+            auto req = makeHTTPRequest(ip, port, to_string(jr), {});
+            async_write(
+                clients.back().first,
+                req,
+                yield[ec]);
+            BEAST_EXPECT(! ec);
+            ++connectionCount;
+        }
+
+        int readCount = 0;
+        for (auto& c : clients)
+        {
+            beast::http::response<beast::http::string_body> resp;
+            async_read(c.first, c.second, resp, yield[ec]);
+            ++readCount;
+            // expect the reads to fail for the clients that connected at or
+            // above the limit. If limit is 0, all reads should succeed
+            BEAST_EXPECT((limit == 0 || readCount < limit-1) ? (! ec) : ec);
+        }
+    }
+
+    void
+    testWSHandoff(boost::asio::yield_context& yield)
+    {
+        testcase ("Connection with WS handoff");
+
+        using namespace test::jtx;
+        Env env {*this, envconfig([](std::unique_ptr<Config> cfg) {
+            (*cfg)["port_ws"].set("protocol","wss");
+            return cfg;
+        })};
+
+        auto const port = env.app().config()["port_ws"].
+            get<std::uint16_t>("port").value();
+        auto const ip = env.app().config()["port_ws"].
+            get<std::string>("ip").value();
+        beast::http::response<beast::http::string_body> resp;
+        boost::system::error_code ec;
+        doRequest(
+            yield, makeWSUpgrade(ip, port), ip, port, true, resp, ec);
+        BEAST_EXPECT(resp.result() == beast::http::status::switching_protocols);
+        BEAST_EXPECT(resp.find("Upgrade") != resp.end() &&
+               resp["Upgrade"] == "websocket");
+        BEAST_EXPECT(resp.find("Connection") != resp.end() &&
+               resp["Connection"] == "upgrade");
+    }
+
+    void
+    testNoRPC(boost::asio::yield_context& yield)
+    {
+        testcase ("Connection to port with no RPC enabled");
+
+        using namespace test::jtx;
+        Env env {*this};
+
+        auto const port = env.app().config()["port_ws"].
+            get<std::uint16_t>("port").value();
+        auto const ip = env.app().config()["port_ws"].
+            get<std::string>("ip").value();
+        beast::http::response<beast::http::string_body> resp;
+        boost::system::error_code ec;
+        // body content is required here to avoid being
+        // detected as a status request
+        doRequest(yield,
+            makeHTTPRequest(ip, port, "foo", {}), ip, port, false, resp, ec);
+        BEAST_EXPECT(resp.result() == beast::http::status::forbidden);
+        BEAST_EXPECT(resp.body == "Forbidden\r\n");
+    }
+
+    void
+    testWSRequests(boost::asio::yield_context& yield)
+    {
+        testcase ("WS client sends assorted input");
+
+        using namespace test::jtx;
+        using namespace boost::asio;
+        using namespace beast::http;
+        Env env {*this};
+
+        auto const port = env.app().config()["port_ws"].
+            get<std::uint16_t>("port").value();
+        auto const ip = env.app().config()["port_ws"].
+            get<std::string>("ip").value();
+        boost::system::error_code ec;
+
+        io_service& ios = get_io_service();
+        ip::tcp::resolver r{ios};
+
+        auto it =
+            r.async_resolve(
+                ip::tcp::resolver::query{ip, to_string(port)}, yield[ec]);
+        if(! BEAST_EXPECT(! ec))
+            return;
+
+        ip::tcp::socket sock{ios};
+        async_connect(sock, it, yield[ec]);
+        if(! BEAST_EXPECT(! ec))
+            return;
+
+        beast::websocket::stream<boost::asio::ip::tcp::socket&> ws{sock};
+        ws.handshake(ip + ":" + to_string(port), "/");
+
+        // helper lambda, used below
+        auto sendAndParse = [&](std::string const& req) -> Json::Value
+        {
+            ws.async_write_frame(true, buffer(req), yield[ec]);
+            if(! BEAST_EXPECT(! ec))
+                return Json::objectValue;
+
+            beast::multi_buffer sb;
+            ws.async_read(sb, yield[ec]);
+            if(! BEAST_EXPECT(! ec))
+                return Json::objectValue;
+
+            Json::Value resp;
+            Json::Reader jr;
+            if(! BEAST_EXPECT(jr.parse(
+                boost::lexical_cast<std::string>(
+                    beast::buffers(sb.data())), resp)))
+                return Json::objectValue;
+            sb.consume(sb.size());
+            return resp;
+        };
+
+        { // send invalid json
+            auto resp = sendAndParse("NOT JSON");
+            BEAST_EXPECT(resp.isMember(jss::error) &&
+                resp[jss::error] == "jsonInvalid");
+            BEAST_EXPECT(! resp.isMember(jss::status));
+        }
+
+        { // send incorrect json (method and command fields differ)
+            Json::Value jv;
+            jv[jss::command] = "foo";
+            jv[jss::method] = "bar";
+            auto resp = sendAndParse(to_string(jv));
+            BEAST_EXPECT(resp.isMember(jss::error) &&
+                resp[jss::error] == "missingCommand");
+            BEAST_EXPECT(resp.isMember(jss::status) &&
+                resp[jss::status] == "error");
+        }
+
+        { // send a ping (not an error)
+            Json::Value jv;
+            jv[jss::command] = "ping";
+            auto resp = sendAndParse(to_string(jv));
+            BEAST_EXPECT(resp.isMember(jss::status) &&
+                resp[jss::status] == "success");
+            BEAST_EXPECT(resp.isMember(jss::result) &&
+                resp[jss::result].isMember(jss::role) &&
+                resp[jss::result][jss::role] == "admin");
         }
     }
 
@@ -554,7 +816,7 @@ class ServerStatus_test :
 
         doRequest(
             yield,
-            makeHTTPRequest(*ip_ws, *port_ws, ""),
+            makeHTTPRequest(*ip_ws, *port_ws, "", {}),
             *ip_ws,
             *port_ws,
             false,
@@ -586,7 +848,7 @@ class ServerStatus_test :
         // being enabled
         doRequest(
             yield,
-            makeHTTPRequest(*ip_ws, *port_ws, ""),
+            makeHTTPRequest(*ip_ws, *port_ws, "", {}),
             *ip_ws,
             *port_ws,
             false,
@@ -603,7 +865,7 @@ class ServerStatus_test :
 
         doRequest(
             yield,
-            makeHTTPRequest(*ip_ws, *port_ws, ""),
+            makeHTTPRequest(*ip_ws, *port_ws, "", {}),
             *ip_ws,
             *port_ws,
             false,
@@ -617,35 +879,128 @@ class ServerStatus_test :
             resp.body.find("cannot accept clients:") != std::string::npos);
         BEAST_EXPECT(
             resp.body.find("Server version too old") != std::string::npos);
+    }
 
+    void
+    testRPCRequests(boost::asio::yield_context& yield)
+    {
+        testcase ("RPC client sends assorted input");
+
+        using namespace test::jtx;
+        Env env {*this};
+
+        boost::system::error_code ec;
+        {
+            beast::http::response<beast::http::string_body> resp;
+            doHTTPRequest(env, yield, false, resp, ec, "{}");
+            BEAST_EXPECT(resp.result() == beast::http::status::bad_request);
+            BEAST_EXPECT(resp.body == "Unable to parse request\r\n");
+        }
+
+        Json::Value jv;
+        {
+            beast::http::response<beast::http::string_body> resp;
+            jv[jss::method] = Json::nullValue;
+            doHTTPRequest(env, yield, false, resp, ec, to_string(jv));
+            BEAST_EXPECT(resp.result() == beast::http::status::bad_request);
+            BEAST_EXPECT(resp.body == "Null method\r\n");
+        }
+
+        {
+            beast::http::response<beast::http::string_body> resp;
+            jv[jss::method] = 1;
+            doHTTPRequest(env, yield, false, resp, ec, to_string(jv));
+            BEAST_EXPECT(resp.result() == beast::http::status::bad_request);
+            BEAST_EXPECT(resp.body == "method is not string\r\n");
+        }
+
+        {
+            beast::http::response<beast::http::string_body> resp;
+            jv[jss::method] = "";
+            doHTTPRequest(env, yield, false, resp, ec, to_string(jv));
+            BEAST_EXPECT(resp.result() == beast::http::status::bad_request);
+            BEAST_EXPECT(resp.body == "method is empty\r\n");
+        }
+
+        {
+            beast::http::response<beast::http::string_body> resp;
+            jv[jss::method] = "some_method";
+            jv[jss::params] = "params";
+            doHTTPRequest(env, yield, false, resp, ec, to_string(jv));
+            BEAST_EXPECT(resp.result() == beast::http::status::bad_request);
+            BEAST_EXPECT(resp.body == "params unparseable\r\n");
+        }
+
+        {
+            beast::http::response<beast::http::string_body> resp;
+            jv[jss::params] = Json::arrayValue;
+            jv[jss::params][0u] = "not an object";
+            doHTTPRequest(env, yield, false, resp, ec, to_string(jv));
+            BEAST_EXPECT(resp.result() == beast::http::status::bad_request);
+            BEAST_EXPECT(resp.body == "params unparseable\r\n");
+        }
+    }
+
+    void
+    testStatusNotOkay(boost::asio::yield_context& yield)
+    {
+        testcase ("Server status not okay");
+
+        using namespace test::jtx;
+        Env env {*this, envconfig([](std::unique_ptr<Config> cfg) {
+            cfg->ELB_SUPPORT = true;
+            return cfg;
+        })};
+
+        //raise the fee so that the server is considered overloaded
+        env.app().getFeeTrack().raiseLocalFee();
+
+        beast::http::response<beast::http::string_body> resp;
+        boost::system::error_code ec;
+        doHTTPRequest(env, yield, false, resp, ec);
+        BEAST_EXPECT(resp.result() == beast::http::status::internal_server_error);
+        std::regex body {"Server cannot accept clients"};
+        BEAST_EXPECT(std::regex_search(resp.body, body));
     }
 
 public:
     void
     run()
     {
-        yield_to([&](boost::asio::yield_context& yield)
-        {
-            testWSClientToHttpServer(yield);
-            testStatusRequest(yield);
-            testTruncatedWSUpgrade(yield);
-            // these are secure/insecure protocol pairs, i.e. for
-            // each item, the second value is the secure or insecure equivalent
-            testCantConnect("ws", "wss", yield);
-            testCantConnect("ws2", "wss2", yield);
-            testCantConnect("http", "https", yield);
-            //THIS HANGS - testCantConnect("wss", "ws", yield);
-            testCantConnect("wss2", "ws2", yield);
-            testCantConnect("https", "http", yield);
-            testAmendmentBlock(yield);
-        });
-
         for (auto it : {"http", "ws", "ws2"})
         {
             testAdminRequest (it, true, true);
             testAdminRequest (it, true, false);
             testAdminRequest (it, false, false);
         }
+
+        yield_to([&](boost::asio::yield_context& yield)
+        {
+            testWSClientToHttpServer (yield);
+            testStatusRequest (yield);
+            testTruncatedWSUpgrade (yield);
+
+            // these are secure/insecure protocol pairs, i.e. for
+            // each item, the second value is the secure or insecure equivalent
+            testCantConnect ("ws", "wss", yield);
+            testCantConnect ("ws2", "wss2", yield);
+            testCantConnect ("http", "https", yield);
+            testCantConnect ("wss", "ws", yield);
+            testCantConnect ("wss2", "ws2", yield);
+            testCantConnect ("https", "http", yield);
+
+            testAmendmentBlock(yield);
+            testAuth (false, yield);
+            testAuth (true, yield);
+            testLimit (yield, 5);
+            testLimit (yield, 0);
+            testWSHandoff (yield);
+            testNoRPC (yield);
+            testWSRequests (yield);
+            testRPCRequests (yield);
+            testStatusNotOkay (yield);
+        });
+
     };
 };
 

src/test/server/Server_test.cpp

@@ -23,6 +23,9 @@
 #include <ripple/server/Server.h>
 #include <ripple/server/Session.h>
 #include <ripple/beast/unit_test.h>
+#include <ripple/core/ConfigSections.h>
+#include <test/jtx.h>
+#include <test/jtx/envconfig.h>
 #include <boost/asio.hpp>
 #include <boost/optional.hpp>
 #include <boost/utility/in_place_factory.hpp>
@@ -367,11 +370,186 @@ public:
         }
     }
 
+    /**
+     * @brief sink for writing all log messages to a stringstream
+     */
+    class CaptureSink : public beast::Journal::Sink
+    {
+        std::stringstream& strm_;
+    public:
+        CaptureSink(beast::severities::Severity threshold,
+            std::stringstream& strm)
+        : beast::Journal::Sink(threshold, false)
+        , strm_(strm)
+        {
+        }
+
+        void
+        write(beast::severities::Severity level, std::string const& text) override
+        {
+            strm_ << text;
+        }
+    };
+
+    /**
+     * @brief Log manager for CaptureSinks. This class holds the stream
+     * instance that is written to by the sinks. Upon destruction, all
+     * contents of the stream are assigned to the string specified in the
+     * ctor
+     */
+    class CaptureLogs : public Logs
+    {
+        std::stringstream strm_;
+        std::string& result_;
+
+    public:
+        CaptureLogs(std::string& result)
+            : Logs (beast::severities::kInfo)
+            , result_(result)
+        {
+        }
+
+        ~CaptureLogs() override
+        {
+            result_ = strm_.str();
+        }
+
+        std::unique_ptr<beast::Journal::Sink>
+        makeSink(std::string const& partition,
+            beast::severities::Severity threshold) override
+        {
+            return std::make_unique<CaptureSink>(threshold, strm_);
+        }
+    };
+
+    void
+    testBadConfig ()
+    {
+        testcase ("Server config - invalid options");
+        using namespace test::jtx;
+
+        std::string messages;
+
+        except ([&]
+        {
+            Env env {*this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    (*cfg).deprecatedClearSection("port_rpc");
+                    return cfg;
+                }),
+                std::make_unique<CaptureLogs>(messages)};
+        });
+        BEAST_EXPECT (
+            messages.find ("Missing 'ip' in [port_rpc]")
+            != std::string::npos);
+
+        except ([&]
+        {
+            Env env {*this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    (*cfg).deprecatedClearSection("port_rpc");
+                    (*cfg)["port_rpc"].set("ip", "127.0.0.1");
+                    return cfg;
+                }),
+                std::make_unique<CaptureLogs>(messages)};
+        });
+        BEAST_EXPECT (
+            messages.find ("Missing 'port' in [port_rpc]")
+            != std::string::npos);
+
+        except ([&]
+        {
+            Env env {*this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    (*cfg).deprecatedClearSection("port_rpc");
+                    (*cfg)["port_rpc"].set("ip", "127.0.0.1");
+                    (*cfg)["port_rpc"].set("port", "0");
+                    return cfg;
+                }),
+                std::make_unique<CaptureLogs>(messages)};
+        });
+        BEAST_EXPECT (
+            messages.find ("Invalid value '0' for key 'port' in [port_rpc]")
+            != std::string::npos);
+
+        except ([&]
+        {
+            Env env {*this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    (*cfg).deprecatedClearSection("port_rpc");
+                    (*cfg)["port_rpc"].set("ip", "127.0.0.1");
+                    (*cfg)["port_rpc"].set("port", "8081");
+                    (*cfg)["port_rpc"].set("protocol", "");
+                    return cfg;
+                }),
+                std::make_unique<CaptureLogs>(messages)};
+        });
+        BEAST_EXPECT (
+            messages.find ("Missing 'protocol' in [port_rpc]")
+            != std::string::npos);
+
+        except ([&] //this creates a standard test config without the server
+                    //section
+        {
+            Env env {*this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    cfg = std::make_unique<Config>();
+                    cfg->overwrite (
+                        ConfigSection::nodeDatabase (), "type", "memory");
+                    cfg->overwrite (
+                        ConfigSection::nodeDatabase (), "path", "main");
+                    cfg->deprecatedClearSection (
+                        ConfigSection::importNodeDatabase ());
+                    cfg->legacy("database_path", "");
+                    cfg->setupControl(true, true, true);
+                    (*cfg)["port_peer"].set("ip", "127.0.0.1");
+                    (*cfg)["port_peer"].set("port", "8080");
+                    (*cfg)["port_peer"].set("protocol", "peer");
+                    (*cfg)["port_rpc"].set("ip", "127.0.0.1");
+                    (*cfg)["port_rpc"].set("port", "8081");
+                    (*cfg)["port_rpc"].set("protocol", "http,ws2");
+                    (*cfg)["port_rpc"].set("admin", "127.0.0.1");
+                    (*cfg)["port_ws"].set("ip", "127.0.0.1");
+                    (*cfg)["port_ws"].set("port", "8082");
+                    (*cfg)["port_ws"].set("protocol", "ws");
+                    (*cfg)["port_ws"].set("admin", "127.0.0.1");
+                    return cfg;
+                }),
+                std::make_unique<CaptureLogs>(messages)};
+        });
+        BEAST_EXPECT (
+            messages.find ("Required section [server] is missing")
+            != std::string::npos);
+
+        except ([&] //this creates a standard test config without some of the
+                    //port sections
+        {
+            Env env {*this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    cfg = std::make_unique<Config>();
+                    cfg->overwrite (ConfigSection::nodeDatabase (), "type", "memory");
+                    cfg->overwrite (ConfigSection::nodeDatabase (), "path", "main");
+                    cfg->deprecatedClearSection (ConfigSection::importNodeDatabase ());
+                    cfg->legacy("database_path", "");
+                    cfg->setupControl(true, true, true);
+                    (*cfg)["server"].append("port_peer");
+                    (*cfg)["server"].append("port_rpc");
+                    (*cfg)["server"].append("port_ws");
+                    return cfg;
+                }),
+                std::make_unique<CaptureLogs>(messages)};
+        });
+        BEAST_EXPECT (
+            messages.find ("Missing section: [port_peer]")
+            != std::string::npos);
+    }
+
     void
     run()
     {
         basicTests();
         stressTest();
+        testBadConfig();
     }
 };