Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow IPv6 in Proxy settings and moving validation out from the UI into the model/ interface #430

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Copy link
Contributor

@johnny9 johnny9 Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these actually default settings values or just values presented to the user? If they are just hints, they can be left as strings in the QML.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are proper default values. I would leave them here for now, but maybe they can be moved into the netbase later.


//! 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this should return a string. I think it should be an in/out Proxy&.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for display purposes, it doesn't make sense to build the object which also involves CService and so on, just to come back with eg ipv4.proxy.ToStringAddrPort();, I'd leave it as it is but can be done on a follow-up if more ppl think the change is required.

{
return std::string(DEFAULT_PROXY_HOST) + ":" + ToString(DEFAULT_PROXY_PORT);
}
bool validateProxyAddress(const std::string& addr_port) override
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateProxyAddress shouldn't be in this module. It should exist in the nodemodel or possibly in netbase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, maybe it could be moved deeper into de netbase but I didn't what to touch that at this stage, it can be done later once we update the interfaces upstream.

{
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;
pablomartin4btc marked this conversation as resolved.
Show resolved Hide resolved
}
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
Loading