-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Validator] Update regular expression in URL validator #62028
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
[Validator] Update regular expression in URL validator #62028
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
To achieve better compatibility with RFC 3986, the regular expression which validates URLs now allows more characters in the userinfo part. Add test cases; update change log.
d385f3e to
82bbb9c
Compare
nicolas-grekas
left a comment
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.
I rebased for 6.4 as a bugfix and further tweaked the regex a bit.
| {^ | ||
| (%s):// # protocol | ||
| (((?:[\_\.\pL\pN-]|%%[0-9A-Fa-f]{2})+:)?((?:[\_\.\pL\pN-]|%%[0-9A-Fa-f]{2})+)@)? # basic auth | ||
| ((?:[\pL\pN\-._~!$&'()*+,;=]|%%[0-9A-Fa-f]{2})++(?::(?:[:\pL\pN\-._~!$&'()*+,;=]|%%[0-9A-Fa-f]{2})*+)?@)? # basic auth |
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 RFC allows empty user part, so technically we could go with just the following.
But this makes tests fail as we consider userinfo with no username as invalid (http://:[email protected])
| ((?:[\pL\pN\-._~!$&'()*+,;=]|%%[0-9A-Fa-f]{2})++(?::(?:[:\pL\pN\-._~!$&'()*+,;=]|%%[0-9A-Fa-f]{2})*+)?@)? # basic auth | |
| ((?:[:\pL\pN\-._~!$&'()*+,;=]|%%[0-9A-Fa-f]{2})++)@)? # basic auth |
(support for basic auth was added 11 years ago in #11601)
|
Thank you @mjaschen. |
The
UrlValidator::validate()method currently fails for some valid URLs, particularly URLs containing login data with special characters.Example failing case:
The current regular expression only accepts a subset of allowed characters in the userinfo part of the URL, see
UrlValidator.php:26.Changes in this pull request:
UrlValidator::PATTERNto support all characters permitted in theuserinfopart of a URL according to RFC 3986.[\_\.]→[_.])userinfopart of URLs.References:
Relevant ABNF for
userinfoin URIs: