[Validator] Allow basic auth in url when using UrlValidator. #11601
Conversation
| @@ -24,6 +24,7 @@ class UrlValidator extends ConstraintValidator | |||
| { | |||
| const PATTERN = '~^ | |||
| (%s):// # protocol | |||
| ((.+?):(.+?)@)? # basic auth | |||
There was a problem hiding this comment.
.+? 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.
@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.
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.
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.comcan be valid.