Skip to content

Commit

Permalink
Merge bitcoin-core#430: Allow IPv6 in Proxy settings and moving valid…
Browse files Browse the repository at this point in the history
…ation out from the UI into the model/ interface

4dea7c9 qml, proxy: Allow IPv6 and move out UI validation (pablomartin4btc)

Pull request description:

  The main purposes of this PR are:
  - Allow configure proxy server with IPv6 (after testing exhaustively in current `bitcoin-qt` [repo](https://github.com/bitcoin-core/gui/) and `bitcoind` - e.g. [IPv6 on Qt](bitcoin-core/gui#836 (comment)); [IP validation in Qt](bitcoin-core/gui#813))
  - More importantly: moving the validation logic out from the UI (component/ control/ widget) into the back-end (model/ interfaces). It seemed currently in [Qt](https://github.com/bitcoin-core/gui/) all this logic (validation, network classes) was coupled to the UI since the beginning (more than [12y ago](https://github.com/bitcoin-core/gui/blame/c4443c2be141e5f45bb10376056f3083e97cde50/src/qt/optionsmodel.cpp)) instead of using interfaces (introduced much later) that would be the correct thing to do.

  Feedback and suggestions are very welcome and will help establish a foundation for leaving business logic out of the UI going forward.

  ---
  <details>
  <summary>In order to test it, if it's the first time you run <code>./src/qt/bitcoin-qt</code> you need to go thru the on-boarding flow (<i>start->next->next->next->next->connection settings->Proxy settings</i>) otherwise you need to go to the settings (top right gear icon) then <i>Connection->Proxy settings</i>.</summary>

  ![image](https://github.com/user-attachments/assets/9f10c4b7-b7e4-4807-b895-1c03d9c00037)

  </details>

  Before this change, only IPv4 address were allow in the value input, now also IPv6 can be entered.

  ---
  There are some [look and feel kind of issues](bitcoin-core#430 (comment)) that will be handled on follow-ups (issues: bitcoin-core#437 and bitcoin-core#438).

ACKs for top commit:
  D33r-Gee:
    tACK [4dea7c9](bitcoin-core@4dea7c9) on Ubuntu 20.04 works as expected.
  vasild:
    ACK 4dea7c9
  johnny9:
    ACK 4dea7c9

Tree-SHA512: f633f5729f4ca2a8433560b15722bccd5938bfa55643cd3d925b75ed01006dbc7a56fe3a20766a6bb9e2bb601c8a3828c177de3cccc7728959eb48987838e90c
  • Loading branch information
hebasto committed Feb 3, 2025
2 parents b4bebc3 + 4dea7c9 commit 765f608
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 34 deletions.
9 changes: 9 additions & 0 deletions src/interfaces/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
#include <tuple>
#include <vector>

static const char DEFAULT_PROXY_HOST[] = "127.0.0.1";
static constexpr uint16_t DEFAULT_PROXY_PORT = 9050;

class BanMan;
class CFeeRate;
class CNodeStats;
Expand Down Expand Up @@ -126,6 +129,12 @@ class Node
//! Get proxy.
virtual bool getProxy(Network net, Proxy& proxy_info) = 0;

//! Get default proxy address.
virtual std::string defaultProxyAddress() = 0;

//! Validate a proxy address.
virtual bool validateProxyAddress(const std::string& addr_port) = 0;

//! Get number of connections.
virtual size_t getNodeCount(ConnectionDirection flags) = 0;

Expand Down
22 changes: 22 additions & 0 deletions src/node/interfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <uint256.h>
#include <univalue.h>
#include <util/check.h>
#include <util/strencodings.h>
#include <util/translation.h>
#include <validation.h>
#include <validationinterface.h>
Expand Down Expand Up @@ -169,6 +170,27 @@ class NodeImpl : public Node
}
void mapPort(bool use_upnp, bool use_natpmp) override { StartMapPort(use_upnp, use_natpmp); }
bool getProxy(Network net, Proxy& proxy_info) override { return GetProxy(net, proxy_info); }
std::string defaultProxyAddress() override
{
return std::string(DEFAULT_PROXY_HOST) + ":" + ToString(DEFAULT_PROXY_PORT);
}
bool validateProxyAddress(const std::string& addr_port) override
{
uint16_t port{0};
std::string hostname;
// First, attempt to split the input address into hostname and port components.
// We call SplitHostPort to validate that a port is provided in addr_port.
// If either splitting fails or port is zero (not specified), return false.
if (!SplitHostPort(addr_port, port, hostname) || !port) return false;

// Create a service endpoint (CService) from the address and port.
// If port is missing in addr_port, DEFAULT_PROXY_PORT is used as the fallback.
CService serv(LookupNumeric(addr_port, DEFAULT_PROXY_PORT));

// Construct the Proxy with the service endpoint and return if it's valid
Proxy addrProxy = Proxy(serv, true);
return addrProxy.IsValid();
}
size_t getNodeCount(ConnectionDirection flags) override
{
return m_context->connman ? m_context->connman->GetNodeCount(flags) : 0;
Expand Down
23 changes: 17 additions & 6 deletions src/qml/components/ProxySettings.qml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import QtQuick.Controls 2.15
import QtQuick.Layouts 1.15
import "../controls"

import org.bitcoincore.qt 1.0

ColumnLayout {
property string ipAndPortHeader: qsTr("IP and Port")
property string invalidIpError: qsTr("Invalid IP address or port format. Use '255.255.255.255:65535' or '[ffff::]:65535'")

spacing: 4
Header {
headerBold: true
Expand Down Expand Up @@ -41,14 +46,17 @@ ColumnLayout {
Setting {
id: defaultProxy
Layout.fillWidth: true
header: qsTr("IP and Port")
errorText: qsTr("Invalid IP address or port format. Please use the format '255.255.255.255:65535'.")
header: ipAndPortHeader
errorText: invalidIpError
state: !defaultProxyEnable.loadedItem.checked ? "DISABLED" : "FILLED"
showErrorText: !defaultProxy.loadedItem.validInput && defaultProxyEnable.loadedItem.checked
actionItem: IPAddressValueInput {
parentState: defaultProxy.state
description: "127.0.0.1:9050"
description: nodeModel.defaultProxyAddress()
activeFocusOnTab: true
onTextChanged: {
validInput = nodeModel.validateProxyAddress(text);
}
}
onClicked: {
loadedItem.filled = true
Expand Down Expand Up @@ -89,14 +97,17 @@ ColumnLayout {
Setting {
id: torProxy
Layout.fillWidth: true
header: qsTr("IP and Port")
errorText: qsTr("Invalid IP address or port format. Please use the format '255.255.255.255:65535'.")
header: ipAndPortHeader
errorText: invalidIpError
state: !torProxyEnable.loadedItem.checked ? "DISABLED" : "FILLED"
showErrorText: !torProxy.loadedItem.validInput && torProxyEnable.loadedItem.checked
actionItem: IPAddressValueInput {
parentState: torProxy.state
description: "127.0.0.1:9050"
description: nodeModel.defaultProxyAddress()
activeFocusOnTab: true
onTextChanged: {
validInput = nodeModel.validateProxyAddress(text);
}
}
onClicked: {
loadedItem.filled = true
Expand Down
30 changes: 2 additions & 28 deletions src/qml/controls/IPAddressValueInput.qml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ TextInput {
property bool validInput: true
enabled: true
state: root.parentState
validator: RegExpValidator { regExp: /[0-9.:]*/ } // Allow only digits, dots, and colons
validator: RegularExpressionValidator { regularExpression: /^[\][0-9a-f.:]+$/i } // Allow only IPv4/ IPv6 chars

maximumLength: 21
maximumLength: 47

states: [
State {
Expand Down Expand Up @@ -53,30 +53,4 @@ TextInput {
Behavior on color {
ColorAnimation { duration: 150 }
}

function isValidIPPort(input)
{
var parts = input.split(":");
if (parts.length !== 2) return false;
if (parts[1].length === 0) return false; // port part is empty
var ipAddress = parts[0];
var ipAddressParts = ipAddress.split(".");
if (ipAddressParts.length !== 4) return false;
for (var i = 0; (i < ipAddressParts.length); i++) {
if (ipAddressParts[i].length === 0) return false; // ip group number part is empty
if (parseInt(ipAddressParts[i]) > 255) return false;
}
var port = parseInt(parts[1]);
if (port < 1 || port > 65535) return false;
return true;
}

// Connections element to ensure validation on editing finished
Connections {
target: root
function onTextChanged() {
// Validate the input whenever editing is finished
validInput = isValidIPPort(root.text);
}
}
}
10 changes: 10 additions & 0 deletions src/qml/models/nodemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,13 @@ void NodeModel::ConnectToNumConnectionsChangedSignal()
setNumOutboundPeers(new_num_peers.outbound_full_relay + new_num_peers.block_relay);
});
}

bool NodeModel::validateProxyAddress(QString address_port)
{
return m_node.validateProxyAddress(address_port.toStdString());
}

QString NodeModel::defaultProxyAddress()
{
return QString::fromStdString(m_node.defaultProxyAddress());
}
3 changes: 3 additions & 0 deletions src/qml/models/nodemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ class NodeModel : public QObject
void startShutdownPolling();
void stopShutdownPolling();

Q_INVOKABLE bool validateProxyAddress(QString addr_port);
Q_INVOKABLE QString defaultProxyAddress();

public Q_SLOTS:
void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info);

Expand Down

0 comments on commit 765f608

Please sign in to comment.