Opened 4 years ago

Last modified 4 years ago

#31311 closed Cleanup/optimization

Unneeded escape sequences in character classes — at Version 4

Reported by: Kimball Leavitt Owned by: nobody
Component: Core (Other) Version: 3.0
Severity: Normal Keywords: validators, regex
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Kimball Leavitt)

Change History (4)

comment:1 by Carlton Gibson, 4 years ago

Has patch: unset
Triage Stage: UnreviewedAccepted

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.

comment:2 by Kimball Leavitt, 4 years ago

Has patch: set

comment:3 by Carlton Gibson, 4 years ago

Triage Stage: AcceptedReady for checkin

PR

Yep, this looks fine. Thank you.

comment:4 by Kimball Leavitt, 4 years ago

Description: modified (diff)
Triage Stage: Ready for checkinAccepted

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).

Ah, you beat me to it lol.

Last edited 4 years ago by Kimball Leavitt (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top