Don't permit port in proxy IP option

[willcl-ark]

Fixes: https://github.com/bitcoin-core/gui/issues/809

Previously it was possible through the GUI to enter an IP address:port into the "Proxy IP" configuration box. After the node was restarted the errant setting would prevent the node starting back up until manually removed from settings.json.

Prevent this with a simple check for ":" in the string. This is acceptable here in the GUI setting because we already fail on a hostname such as "http://x.x.x.x", so it won't cause false positives.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.	Type	Reviewers
ACK	furszy, hebasto
Concept ACK	pablomartin4btc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

[willcl-ark]

I'd suggest to use a regex validator for the IP address, it's much simpler I think and the user won't be allow to enter any other symbols or chars (typo?) that aren't digits (plus user can't type more than 3 subnets separated by dots where currently you could and any symbols)

I'm not convinced that a large regex is "simpler" than checking if a ":" is in the string?

Thanks for the other suggestions, but I don't feel that interested in picking them here with the QML work going on, and them not being much of an issue IMO. In contrast this fix prevents a bug whereby an incorrect setting can stop the node starting up.

The only potential issue I saw with my own solution was that it would outright prevent setting the tor proxy IP to an IPv6 address, however from my (quick) research, it didn't seem that this was actually something anyone did (and it might not even be possible/practical to do). If I'm wrong on this, I'll be happy to update the solution further, but from what I can see even setting Tor ORPort to IPV6 is barely supported, and ControlPort is limited to an IPv4 address:port...

[pablomartin4btc]

I'm not convinced that a large regex is "simpler" than checking if a ":" is in the string?

The large one was for the port number, the one I referred as "simpler" was for the IP address: QRegularExpression ipRegex("^([0-9]{1,3}\\.){3}[0-9]{1,3}$");. What I meant is that we know in advance that the ":" would be invalid so we could restrict the user to even enter it. As current we could see this (not only ":"):

The regex would keep it cleaner, more user friendly and we don't have to do an extra validation, but I'll stop trying to convince you :smile: (btw, also we'd need to set the max length to 15/ follow up).

I don't feel that interested in picking them here with the QML work going on, and them not being much of an issue IMO.

QML is far for being at the stage where QT currently is, improving user experience could also be useful for both projects feedback.

In contrast this fix prevents a bug whereby an incorrect setting can stop the node starting up.

I agree and there's a workaround already which is just removing the IP line from the file.

The only potential issue I saw with my own solution was that it would outright prevent setting the tor proxy IP to an IPv6 address, however from my (quick) research, it didn't seem that this was actually something anyone did (and it might not even be possible/practical to do). If I'm wrong on this, I'll be happy to update the solution further, but from what I can see even setting Tor ORPort to IPV6 is barely supported, and ControlPort is limited to an IPv4 address:port...

Is it even possible to enable those checkboxes?

[luke-jr]

@jarolrod Isn't that a bug independent from the validation one?

[fanquake]

Backported in https://github.com/bitcoin/bitcoin/pull/30092.