From cb08f2b6ec425c4686c8c5c7fc4a3209e91166f2 Mon Sep 17 00:00:00 2001 From: RichardAH Date: Wed, 15 Mar 2023 05:06:30 +0100 Subject: [PATCH] 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 --- src/ripple/core/impl/Config.cpp | 23 +++++++++ src/ripple/net/impl/RPCCall.cpp | 22 ++++++-- src/ripple/rpc/handlers/Connect.cpp | 8 +-- src/test/core/Config_test.cpp | 79 +++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index c2cfb14d2..123749728 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #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& 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_)) diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index b475afe9d..b5a167f76 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -482,17 +482,31 @@ private: return jvRequest; } - // connect [port] + // connect [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; } diff --git a/src/ripple/rpc/handlers/Connect.cpp b/src/ripple/rpc/handlers/Connect.cpp index 532e04087..ed366f64b 100644 --- a/src/ripple/rpc/handlers/Connect.cpp +++ b/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 diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index da29fafac..b455762de 100644 --- a/src/test/core/Config_test.cpp +++ b/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();