Validate ignored peers and BTC nodes #3895
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will allow for a custom validation error message to be defined, particularly useful for providing a user-friendly message for regex validation.
Ignored peers and BTC nodes input fields will now only accept IPv4 and V2 onion addresses, with multiple addresses separated using a comma.
The shutdown popup was appearing when the BTC nodes input field lost focus regardless if the input had changed or not.
If the user entered an invalid hostname for a custom BTC node, such as a V3 onion address, after restarting Bisq they would be presented with an error due to the node being unreachable and unable to continue nor correct the config. So now a warning message will be shown in this situation informing the user of an invalid config and once they restart their client they will connect to the provided BTC nodes. Fixes bisq-network#3137
sqrrm
approved these changes
Jan 14, 2020
ripcurlx
added
the
is:priority
|
@devinbileck it seems your new input validation is too strict, totally valid FQDNs like |
wiz
reviewed
Feb 9, 2020
| assertFalse(regexValidator.validate("onion:9999,abcdefghijklmnop.onion:9999").isValid); | ||
| assertFalse(regexValidator.validate("abcdefghijklmnop.onion:").isValid); | ||
| assertFalse(regexValidator.validate("32zzibxmqi2ybxpqyggwwuwz7a3lbvtzoloti7cxoevyvijexvgsfeid.onion:8333").isValid); | ||
|
|
There was a problem hiding this comment.
Suggested change
| assertTrue(regexValidator.validate("example.com").isValid); |
wiz
reviewed
Feb 9, 2020
| @@ -1107,4 +1108,14 @@ public static MaterialDesignIcon getIconForSignState(AccountAgeWitnessService.Si | |||
| state.equals(AccountAgeWitnessService.SignState.PEER_SIGNER)) ? | |||
| MaterialDesignIcon.APPROVAL : MaterialDesignIcon.ALERT_CIRCLE_OUTLINE; | |||
| } | |||
|
|
|||
| public static RegexValidator addressRegexValidator() { | |||
| RegexValidator regexValidator = new RegexValidator(); | |||
There was a problem hiding this comment.
Missing the regex to validate example.com here?
|
Also need to handle IPv6 addresses, i.e. |
|
Thanks. I will submit a PR shortly. |
devinbileck
added a commit
to devinbileck/bisq
that referenced
this pull request
Feb 10, 2020
The original implementation of bisq-network#3895 did not validate IPv6 nor FQDN addresses.
Merged
ripcurlx
pushed a commit
to devinbileck/bisq
that referenced
this pull request
Feb 11, 2020
The original implementation of bisq-network#3895 did not validate IPv6 nor FQDN addresses.
ripcurlx
pushed a commit
that referenced
this pull request
Feb 11, 2020
The original implementation of #3895 did not validate IPv6 nor FQDN addresses.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In addition to adding input validation for ignored peers and BTC nodes, this PR also fixes #3137.
Since with input validation the user is no longer able to enter an invalid BTC host, to verify #3137 start the app with the following option:
--btcNodes=32zzibxmqi2ybxpqyggwwuwz7a3lbvtzoloti7cxoevyvijexvgsfeid.onion:8333You will be presented with the following:
After shutting down the app, restart it without the previous option and observe the BTC config. You will see the invalid custom BTC config but it will be using the provided BTC nodes.