[Django] #31311: Unneeded escape sequences in character classes
Django
-------------------------------------+-------------------------------------
Reporter: kimbo | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core | Version: 3.0
(Other) |
Severity: Normal | Keywords: validators, regex
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
There are at least three places in the regex in django/core/validators.py
that have unnecessary escape sequences inside of character classes:
-
https://github.com/django/django/blob/master/django/core/validators.py#L68
-
https://github.com/django/django/blob/master/django/core/validators.py#L85
-
https://github.com/django/django/blob/master/django/core/validators.py#L162
--
Ticket URL: <https://code.djangoproject.com/ticket/31311>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Kimball Leavitt | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: 3.0
Severity: Normal | Resolution:
Keywords: validators, regex | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* has_patch: 1 => 0
* stage: Unreviewed => Accepted
Comment:
The third example here (L162) is:
{{{
r'((?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+)(?:[A-Z0-9-]{2,63}(?<!-))\Z',
}}}
I can't see an escape inside a character class there.
Might just be early?
I applied a quick adjustment, which seemed to run OK. (Existing tests
passed.)
{{{
diff --git a/django/core/validators.py b/django/core/validators.py
index dc0519bb6a..c19fa0becd 100644
--- a/django/core/validators.py
+++ b/django/core/validators.py
@@ -65,7 +65,7 @@ class URLValidator(RegexValidator):
# IP patterns
ipv4_re =
r'(?:25[0-5]|2[0-4]\d|[0-1]?\d?\d)(?:\.(?:25[0-5]|2[0-4]\d|[0-1]?\d?\d)){3}'
- ipv6_re = r'\[[0-9a-f:\.]+\]' # (simple regex, validated later)
+ ipv6_re = r'\[[0-9a-f:.]+\]' # (simple regex, validated later)
# Host patterns
hostname_re = r'[a-z' + ul + r'0-9](?:[a-z' + ul + r'0-9-]{0,61}[a-z'
+ ul + r'0-9])?'
@@ -82,7 +82,7 @@ class URLValidator(RegexValidator):
host_re = '(' + hostname_re + domain_re + tld_re + '|localhost)'
regex = _lazy_re_compile(
- r'^(?:[a-z0-9\.\-\+]*)://' # scheme is validated separately
+ r'^(?:[a-z0-9.\-+]*)://' # scheme is validated separately
r'(?:[^\s:@/]+(?::[^\s:@/]*)?@)?' # user:pass authentication
r'(?:' + ipv4_re + '|' + ipv6_re + '|' + host_re + ')'
r'(?::\d{2,5})?' # port
}}}
Happy to look more closely in a PR.
Thanks for the report.
--
Ticket URL: <https://code.djangoproject.com/ticket/31311#comment:1>
Django
Reporter: Kimball Leavitt | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: 3.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Kimball Leavitt):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31311#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Core (Other) | Version: 3.0
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/12498 PR]
Yep, this looks fine. Thank you.
--
Ticket URL: <https://code.djangoproject.com/ticket/31311#comment:3>
Django
Reporter: Kimball Leavitt | Owner: nobody
Component: Core (Other) | Version: 3.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Kimball Leavitt):
* stage: Ready for checkin => Accepted
Old description:
> There are at least three places in the regex in django/core/validators.py
> that have unnecessary escape sequences inside of character classes:
>
> -
> https://github.com/django/django/blob/master/django/core/validators.py#L68
> -
> https://github.com/django/django/blob/master/django/core/validators.py#L85
> -
> https://github.com/django/django/blob/master/django/core/validators.py#L162
New description:
There are at least three places in the regex in django/core/validators.py
that have unnecessary escape sequences inside of character classes:
-
https://github.com/django/django/blob/master/django/core/validators.py#L68
-
https://github.com/django/django/blob/master/django/core/validators.py#L85
-
https://github.com/django/django/blob/master/django/core/validators.py#L166
--
Comment:
Thanks for the review. I submitted a PR for this yesterday -
https://github.com/django/django/pull/12498/files.
You're right, the third one was wrong. I made the fix first, then
submitted the ticket, so I hastily grabbed the wrong line. I changed it so
it's correct
(https://github.com/django/django/blob/master/django/core/validators.py#L166).
--
Ticket URL: <https://code.djangoproject.com/ticket/31311#comment:4>
Django
Reporter: Kimball Leavitt | Owner: nobody
Component: Core (Other) | Version: 3.0
Keywords: validators, regex | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"7c6b66383da5f9a67142334cd2ed2d769739e8f1" 7c6b663]:
{{{
#!CommitTicketReference repository=""
revision="7c6b66383da5f9a67142334cd2ed2d769739e8f1"
Fixed #31311 -- Removed unneeded escapes in validator regexes.
Special characters lose their special meaning inside sets of characters.
"-" lose its special meaning if it's placed as the first or last
character.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31311#comment:5>