while reviewing GetBindAddresses(), i ended up rewriting it to lower the cognitive load a bit and help me get my head around the flow. thought i could share it and see what you think:
<details><summary>diff</summary>
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 4b17dc5d89..4d97422f4f 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -212,51 +212,57 @@ static void RejectRequest(std::unique_ptr<HTTPRequest> hreq)
static std::vector<SocketAddr> GetBindAddresses()
{
- uint16_t http_port{static_cast<uint16_t>(gArgs.GetIntArg("-rpcport", BaseParams().RPCPort()))};
- std::vector<SocketAddr> endpoints;
+ const uint16_t http_port{static_cast<uint16_t>(gArgs.GetIntArg("-rpcport", BaseParams().RPCPort()))};
+ const auto binds{gArgs.GetArgs("-rpcbind")};
+ const bool has_allowip{!gArgs.GetArgs("-rpcallowip").empty()};
+ const auto localhost_endpoints{[http_port]() -> std::vector<SocketAddr> {
+ return {SocketAddr{Lookup("::1", http_port, false).value()},
+ SocketAddr{Lookup("127.0.0.1", http_port, false).value()}};
+ }};
+
+ // default to loopback when no rpc binds are provided
+ if (binds.empty()) {
+ if (has_allowip) {
+ LogWarning("Option -rpcallowip was specified without -rpcbind; this doesn't usually make sense");
+ }
+ return localhost_endpoints();
+ }
- // Cleared as soon as any -rpcbind value is an IP address, whether or not it
- // resolves, so that a lookup failure can never make a TCP bind request
- // look like a unix-socket-only configuration.
- bool only_unix_sockets{true};
- // Set to true if no addresses were specified, or
- // if we need to override a user misconfiguration.
- bool force_localhost{false};
+ std::vector<SocketAddr> endpoints;
+ // Set before parsing or looking up, so that a failure can never make
+ // a TCP bind request look like a unix-socket-only configuration.
+ bool has_ip{false};
// Determine what addresses to bind to.
- if (!gArgs.GetArgs("-rpcbind").empty()) {
+ for (const std::string& bind : binds) {
// Specific bind addresses.
- for (const std::string& strRPCBind : gArgs.GetArgs("-rpcbind")) {
- if (IsUnixSocketValue(strRPCBind, /*allow_default=*/true)) {
- // UNIX socket is a filesystem path, resolved like -ipcbind
- const auto unix_addr{ResolveUnixSocketAddr(strRPCBind, gArgs.GetDataDirNet(), DEFAULT_HTTP_UNIX_SOCKET_NAME)};
- if (!unix_addr) {
- LogError("Invalid unix socket path specified in -rpcbind: '%s' (%s)", strRPCBind, unix_addr.error());
- return {}; // empty, refuse to start with a misconfigured bind
- }
- endpoints.emplace_back(*unix_addr);
- continue;
- }
- // IPv4 or IPv6 address
- only_unix_sockets = false;
- uint16_t port{http_port};
- std::string host;
- if (!SplitHostPort(strRPCBind, port, host)) {
- LogError("%s\n", InvalidPortErrMsg("-rpcbind", strRPCBind).original);
+ if (IsUnixSocketValue(bind, /*allow_default=*/true)) {
+ // UNIX socket is a filesystem path, resolved like -ipcbind
+ const auto unix_addr{ResolveUnixSocketAddr(bind, gArgs.GetDataDirNet(), DEFAULT_HTTP_UNIX_SOCKET_NAME)};
+ if (!unix_addr) {
+ LogError("Invalid unix socket path specified in -rpcbind: '%s' (%s)", bind, unix_addr.error());
return {}; // empty, refuse to start with a misconfigured bind
}
- const std::optional<CService> addr{Lookup(strRPCBind, port, false)};
- if (!addr) {
- LogWarning("Could not bind RPC on address %s port %i: Address lookup failed.", strRPCBind, port);
- continue;
- }
- if (addr->IsBindAny()) {
- LogWarning("The RPC server is not safe to expose to untrusted networks such as the public internet");
- }
- endpoints.emplace_back(addr.value());
+ endpoints.emplace_back(*unix_addr);
+ continue;
}
- } else {
- force_localhost = true;
+ // IPv4 or IPv6 address
+ has_ip = true;
+ uint16_t port{http_port};
+ std::string host;
+ if (!SplitHostPort(bind, port, host)) {
+ LogError("%s\n", InvalidPortErrMsg("-rpcbind", bind).original);
+ return {}; // empty, refuse to start with a misconfigured bind
+ }
+ const std::optional<CService> addr{Lookup(bind, port, false)};
+ if (!addr) {
+ LogWarning("Could not bind RPC on address %s port %i: Address lookup failed.", host, port);
+ continue;
+ }
+ if (addr->IsBindAny()) {
+ LogWarning("The RPC server is not safe to expose to untrusted networks such as the public internet");
+ }
+ endpoints.emplace_back(addr.value());
}
// Access to unix sockets is controlled by filesystem permissions, so if we
@@ -264,19 +270,9 @@ static std::vector<SocketAddr> GetBindAddresses()
// Otherwise, to prevent misconfiguration and accidental exposure of the HTTP
// interface, require -rpcallowip whenever an IP address is bound.
// If the user is misconfigured we restrict HTTP to localhost and log warnings.
- if (!only_unix_sockets && gArgs.GetArgs("-rpcallowip").empty()) {
+ if (has_ip && !has_allowip) {
LogWarning("Option -rpcbind was ignored because -rpcallowip was not specified, refusing to allow everyone to connect");
- force_localhost = true;
- }
-
- // Default to loopback, or replace user misconfiguration.
- if (force_localhost) {
- endpoints.clear();
- endpoints.emplace_back(Lookup("::1", http_port, false).value());
- endpoints.emplace_back(Lookup("127.0.0.1", http_port, false).value());
- if (!gArgs.GetArgs("-rpcallowip").empty()) {
- LogWarning("Option -rpcallowip was specified without -rpcbind; this doesn't usually make sense");
- }
+ return localhost_endpoints();
}
return endpoints;
}
</details>
this is purely a suggestion, so feel free to take or leave it :)
