[Django] #20003: URLValidator does not accept urls with usernames or passwords in them
Django
-------------------------------+--------------------------
Reporter: marshall@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Severity: Normal | Keywords: URLValidator
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------
{{{
In [1]: URLValidator()('https://user:pass@from django.core.validators
import URLValidator
In [2]: from django.core.validators
importURLValidator()('https://user:pa...@domain.com')
---------------------------------------------------------------------------
ValidationError Traceback (most recent call
last)
<ipython-input-2-3adf7dbac94c> in <module>()
----> 1 URLValidator()('https://user:pa...@domain.com')
/Users/marshall/.virtualenvs/django/lib/python2.7/site-
packages/django/core/validators.pyc in __call__(self, value)
72 raise e
73 url = urlparse.urlunsplit((scheme, netloc, path,
query, fragment))
---> 74 super(URLValidator, self).__call__(url)
75 else:
76 raise
/Users/marshall/.virtualenvs/django/lib/python2.7/site-
packages/django/core/validators.pyc in __call__(self, value)
42 """
43 if not self.regex.search(smart_unicode(value)):
---> 44 raise ValidationError(self.message, code=self.code)
45
46 class URLValidator(RegexValidator):
ValidationError: [u'Enter a valid value.']
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20003>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: marshall@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: URLValidator | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by marshall@…):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Sorry, readline munged the display, what you should see for the first two
lines are:
{{{
In [1]: from django.core.validators import URLValidator
In [2]: URLValidator()('https://user:pa...@domain.com')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:1>
Django
Reporter: marshall@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ptone):
* stage: Unreviewed => Accepted
Comment:
http://url.spec.whatwg.org/#concept-url-userinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:2>
Django
Reporter: marshall@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* component: Uncategorized => Core (Other)
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:3>
Django
Reporter: marshall@… | Owner: matiasb
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by matiasb):
* status: new => assigned
* owner: nobody => matiasb
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:4>
Django
Reporter: marshall@… | Owner: matiasb
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by matiasb):
* has_patch: 0 => 1
Comment:
Proposed update: https://github.com/django/django/pull/998
Tests passing with sqlite.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:5>
Django
Reporter: marshall@… | Owner: matiasb
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by apollo13):
See my comments on the pull request
(https://github.com/django/django/pull/998#issuecomment-18097508)
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:6>
Django
Reporter: marshall@… | Owner: matiasb
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:7>
Django
Reporter: marshall@… | Owner: matiasb
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by aaugustin):
#21532 was a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:8>
Django
Reporter: marshall@… | Owner: matiasb
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by dbrgn):
These URLs could be used for a thorough test case:
http://mathiasbynens.be/demo/url-regex
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:9>
Django
Reporter: marshall@… | Owner: matiasb
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by dbrgn):
* cc: gezuru@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:10>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by dbrgn):
* owner: matiasb =>
* needs_better_patch: 1 => 0
* status: assigned => new
Comment:
Another attempt at fixing this issue (including an extended test suite):
https://github.com/django/django/pull/2873
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:11>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Florian and I (and some other core devs I think) are wary of continuing to
expand the regex for every use case out there. The validator allows
specifying a custom `regex` and I think we should consider discontinuing
"enhancements" to the core regex and let users bring their own as needed.
If we go this route, we should beef up the documentation about what is and
isn't supported by default.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:12>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dbrgn):
If that is the plan, the documentation really does need an update. The
validator claims to validate URLs. Most people who use the URL Validator
assume it will validate URLs. But in the current state it is broken and
does not. It would be fine if it would consider some invalid URL edge
cases valid (meaning it's not too strict), but if it returns a
ValidationError for perfectly valid URLs, that's a clear bug to me.
Even if you look online for other ways to validate URLs in Python, you get
pointed to the Django implementation:
http://stackoverflow.com/q/7160737/284318
I think this is a bug that should be addressed somehow. Either by a fix
(possibly with a different implementation than the approach I took) or by
a deprecation of the validator.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:13>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
I'm on Danilo's side on this issue, I don't see why we wouldn't improve
the regex if we have a good patch with solid tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:14>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
Are there possibly any security issues with including a username in the
url?
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:15>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dbrgn):
@collinanderson I don't think so. The job of the URLValidator is to test
whether an URL is valid according to some RFCs, not to decide whether a
specific URL is a good idea in a specific case :)
It could have been a security issue if `verify_exists` were still
available (e.g. because the credentials would show up in network traffic),
but as this functionality has been removed in 1.5 that's not a concern
anymore.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:16>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
The proposed change appears vulnerable to the "catastrophic backtracking
situation" described in 9f8287a3f145fe5cbe71ef5573ea8898404727ad as the
test added there now hangs with the proposed changes. This is one reason
I'm reluctant to add regex complexity.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:17>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by dbrgn):
* needs_better_patch: 1 => 0
Comment:
I think I fixed it:
https://github.com/django/django/pull/2873#issuecomment-52542276
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:18>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Some tests on Python 3 are failing.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:19>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by dbrgn):
* needs_better_patch: 1 => 0
Comment:
Pull request has been updated. All tests now pass on all supported Python
versions.
https://github.com/django/django/pull/2873
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:20>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by loic):
Left some comments on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:21>
Django
Reporter: marshall@… | Owner:
Type: Bug | Status: new
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution:
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dbrgn):
I left some comments as well.
Any opinions on the proposed RegEx simplifications?
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:22>
Django
Reporter: marshall@… | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Component: Core (Other) | Version: 1.5
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* owner: => Tim Graham <timograham@…>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"2e65d56156b622e2393dee1af66e9c799a51924f"]:
{{{
#!CommitTicketReference repository=""
revision="2e65d56156b622e2393dee1af66e9c799a51924f"
Fixed #20003 -- Improved and extended URLValidator
This adds support for authentication data (`user:password`) in URLs,
IPv6 addresses, and unicode domains.
The test suite has been improved by adding test URLs from
http://mathiasbynens.be/demo/url-regex (with a few adjustments,
like allowing local and reserved IPs).
The previous URL validation regex failed this test suite on 13
occasions, the validator was updated based on
https://gist.github.com/dperini/729294.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:23>
Django
Reporter: marshall@… | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Component: Core (Other) | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Comment (by Sarah Boyce <42296566+sarahboyce@…>):
In [changeset:"54059125956789ad4c19b77eb7f5cde76eec0643" 5405912]:
{{{#!CommitTicketReference repository=""
revision="54059125956789ad4c19b77eb7f5cde76eec0643"
Fixed #36007 -- Removed dead code from URLValidator.
The "Trivial case failed. Try for possible IDN domain" handling was
obsoleted by ticket-20003, which adjusted the regular expressions to
allow all international domain names (Refs #20003).
Uses of `ul` were moved to DomainNameValidator in ticket-18119
(Refs #18119).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20003#comment:24>