Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#17492 closed Bug (fixed)

url backreferencing can not be reversed.

Reported by: nate_b Owned by: nate_b
Component: Core (Other) Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If a url pattern uses backreferencing (i.e., "(?P=previously_named_group)"), the reversal fails, throwing up "Non-reversible reg-exp portion" ValueError. The problem occurs in django.utils.regex_helper.normalize which gives up on any character following a "(?P" other than "<".

Attachments (2)

17492.diff (5.9 KB) - added by SmileyChris 3 years ago.
regex_backreference.diff (6.7 KB) - added by nate_b 3 years ago.
A patch to regex_helper.py, along with unit tests for normalize.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by nate_b

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to nate_b
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 3 years ago by nate_b

  • Has patch set

This patch resolves the error, allowing for named group backreferencing in url patterns.

It also includes unittests for regex_helper.normalize, testing most of its functionality (namely: empty pattern, escaped characters, ignored groups, non-capturing group, positional group, named group, and finally, backreferenced group, of course).

Last edited 3 years ago by nate_b (previous) (diff)

comment:3 Changed 3 years ago by nate_b

After being encouraged to on IRC - here's my rationale:

I had something like this in my urlpatterns:

url(r'^static_text/(?P<group_one>[a-z][a-z_]*)/(?P<group_two>\d+)/(?P=group_one)_(?P=group_two)_(?P<group_three>\w+)/$', views.a_view)

Because "(?P=" wasn't being parsed, it was choking on reversing... a DIFFERENT pattern. So, I figured the right thing to do would be to recognize this pattern too, since backreferences are so simple (i.e., they have to only have an identifier name between '=' and ')', and that identifier name had to previously have occurred).

Now, in my patch, I don't check that the backreference actually occurred! BUT - that's okay, because if your url pattern has that problem, it didn't compile anyhow, and you never got to 'reverse' anyhow (that, and I reasoned that this behavior is in line with other omissions that 'normalize' allows).

Changed 3 years ago by SmileyChris

comment:4 Changed 3 years ago by SmileyChris

  • Patch needs improvement set

I've added in a urlpatterns_reverse test, and it identifies a problem. See the commented test line in there:

Currently, reversing a named pattern still works with a list of arguments as well as a dictionary. Using a named backreference doesn't work with a list of arguments in the same way (you shouldn't have to provide the name twice).

Unless this can be resolved, I'm not quite sure that this can be added.

Changed 3 years ago by nate_b

A patch to regex_helper.py, along with unit tests for normalize.

comment:5 Changed 3 years ago by nate_b

  • Patch needs improvement unset

Very good catch! Thankfully, it has a simple solution - None can be passed in as the variable, which excludes it. With my replacement patch, your test cases now both pass.

Last edited 3 years ago by nate_b (previous) (diff)

comment:6 Changed 3 years ago by SmileyChris

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

In [17336]:

Fixed #17492 -- Allow reversal of named backreferences. Thanks nate_b

comment:7 Changed 3 years ago by SmileyChris

(added nate_b to AUTHORS in r17337)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.