Allow port numbers be be specified with a colon: (#4328)

Port numbers can now be specified using either a colon or a space.

Examples:

1.2.3.4:51235

1.2.3.4 51235

- In the configuration file, an annoying "gotcha" for node operators is
  accidentally specifying IP:PORT combinations using a colon. The code
  previously expected a space, not a colon. It also does not provide
  good feedback when this operator error is made.
- This change simply allows this mistake (using a colon) to be fixed
  automatically, preserving the intention of the operator.
- Add unit tests, which test the functionality when specifying IP:PORT
  in the configuration file.
- The RPCCall test regime is not specific enough to test this
  functionality, it has been tested by hand.
- Ensure IPv6 addresses are not confused for ip:port

---------

Co-authored-by: Elliot Lee <[email protected]>

src/ripple/core/impl/Config.cpp

@@ -37,6 +37,7 @@
 #include <cstdlib>
 #include <iostream>
 #include <iterator>
+#include <regex>
 #include <thread>
 
 #if BOOST_OS_WINDOWS
@@ -468,6 +469,28 @@ Config::loadFromString(std::string const& fileContents)
     if (auto s = getIniFileSection(secConfig, SECTION_SNTP))
         SNTP_SERVERS = *s;
 
+    // if the user has specified ip:port then replace : with a space.
+    {
+        auto replaceColons = [](std::vector<std::string>& strVec) {
+            const static std::regex e(":([0-9]+)$");
+            for (auto& line : strVec)
+            {
+                // skip anything that might be an ipv6 address
+                if (std::count(line.begin(), line.end(), ':') != 1)
+                    continue;
+
+                std::string result = std::regex_replace(line, e, " $1");
+                // sanity check the result of the replace, should be same length
+                // as input
+                if (result.size() == line.size())
+                    line = result;
+            }
+        };
+
+        replaceColons(IPS_FIXED);
+        replaceColons(IPS);
+    }
+
     {
         std::string dbPath;
         if (getSingleSection(secConfig, "database_path", dbPath, j_))

src/ripple/net/impl/RPCCall.cpp

@@ -482,17 +482,31 @@ class RPCParser
         return jvRequest;
     }
 
-    // connect <ip> [port]
+    // connect <ip[:port]> [port]
     Json::Value
     parseConnect(Json::Value const& jvParams)
     {
         Json::Value jvRequest(Json::objectValue);
-
-        jvRequest[jss::ip] = jvParams[0u].asString();
-
+        std::string ip = jvParams[0u].asString();
         if (jvParams.size() == 2)
+        {
+            jvRequest[jss::ip] = ip;
             jvRequest[jss::port] = jvParams[1u].asUInt();
+            return jvRequest;
+        }
+
+        // handle case where there is one argument of the form ip:port
+        if (std::count(ip.begin(), ip.end(), ':') == 1)
+        {
+            std::size_t colon = ip.find_last_of(":");
+            jvRequest[jss::ip] = std::string{ip, 0, colon};
+            jvRequest[jss::port] =
+                Json::Value{std::string{ip, colon + 1}}.asUInt();
+            return jvRequest;
+        }
 
+        // default case, no port
+        jvRequest[jss::ip] = ip;
         return jvRequest;
     }
 

src/ripple/rpc/handlers/Connect.cpp

@@ -59,13 +59,15 @@ doConnect(RPC::JsonContext& context)
     else
         iPort = DEFAULT_PEER_PORT;
 
-    auto ip =
-        beast::IP::Endpoint::from_string(context.params[jss::ip].asString());
+    auto const ip_str = context.params[jss::ip].asString();
+    auto ip = beast::IP::Endpoint::from_string(ip_str);
 
     if (!is_unspecified(ip))
         context.app.overlay().connect(ip.at_port(iPort));
 
-    return RPC::makeObjectValue("connecting");
+    return RPC::makeObjectValue(
+        "attempting connection to IP:" + ip_str +
+        " port: " + std::to_string(iPort));
 }
 
 }  // namespace ripple

