[Django] #25620: URLValidator regex does not trigger on consecutive periods
Django
------------------------------+--------------------
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
The regular expression for
[https://docs.djangoproject.com/en/1.8/ref/validators/#urlvalidator
URLValidator] accepts consecutive periods as valid. This bug was
introduced in 1.8.3.
== Steps to Reproduce ==
{{{
#!python
>>> from django.core.validators import URLValidator
>>> validate = URLValidator()
>>> validate('http://example..com')
>>> validate('http://example...............com')
}}}
== Expected Result ==
A [https://docs.djangoproject.com/en/1.8/ref/exceptions/#validationerror
ValidationError] exception should be raised.
== Current Result ==
No exception is raised, and the URL is deemed valid.
== Reference ==
[https://tools.ietf.org/html/rfc2181#section-11 RFC 2181]:
The length of any one label is limited to between 1 and 63 octets. A
full domain name is limited to 255 octets (including the separators). The
zero length full name is defined as representing the root of the DNS
tree, and is typically written and displayed as ".".
--
Ticket URL: <https://code.djangoproject.com/ticket/25620>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by DheerendraRathor):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Here are the relevant code part from URLValidator (this is from the master
branch)
{{{
hostname_re = r'[a-z' + ul + r'0-9](?:[a-z' + ul + r'0-9-]*[a-z' + ul +
r'0-9])?'
domain_re = r'(?:\.(?!-)[a-z' + ul + r'0-9-]*(?<!-))*'
tld_re = r'\.(?:[a-z' + ul + r']{2,}|xn--[a-z0-9]+)\.?'
host_re = '(' + hostname_re + domain_re + tld_re + '|localhost)'
}}}
The culprit is domain_re which allows multiple dots due to '*' on [a-z0-9]
part. Changing * to + should solve the problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:1>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by DheerendraRathor):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:2>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by DheerendraRathor):
* Attachment "0001-Fixed-25620-Changed-to-in-domain-name-regex.patch"
added.
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
Could you submit the patch as a pull request?
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:3>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by DheerendraRathor):
Recently I've also noted that scheme regex is `r'^(?:[a-z0-9\.\-]*)://'`
but it should have been `r'^(?:[a-z0-9\.\-\+]+)://'` according to
[https://tools.ietf.org/html/rfc1738#section-2.1 rfc1738]. I'll update my
PR soon.
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:4>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by DheerendraRathor):
* cc: dheeru.rathor14@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:5>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by DheerendraRathor):
Also current regex are not handling label limit of 63 characters and total
limit of 253 characters. Should I modify regex to handle them as well?
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:6>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
It's probably better to handle each issue separately. Otherwise, it's
difficult to determine which change matches which test.
Making the regex more complex must be done very carefully to avoid issues
like 17d3a6d8044752f482453f5906026eaf12c39e8e.
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:7>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by DheerendraRathor):
Cool, then I'll modify regex for domain name and scheme. For length
validation I'll open another ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:8>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"96fe90f5356971e0e51a0bc41e045dde600d7521" 96fe90f]:
{{{
#!CommitTicketReference repository=""
revision="96fe90f5356971e0e51a0bc41e045dde600d7521"
Fixed #25620 -- Made URLValidator prohibit URLs with consecutive dots in
the domain section.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:9>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"540de2f7972c6aa68fbf36c6a0da137d768f2067" 540de2f]:
{{{
#!CommitTicketReference repository=""
revision="540de2f7972c6aa68fbf36c6a0da137d768f2067"
[1.8.x] Fixed #25620 -- Made URLValidator prohibit URLs with consecutive
dots in the domain section.
Backport of 96fe90f5356971e0e51a0bc41e045dde600d7521 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:11>
Django
Reporter: sully90h | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6bb9f51ab8e3cc56b05238ea012763ba775ab896" 6bb9f51]:
{{{
#!CommitTicketReference repository=""
revision="6bb9f51ab8e3cc56b05238ea012763ba775ab896"
[1.9.x] Fixed #25620 -- Made URLValidator prohibit URLs with consecutive
dots in the domain section.
Backport of 96fe90f5356971e0e51a0bc41e045dde600d7521 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25620#comment:10>