-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] Allow basic auth in url when using UrlValidator. #11601
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
Conversation
@@ -24,6 +24,7 @@ class UrlValidator extends ConstraintValidator | |||
{ | |||
const PATTERN = '~^ | |||
(%s):// # protocol | |||
((.+?):(.+?)@)? # 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.
.+?
is equivalent to .*
and anyway, this pattern is wrong .
is too permissive here. For instance, you cannot have a @
in there (you have to urlencode it)
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.
@stof Is this better ([\pL\pN-]*):([\pL\pN-]*)@
?
I'm not an expert in regex, but i need this to be fixed!
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.
Should be (([\pL\pN-]+:)?([\pL\pN-]+)@)?
then, as the first part is optional and at least the second should exists with the @
sign.
OK: user:pass@
, user@
and empty.
WRONG: user:pass
, @
, :pass@
, :pass
Hmm seems regex is failing need to take a look at this.
Ah never mind I forgot to click for the test 😳
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.
Thx @sstok for your help
Improve regex. Add tests.
@stof do i need to fix something else ? |
👍 |
👍 |
Thank you @blaugueux. |
…r. (blaugueux) This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #11601). Discussion ---------- [Validator] Allow basic auth in url when using UrlValidator. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Now an url with basic auth like ```http://username:password@symfony.com``` can be valid. Commits ------- f1ea987 Allow basic auth in url. Improve regex. Add tests.
Huge thanks for all your work @fabpot |
Now an url with basic auth like
http://username:password@symfony.com
can be valid.