Opened 4 years ago

Closed 5 months ago

#16362 closed Cleanup/optimization (fixed)

Ignore, rather than disallow, negative lookahead assertions

Reported by: charles@… Owned by: bpeschier
Component: Core (URLs) Version: 1.3
Severity: Normal Keywords:
Cc: adam@…, bmihelac@…, DrMeers@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

At present, django.utils.regexhelper.normalize() is defined and documented to raise an exception on encountering lookahead and lookbehind matches in the name of reversability. This behaviour unnecessarily restricts the range of valid configurations.

Consider the following case:

url(r'^(?P<city>[-\w._])/', name='city-landing'),

One may wish to assert that certain inputs are *certainly not* cities, and should never match this (in my particular use case, the desired outcome is actually a Resolver404, such that adding an additional, earlier url() entry to the urlconf is not an option):

url(r'^(?!NotACity)(?P<city>[-\w._])/', name='city-landing'),

This *does* mean that reverse('city-landing', kwargs={'name': 'NotACity'}) will result in an invalid result (a URL which, when resolved, does not in fact match the given reversed string). However, such a case clearly constitutes user error -- as the application developer trying to request a city-landing URL for NotACity is asking the impossible -- and does not constitute cause for making such patterns unsupported.

Attachments (1)

django-regex-allow-lookahead-16362.patch (1.5 KB) - added by charles@… 4 years ago.
Initial proposed patch

Download all attachments as: .zip

Change History (9)

Changed 4 years ago by charles@…

Initial proposed patch

comment:1 Changed 4 years ago by ahknight

  • Cc adam@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by aaugustin

  • Component changed from Uncategorized to Core (Other)
  • Needs documentation set
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

Reading the comment on #2977 and duplicate tickets, I think we can accept this. I'm a bit surprised that the patch is so simple, but why not.

Until now, lookahead/behind assertions were forbidden, so it can't be backwards incompatible.

The docs probably needs updating.

comment:3 Changed 4 years ago by bmihelac

  • Cc bmihelac@… added

comment:4 Changed 2 years ago by aaugustin

  • Component changed from Core (Other) to Core (URLs)

comment:5 Changed 18 months ago by DrMeers

  • Cc DrMeers@… added

comment:6 Changed 5 months ago by bpeschier

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to bpeschier
  • Status changed from new to assigned

It was never documented that it was not allowed except for the source comments above normalize. These assertions are part of a valid regular expression, so I only added a release note.

comment:7 Changed 5 months ago by timgraham

  • Triage Stage changed from Accepted to Ready for checkin

Looks good, pending some cosmetic comments.

comment:8 Changed 5 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In b4382b7:

Fixed #16362 -- Allowed lookaround assertions in URL patterns.

Note: See TracTickets for help on using tickets.
Back to Top