Opened 5 years ago

Closed 18 months ago

#16362 closed Cleanup/optimization (fixed)

Ignore, rather than disallow, negative lookahead assertions

Reported by: charles@… Owned by: Bas Peschier
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@… 5 years ago.
Initial proposed patch

Download all attachments as: .zip

Change History (9)

Changed 5 years ago by charles@…

Initial proposed patch

comment:1 Changed 5 years ago by Adam K

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

comment:2 Changed 5 years ago by Aymeric Augustin

Component: UncategorizedCore (Other)
Needs documentation: set
Needs tests: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/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 5 years ago by bmihelac

Cc: bmihelac@… added

comment:4 Changed 4 years ago by Aymeric Augustin

Component: Core (Other)Core (URLs)

comment:5 Changed 3 years ago by Simon Meers

Cc: DrMeers@… added

comment:6 Changed 18 months ago by Bas Peschier

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Bas Peschier
Status: newassigned

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 18 months ago by Tim Graham

Triage Stage: AcceptedReady for checkin

Looks good, pending some cosmetic comments.

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

Resolution: fixed
Status: assignedclosed

In b4382b7:

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

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