src/test/core/Config_test.cpp

@@ -856,6 +856,84 @@ r.ripple.com 51235
             cfg.section(SECTION_IPS_FIXED).values().size() == 2);
     }
 
+    void
+    testColons()
+    {
+        Config cfg;
+        /* NOTE: this string includes some explicit
+         * space chars in order to verify proper trimming */
+        std::string toLoad(R"(
+[port_rpc])"
+                           "\x20"
+                           R"(
+# comment
+    # indented comment
+)"
+                           "\x20\x20"
+                           R"(
+[ips])"
+                           "\x20"
+                           R"(
+r.ripple.com:51235
+
+  [ips_fixed])"
+                           "\x20\x20"
+                           R"(
+    # COMMENT
+    s1.ripple.com:51235
+    s2.ripple.com 51235
+    anotherserversansport
+    anotherserverwithport:12
+    1.1.1.1:1
+    1.1.1.1 1
+    12.34.12.123:12345
+    12.34.12.123 12345
+    ::
+    2001:db8::
+    ::1
+    ::1:12345
+    [::1]:12345
+    2001:db8:3333:4444:5555:6666:7777:8888:12345
+    [2001:db8:3333:4444:5555:6666:7777:8888]:1
+
+
+)");
+        cfg.loadFromString(toLoad);
+        BEAST_EXPECT(
+            cfg.exists("port_rpc") && cfg.section("port_rpc").lines().empty() &&
+            cfg.section("port_rpc").values().empty());
+        BEAST_EXPECT(
+            cfg.exists(SECTION_IPS) &&
+            cfg.section(SECTION_IPS).lines().size() == 1 &&
+            cfg.section(SECTION_IPS).values().size() == 1);
+        BEAST_EXPECT(
+            cfg.exists(SECTION_IPS_FIXED) &&
+            cfg.section(SECTION_IPS_FIXED).lines().size() == 15 &&
+            cfg.section(SECTION_IPS_FIXED).values().size() == 15);
+        BEAST_EXPECT(cfg.IPS[0] == "r.ripple.com 51235");
+
+        BEAST_EXPECT(cfg.IPS_FIXED[0] == "s1.ripple.com 51235");
+        BEAST_EXPECT(cfg.IPS_FIXED[1] == "s2.ripple.com 51235");
+        BEAST_EXPECT(cfg.IPS_FIXED[2] == "anotherserversansport");
+        BEAST_EXPECT(cfg.IPS_FIXED[3] == "anotherserverwithport 12");
+        BEAST_EXPECT(cfg.IPS_FIXED[4] == "1.1.1.1 1");
+        BEAST_EXPECT(cfg.IPS_FIXED[5] == "1.1.1.1 1");
+        BEAST_EXPECT(cfg.IPS_FIXED[6] == "12.34.12.123 12345");
+        BEAST_EXPECT(cfg.IPS_FIXED[7] == "12.34.12.123 12345");
+
+        // all ipv6 should be ignored by colon replacer, howsoever formated
+        BEAST_EXPECT(cfg.IPS_FIXED[8] == "::");
+        BEAST_EXPECT(cfg.IPS_FIXED[9] == "2001:db8::");
+        BEAST_EXPECT(cfg.IPS_FIXED[10] == "::1");
+        BEAST_EXPECT(cfg.IPS_FIXED[11] == "::1:12345");
+        BEAST_EXPECT(cfg.IPS_FIXED[12] == "[::1]:12345");
+        BEAST_EXPECT(
+            cfg.IPS_FIXED[13] ==
+            "2001:db8:3333:4444:5555:6666:7777:8888:12345");
+        BEAST_EXPECT(
+            cfg.IPS_FIXED[14] == "[2001:db8:3333:4444:5555:6666:7777:8888]:1");
+    }
+
     void
     testComments()
     {
@@ -1147,6 +1225,7 @@ r.ripple.com 51235
         testSetup(true);
         testPort();
         testWhitespace();
+        testColons();
         testComments();
         testGetters();
         testAmendment();