Opened 4 years ago
Closed 4 years ago
#32298 closed Cleanup/optimization (fixed)
django.core.validators.URLValidator tests netloc instead of hostname for length
| Reported by: | zt_initech | Owned by: | Akshat Dixit |
|---|---|---|---|
| Component: | Core (Other) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
This code: https://github.com/django/django/blob/master/django/core/validators.py#L141
urlparse: https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlparse
In case of url like
urlparse('https://username1:password1@example.com/foo')
the parse result is:
ParseResult(scheme='https', netloc='username1:password1@example.com', path='/foo', params='', query='', fragment='')
with the username and password in netloc.
Of course if the netloc is too long only because of the password, this causes a valid URL to be not accepted.
The test for length is a requirement for the hostname, so it should test .hostname instead of .netloc
Change History (11)
comment:1 by , 4 years ago
| Component: | Uncategorized → Core (Other) |
|---|---|
| Easy pickings: | set |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Cleanup/optimization |
| Version: | 3.1 → master |
comment:2 by , 4 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 4 years ago
| Has patch: | set |
|---|---|
| Needs tests: | set |
| Triage Stage: | Accepted → Ready for checkin |
follow-up: 5 comment:4 by , 4 years ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
This ticket is not ready for checkin (see Triage stages). Also you shouldn't mark your own patches as ready for checkin.
comment:5 by , 4 years ago
Replying to Mariusz Felisiak:
This ticket is not ready for checkin (see Triage stages). Also you shouldn't mark your own patches as ready for checkin.
I am sorry, I did not understand the terminology correctly. Thankyou for your correction.
comment:6 by , 4 years ago
Hello, Can someone help me in progressing this patch. I have submitted a patch which has passed all the build tests. I will also like to write tests for this bug.
comment:7 by , 4 years ago
The next step is to write a test, you may need to explore a bit the tests folder of Django to find out where other URL validation tests are located.
comment:8 by , 4 years ago
Added test of this case to
tests/validators/tests.py
where all other url validation tests are present and committed changes to same pull request. https://github.com/django/django/pull/13811.
comment:9 by , 4 years ago
| Needs tests: | unset |
|---|
Tests and patch availabe at
comment:10 by , 4 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
https://github.com/django/django/pull/13811