-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
Fixed #29528 -- Made URLValidator reject invalid characters in the username and password. #10097
Changes from 2 commits
f34dcb0
fe29348
d579001
07d9f85
5e4d354
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,6 @@ http://foo.bar/?q=Test%20URL-encoded%20stuff | |
http://مثال.إختبار | ||
http://例子.测试 | ||
http://उदाहरण.परीक्षा | ||
http://-.~_!$&'()*+,;=:%40:80%2f::::::@example.com | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we leave this, but just remove the offending There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of the other colons (in |
||
http://xn--7sbb4ac0ad0be6cf.xn--p1ai | ||
http://1337.net | ||
http://a.b-c.de | ||
|
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 it worth adding an invalid URL for each of the cases here (beyond
/
which is already added)?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 don't feel particularly strongly either way.
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'd say that we shouldn't be able to make incorrect changes to the regex without test failures. For example, the
@
can be removed from the first group without failures and the second group can be reverted to\S
without failures.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've added some test cases that should cover all possible regressions in this regex change.