[Django] #36007: Dead code in URLValidator

13 views
Skip to first unread message

Django

unread,
Dec 12, 2024, 5:43:11 PM12/12/24
to django-...@googlegroups.com
#36007: Dead code in URLValidator
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Type:
| Cleanup/optimization
Status: new | Component:
| Uncategorized
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
There's some dead code in django.core.validators.URLValidator which could
be removed:

* The entire
[https://github.com/django/django/blob/6f38697f90a14f1450a71c1e40aea0f5df7dca86/django/core/validators.py#L183-L193
"Trivial case failed. Try for possible IDN domain"] section is no longer
useful. This code attempts to re-validate a failed URL after encoding an
international domain name using IDNA 2003 ("punycode"). But the
URLValidator regular expressions have allowed ''all'' IDNs since
[https://github.com/django/django/commit/2e65d56156b622e2393dee1af66e9c799a51924f
Commit 2e65d56] (for #20003, in 2015), so the super call will never fail
with a validation error that switching to IDNA 2003 would let pass.

* The
[https://github.com/django/django/blob/6f38697f90a14f1450a71c1e40aea0f5df7dca86/django/core/validators.py#L131
`ul` unicode letters property] is no longer used. The regular expressions
that had used it were moved into DomainNameValidator in
[https://github.com/django/django/commit/4971a9afe5642569f3dcfcd3972ebb39e88dd457
Commit 4971a9a] (for #18119, in Django 5.1).

For the first case, one way to verify the code is no longer in use is to
run URLValidator on `https://މިހާރު.com`, which is a domain allowed by
IDNA 2008 but prohibited by IDNA 2003. If the punycode() branch were
coming into play, that URL would be rejected:

{{{#!python
from django.core.validators import URLValidator
URLValidator()("https://މިހާރު.com")
# (No error)

from django.utils.encoding import punycode
punycode("މިހާރު.com")
# UnicodeError: Violation of BIDI requirement 3
# encoding with 'idna' codec failed
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36007>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 12, 2024, 5:57:21 PM12/12/24
to django-...@googlegroups.com
#36007: Dead code in URLValidator
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Uncategorized | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mike Edmunds):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36007#comment:1>

Django

unread,
Dec 13, 2024, 4:11:39 AM12/13/24
to django-...@googlegroups.com
#36007: Dead code in URLValidator
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Claude Paroz):

* component: Uncategorized => Core (Other)
* stage: Unreviewed => Ready for checkin
* version: 5.1 => dev

--
Ticket URL: <https://code.djangoproject.com/ticket/36007#comment:2>

Django

unread,
Dec 13, 2024, 9:37:53 AM12/13/24
to django-...@googlegroups.com
#36007: Dead code in URLValidator
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by JaeHyuckSa):

* owner: (none) => Mike Edmunds
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36007#comment:3>

Django

unread,
Dec 13, 2024, 10:10:44 AM12/13/24
to django-...@googlegroups.com
#36007: Dead code in URLValidator
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: closed
Component: Core (Other) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

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/36007#comment:5>

Django

unread,
Dec 13, 2024, 10:10:44 AM12/13/24
to django-...@googlegroups.com
#36007: Dead code in URLValidator
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"9a891c387f752b1fa7c4d7b436cd8491e8f16eca" 9a891c38]:
{{{#!CommitTicketReference repository=""
revision="9a891c387f752b1fa7c4d7b436cd8491e8f16eca"
Refs #36007 -- Added IDNA 2008 test case for URLValidator.

Test a domain that is valid under IDNA 2008 but not IDNA 2003. This
helps verify that the branch in URLValidator which calls punycode() is
not actually being used for IDNs. punycode() implements IDNA 2003, so
the domain would fail to validate if that branch were active for IDNs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36007#comment:4>
Reply all
Reply to author
Forward
0 new messages