[Django] #32930: URLValidator does not accept urls with port number < 10
Django
----------------------------------------+------------------------------
Reporter: Wu Haotian | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 3.2
Severity: Normal | Keywords: URLValidator
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------+------------------------------
{{{
>>> from django.core.validators import URLValidator
>>> URLValidator()("https://code.djangoproject.com:1")
Traceback (most recent call last):
.....
raise ValidationError(self.message, code=self.code, params={'value':
value})
django.core.exceptions.ValidationError: <unprintable ValidationError
object>
}}}
The commit[1] for fixing Ticket #20003 added port length checks in
URLValidator, but I didn't see why in the ticket comments or commit
description.
Given that:
The URL Standard from WhatWG[2] defines URL Port as:
A URL’s port is either null or a 16-bit unsigned integer that
identifies a networking port. It is initially null.
RFC 3986[3] defines "port" as "*DIGIT", and does not requires the length
of port.
I believe URLs with port number < 10 should be valid URLs.
[1]
https://github.com/django/django/commit/2e65d56156b622e2393dee1af66e9c799a51924f
#diff-d9609d8dc8482b30eac30df16213cba134562949fd62c97573927b89e880f85bR85
[2] https://url.spec.whatwg.org/#concept-url-port
[3] https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.3
--
Ticket URL: <https://code.djangoproject.com/ticket/32930>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
------------------------------+--------------------------------------
Reporter: Wu Haotian | Owner: Wu Haotian
Type: Bug | Status: assigned
Component: Core (Other) | Version: 3.2
Keywords: URLValidator | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Wu Haotian):
* owner: nobody => Wu Haotian
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:1>
Django
------------------------------+--------------------------------------
Reporter: Wu Haotian | Owner: Wu Haotian
Type: Bug | Status: assigned
Keywords: URLValidator | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Nick Pope):
* stage: Unreviewed => Accepted
Comment:
So I looked into why it was added in
2e65d56156b622e2393dee1af66e9c799a51924f but the best I could come up with
is that is was copied from [https://gist.github.com/dperini/729294 this
gist]. There are unaddressed comments, e.g.
[https://gist.github.com/dperini/729294#gistcomment-1563577 this one],
about this issue with the port number and other things.
Note that
[https://github.com/django/django/commit/2e65d56156b622e2393dee1af66e9c799a51924f
#diff-d9609d8dc8482b30eac30df16213cba134562949fd62c97573927b89e880f85bR120
this line] is also affected and will need fixing.
We can change `(?::\d{2,5})?` to `(?::\d{1,5})?` to fix this naively.
Or we can be a little more strict as the port should only be a 16-bit
unsigned integer (0-65535) and use the following to disallow 65536 and
higher:
{{{
(?:[0-9]|[1-9][0-9]{1,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])?
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:2>
Django
------------------------------+--------------------------------------
Reporter: Wu Haotian | Owner: Wu Haotian
Type: Bug | Status: assigned
Keywords: URLValidator | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Collin Anderson, Florian Apolloner (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:3>
Django
------------------------------+--------------------------------------
Reporter: Wu Haotian | Owner: Wu Haotian
Type: Bug | Status: assigned
Keywords: URLValidator | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Replying to [comment:2 Nick Pope]:
> Or we can be a little more strict as the port should only be a 16-bit
unsigned integer (0-65535) and use the following to disallow 65536 and
higher:
> {{{
>
(?:[0-9]|[1-9][0-9]{1,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])?
> }}}
I would keep the regex simple and perform range validation on it
afterwards… FWIW I do not think that 0 would be a valid port for a URL
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:4>
Django
------------------------------+--------------------------------------
Reporter: Wu Haotian | Owner: Wu Haotian
Type: Bug | Status: assigned
Keywords: URLValidator | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Wu Haotian):
It's hard to validating port using regex only.. Given that `0443` and
`000080` are both valid ports in URL.
{{{
>>> import requests
>>> requests.get("https://code.djangoproject.com:0443")
<Response [200]>
>>> requests.get("https://code.djangoproject.com:00000443")
<Response [200]>
}}}
{{{
> curl -I http://example.com:0000000000080
HTTP/1.1 200 OK
Content-Encoding: gzip
Accept-Ranges: bytes
Age: 591287
Cache-Control: max-age=604800
Content-Type: text/html; charset=UTF-8
Date: Thu, 15 Jul 2021 09:51:52 GMT
Etag: "3147526947"
Expires: Thu, 22 Jul 2021 09:51:52 GMT
Last-Modified: Thu, 17 Oct 2019 07:18:26 GMT
Server: ECS (sab/5693)
X-Cache: HIT
Content-Length: 648
}}}
I'd prefer to use `\d+` -- it's not perfect, but should produce fewer
false negatives.
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:5>
Django
------------------------------+--------------------------------------
Reporter: Wu Haotian | Owner: Wu Haotian
Type: Bug | Status: assigned
Keywords: URLValidator | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/14650 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:6>
Django
------------------------------+--------------------------------------
Reporter: Wu Haotian | Owner: Wu Haotian
Type: Bug | Status: assigned
Keywords: URLValidator | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Wu Haotian | Owner: Wu
| Haotian
Type: Bug | Status: assigned
Keywords: URLValidator | 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 Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Wu Haotian | Owner: Wu
| Haotian
Component: Core (Other) | Version: 3.2
Keywords: URLValidator | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"65b880b7268dd8fe97fde5af77bede46305eb499" 65b880b]:
{{{
#!CommitTicketReference repository=""
revision="65b880b7268dd8fe97fde5af77bede46305eb499"
Fixed #32930 -- Fixed URLValidator when port numbers < 10.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32930#comment:9>