Merge bitcoin/bitcoin#29521: cli: Detect port errors in rpcconnect and rpcport

24bc46c83b39149f4845a575a82337eb46d91bdb cli: Add warning for duplicate port definition (tdb3)
e208fb5d3bea4c1fb750cb0028819635ecdeb415 cli: Sanitize ports in rpcconnect and rpcport (tdb3)

Pull request description:

  Adds invalid port detection to bitcoin-cli for -rpcconnect and -rpcport.

  In addition to detecting malformed/invalid ports (e.g. those outside of the 16-bit port range, not numbers, etc.), bitcoin-cli also now considers usage of port 0 to be invalid.  bitcoin-cli previously considered port 0 to be valid and attempted to use it to reach bitcoind.

  Functional tests were added for invalid port detection as well as port prioritization.
  Additionally, a warning is provided when a port is specified in both -rpcconnect and -rpcport.

  This PR is an alternate approach to PR #27820 (e.g. SplitHostPort is unmodified).

  Considered an alternative to 127.0.0.1 being specified in functional tests, but at first glance, this might need an update to test_framework/util.py (e.g.  rpc_url), which might be left to a future PR.

ACKs for top commit:
  S3RK:
    light code review ACK 24bc46c83b39149f4845a575a82337eb46d91bdb
  achow101:
    ACK 24bc46c83b39149f4845a575a82337eb46d91bdb
  cbergqvist:
    re ACK 24bc46c83b39149f4845a575a82337eb46d91bdb

Tree-SHA512: c83ab6a30a08dd1ac8b368a7dcc2b4f23170f0b61dd67ffcad7bcda05096d333bcb9821fba11018151f55b2929c0a333bfec15b8bb863d83f41fc1974c6efca5

1 files changed, 49 insertions, 0 deletions

diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py
index 83bb5121e5..a6628dcbf3 100755
--- a/test/functional/interface_bitcoin_cli.py
+++ b/test/functional/interface_bitcoin_cli.py
@@ -8,6 +8,7 @@ from decimal import Decimal
 import re
 
 from test_framework.blocktools import COINBASE_MATURITY
+from test_framework.netutil import test_ipv6_local
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -15,6 +16,7 @@ from test_framework.util import (
     assert_raises_process_error,
     assert_raises_rpc_error,
     get_auth_cookie,
+    rpc_port,
 )
 import time
 
@@ -107,6 +109,53 @@ class TestBitcoinCli(BitcoinTestFramework):
         self.log.info("Test connecting to a non-existing server")
         assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcport=1').echo)
 
+        self.log.info("Test handling of invalid ports in rpcconnect")
+        assert_raises_process_error(1, "Invalid port provided in -rpcconnect: 127.0.0.1:notaport", self.nodes[0].cli("-rpcconnect=127.0.0.1:notaport").echo)
+        assert_raises_process_error(1, "Invalid port provided in -rpcconnect: 127.0.0.1:-1", self.nodes[0].cli("-rpcconnect=127.0.0.1:-1").echo)
+        assert_raises_process_error(1, "Invalid port provided in -rpcconnect: 127.0.0.1:0", self.nodes[0].cli("-rpcconnect=127.0.0.1:0").echo)
+        assert_raises_process_error(1, "Invalid port provided in -rpcconnect: 127.0.0.1:65536", self.nodes[0].cli("-rpcconnect=127.0.0.1:65536").echo)
+
+        self.log.info("Checking for IPv6")
+        have_ipv6 = test_ipv6_local()
+        if not have_ipv6:
+            self.log.info("Skipping IPv6 tests")
+
+        if have_ipv6:
+            assert_raises_process_error(1, "Invalid port provided in -rpcconnect: [::1]:notaport", self.nodes[0].cli("-rpcconnect=[::1]:notaport").echo)
+            assert_raises_process_error(1, "Invalid port provided in -rpcconnect: [::1]:-1", self.nodes[0].cli("-rpcconnect=[::1]:-1").echo)
+            assert_raises_process_error(1, "Invalid port provided in -rpcconnect: [::1]:0", self.nodes[0].cli("-rpcconnect=[::1]:0").echo)
+            assert_raises_process_error(1, "Invalid port provided in -rpcconnect: [::1]:65536", self.nodes[0].cli("-rpcconnect=[::1]:65536").echo)
+
+        self.log.info("Test handling of invalid ports in rpcport")
+        assert_raises_process_error(1, "Invalid port provided in -rpcport: notaport", self.nodes[0].cli("-rpcport=notaport").echo)
+        assert_raises_process_error(1, "Invalid port provided in -rpcport: -1", self.nodes[0].cli("-rpcport=-1").echo)
+        assert_raises_process_error(1, "Invalid port provided in -rpcport: 0", self.nodes[0].cli("-rpcport=0").echo)
+        assert_raises_process_error(1, "Invalid port provided in -rpcport: 65536", self.nodes[0].cli("-rpcport=65536").echo)
+
+        self.log.info("Test port usage preferences")
+        node_rpc_port = rpc_port(self.nodes[0].index)
+        # Prevent bitcoin-cli from using existing rpcport in conf
+        conf_rpcport = "rpcport=" + str(node_rpc_port)
+        self.nodes[0].replace_in_config([(conf_rpcport, "#" + conf_rpcport)])
+        # prefer rpcport over rpcconnect
+        assert_raises_process_error(1, "Could not connect to the server 127.0.0.1:1", self.nodes[0].cli(f"-rpcconnect=127.0.0.1:{node_rpc_port}", "-rpcport=1").echo)
+        if have_ipv6:
+            assert_raises_process_error(1, "Could not connect to the server ::1:1", self.nodes[0].cli(f"-rpcconnect=[::1]:{node_rpc_port}", "-rpcport=1").echo)
+
+        assert_equal(BLOCKS, self.nodes[0].cli("-rpcconnect=127.0.0.1:18999", f'-rpcport={node_rpc_port}').getblockcount())
+        if have_ipv6:
+            assert_equal(BLOCKS, self.nodes[0].cli("-rpcconnect=[::1]:18999", f'-rpcport={node_rpc_port}').getblockcount())
+
+        # prefer rpcconnect port over default
+        assert_equal(BLOCKS, self.nodes[0].cli(f"-rpcconnect=127.0.0.1:{node_rpc_port}").getblockcount())
+        if have_ipv6:
+            assert_equal(BLOCKS, self.nodes[0].cli(f"-rpcconnect=[::1]:{node_rpc_port}").getblockcount())
+
+        # prefer rpcport over default
+        assert_equal(BLOCKS, self.nodes[0].cli(f'-rpcport={node_rpc_port}').getblockcount())
+        # Re-enable rpcport in conf if present
+        self.nodes[0].replace_in_config([("#" + conf_rpcport, conf_rpcport)])
+
         self.log.info("Test connecting with non-existing RPC cookie file")
         assert_raises_process_error(1, "Could not locate RPC credentials", self.nodes[0].cli('-rpccookiefile=does-not-exist', '-rpcpassword=').echo)