#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 Changed 18 months ago by Claude Paroz

Component: UncategorizedCore (Other)
Easy pickings: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization
Version: 3.1master

comment:2 Changed 18 months ago by Akshat Dixit

Owner: changed from nobody to Akshat Dixit
Status: newassigned

comment:3 Changed 18 months ago by Akshat Dixit

Has patch: set
Needs tests: set
Triage Stage: AcceptedReady for checkin

comment:4 Changed 18 months ago by Mariusz Felisiak

Triage Stage: Ready for checkinAccepted

This ticket is not ready for checkin (see Triage stages). Also you shouldn't mark your own patches as ready for checkin.

comment:5 in reply to:  4 Changed 18 months ago by Akshat Dixit

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 Changed 18 months ago by Akshat Dixit

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 Changed 18 months ago by Claude Paroz

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 Changed 18 months ago by Akshat Dixit

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 Changed 18 months ago by Akshat Dixit

Needs tests: unset

Tests and patch available at

https://github.com/django/django/pull/13811

Last edited 18 months ago by Akshat Dixit (previous) (diff)

comment:10 Changed 18 months ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

comment:11 Changed 18 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In b41d38a:

Fixed #32298 -- Fixed URLValidator hostname length validation.

URLValidator now validates the maximum length of a hostname without
the userinfo and port.

Note: See TracTickets for help on using tickets.
Back to Top