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.

Sign up for GitHub

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

Jump to bottom

Fix address regex #3961

Merged
merged 1 commit into from
Feb 11, 2020
Merged

Fix address regex #3961

merged 1 commit into from
Feb 11, 2020

Conversation

devinbileck
Copy link
Member

The original implementation of #3895 did not validate IPv6 nor FQDN addresses.

@devinbileck devinbileck requested a review from wiz February 10, 2020 23:55
@ripcurlx
Copy link
Contributor

ripcurlx commented Feb 11, 2020 •
edited
Loading

@devinbileck Could you please resolve the conflict caused by a merged PR? Thanks!

wiz
wiz suggested changes Feb 11, 2020
View reviewed changes
Copy link
Contributor

@wiz wiz left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this so quickly. I'm testing it now but unfortunately my configuration still fails to pass your validation, I guess the following testcase is not handled by the FQDN regex:

foo.example.com:8333,bar.example.com:8333

@wiz
Copy link
Contributor

wiz commented Feb 11, 2020

Also, probably unrelated to your change, but for some reason IPv6 address btcnodes work on the command line but don't work when set in the GUI - I always set my stuff from the command line but kinda surprised nobody else noticed that bug.

wiz
wiz previously approved these changes Feb 11, 2020
View reviewed changes
Copy link
Contributor

@wiz wiz left a comment

Choose a reason for hiding this comment

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

Actually will ACK for now and will submit a separate PR to fix the remaining FQDN regex case

@devinbileck @ripcurlx
Fix address regex
0f71ebb 
The original implementation of bisq-network#3895 did not validate IPv6 nor FQDN
addresses.
@ripcurlx
Copy link
Contributor

☝️ resolved the conflict with master and force pushed it

ripcurlx
ripcurlx approved these changes Feb 11, 2020
View reviewed changes
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK - based on #3961 (review)

@ripcurlx ripcurlx merged commit 777052f into bisq-network:master Feb 11, 2020
@wiz wiz mentioned this pull request Feb 11, 2020
Merged
@devinbileck devinbileck deleted the fix-address-regex branch February 11, 2020 17:26
@ripcurlx ripcurlx mentioned this pull request Feb 14, 2020
Closed
@devinbileck devinbileck mentioned this pull request Mar 11, 2020
Closed
@ripcurlx ripcurlx added the is:priority PR or issue marked with this label is up for compensation label Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ripcurlx ripcurlx ripcurlx approved these changes

@wiz wiz wiz left review comments

Assignees
No one assigned
Labels
is:priority PR or issue marked with this label is up for compensation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@devinbileck @ripcurlx @wiz