-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix: Improve RegExp for URLs in Admin settings #3553
Conversation
@kushthedude Please review. |
// user:pass authentication | ||
+ '(?:\\S+(?::\\S*)?@)?' | ||
+ '(?:' | ||
// IP address exclusion | ||
// private & local networks | ||
+ '(?!(?:10|127)(?:\\.\\d{1,3}){3})' | ||
+ '(?!(?:10)(?:\\.\\d{1,3}){3})' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any link/resource to verify the validity of this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Loopback IP address ranges from 127.0.0.0 to 127.255.255.255 and as per the Issue the Loopback address(es) should be valid for testing purposes so I removed the 127 address from the exclusion list as it was just excluding any addresses beginning with 10/127.
@reetam-nandi Can you please add a gif or video, showing reg-ex is working fine for all previous rules and new rules ? |
@kushthedude please check the following link which tests out the RegEx with necessary test cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Fixes #3528
Short description of what this resolves:
RegExp for URLs improved to allow acceptance of localhost or 127.0.0.1 URLs in Admin settings.
Checklist
development
branch.