Opened 17 years ago

Closed 17 years ago

#4453 closed (fixed)

url pattern name can contain dots, but reverse('pattern.name.with.dots') will always fail

Reported by: forest@… Owned by: Jacob
Component: Uncategorized Version: dev
Severity: Keywords: url reverse dot
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

It seems somewhat natural to include dots in URL pattern names in an effort to avoid name clashes. However, the reverse resolver will always fail to find the correct URL pattern if that is done.

Attachments (1)

reverse_helper.txt (4.8 KB ) - added by Todd O'Bryan <toddobryan@…> 17 years ago.
patch against django.core.urlresolvers

Download all attachments as: .zip

Change History (16)

comment:1 by anonymous, 17 years ago

Keywords: url reverse dot added

comment:2 by Chris Beaven, 17 years ago

Resolution: wontfix
Status: newclosed

I'm not sure that there's a real problem here. It's not that "natural" to include dots in pattern names when you know dots are used to find out modules. My suggestion: use dashes or underscores. Or even spaces.

comment:3 by Malcolm Tredinnick, 17 years ago

Resolution: wontfix
Status: closedreopened

It's a bug.

comment:4 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedAccepted

I'm still a bit confused, but Malcolm's smarter than me. Accepted it is.

comment:5 by Forest Bond <forest@…>, 17 years ago

Well, regardless of whether dots should be accepted in view names, it's a bug to allow them and then not deal with them effectively in reverse. If django doesn't want dots there, it would be nice to see an exception when the URLs are read.

Personally, I see no reason dots shouldn't be allowed. I looked at the code that's choking on them, and I distinctly remember thinking that the breakage is due to a pretty straight-forward shortcoming that one often sees in first-iteration code.

comment:6 by Todd O'Bryan <toddobryan@…>, 17 years ago

Summary: url pattern name can contain dots, but reverse('pattern.name.with.dots') will always faildjango.core.url_resolvers.reverse_url doesn't reverse escaped characters

It's actually not just dots, but any backslash-escaped character in a url. For example,

urlpatterns = patterns('',
    (r'^prices/less_than_\$(?P<price>\d+)/$', 'cost_less'),                   
    (r'^headlines/(?P<year>/d+)\.(?P<month>\d+)\.(?P<day>\d+)/$', 'daily_headlines'),
    (r'^priests/(?P<name>\w+)\+/$', 'priest_homepage'),
    (r'^windows_path/(?P<drive_name>[A-Z]):\\\\(?P<path>.+)', 'windows_path'),
)

The dollar sign, dot, plus, and backslash in each of the URL patterns match a single character, but don't get converted back to that character by the reverse function.

It seems that there aren't that many of these. Any escape sequence that doesn't match a constant string (i.e. something like \s or \d or \w) had better be part of a pattern so that it can be replaced with the right string to get the URL you're expecting. That leaves the following, I think.

Pattern Replacement
\A '' (equivalent to ^)
\Z '' (equivalent to $)
\b and \B '' (these shouldn't appear in urls, but can only match the empty string)
\., \^, \$, \*, \+, \?, \(, \), \{, \}, \[, \], and \\ the same character, without a backslash

As a first stab, I'd just get rid of \A, \Z, \b, and \B, just as the current code does for ^ and $. This is actually kind of complicated, because you have to make sure that the \ in front isn't part of a pair of backslashes. In other words, \\b should become \b, but \\\b should just become \. Also, the current code removes all ^ and $. That's wrong if they're preceded by a backslash and meant to be the actual character.

There are some gotchas--when you insert values, you have to escape characters that you'll be unescaping later. I do check for character classes that don't map to a single definite character (e.g., \d and \w) and raise an exception if they're still there when we finish (since the reverse lookup can't work). I don't check for things like [a-z] or a{2,3}, but that will almost guarantee the reversing fails, too.

by Todd O'Bryan <toddobryan@…>, 17 years ago

Attachment: reverse_helper.txt added

patch against django.core.urlresolvers

comment:7 by Todd O'Bryan <toddobryan@…>, 17 years ago

Component: UncategorizedTemplate system
Has patch: set

comment:8 by Chris Beaven, 17 years ago

Ooh, I get it now. I was thinking of named URL patterns, not the pattern itself.

My patch in #2977 would also address this.

comment:9 by Forest Bond <forest@…>, 17 years ago

Component: Template systemUncategorized
Summary: django.core.url_resolvers.reverse_url doesn't reverse escaped charactersurl pattern name can contain dots, but reverse('pattern.name.with.dots') will always fail

No, no, no! I really did mean dots in pattern names:

urlpatterns = patterns('',
  (r'^xyz/$', 'some.name.with.dots'),                   
)

Todd, you've obviously spent a fair amount of time characterizing your bug, however, please open a new ticket, since it is a distinct issue, and not the originally reported one. I apologize if my original report was not clear enough.

comment:10 by Forest Bond <forest@…>, 17 years ago

Has patch: unset

* Please ignore patch attached to this ticket, as it fixes the wrong bug. *

comment:11 by Todd O'Bryan <toddobryan@…>, 17 years ago

OK...now I don't understand what you're talking about...

What you seem to be calling a pattern name is actually the path to the view, and it works fine with dots in it, as far as I can tell. In fact, I have lots of {% url blah.blah2.blah3 %} tags in my templates and they seem to do the right thing.

Can you provide a test case that does what you're talking about?

comment:12 by Todd O'Bryan <toddobryan@…>, 17 years ago

Aha!!! Your example is the path to the view function--I just didn't know about the new names functionality. You're having trouble with this

urlpatterns = patterns('',
  url(r'^xyz/$', 'path.to.view', name='some.name.with.dots'),                   
)

It doesn't seem like that would be too hard to fix. Since the URL namespace is global, you'd just have to check the name dictionary before you try checking view paths. I'll see if I can figure that out.

Also, I will happily open a new ticket for my problem and apologize for grabbing your issue because I was misunderstanding.

comment:13 by Forest Bond <forest@…>, 17 years ago

Ah, crud. Sorry, i mis-typed that example in a rush. Your example correctly illustrates the bug.

Misunderstanding understandable. No hard feelings on this end, and thanks sticking around :)

in reply to:  13 comment:14 by Forest Bond <forest@…>, 17 years ago

Replying to Forest Bond <forest@alittletooquiet.net>:

No hard feelings on this end, and thanks sticking around :)

Thanks *for* sticking around, that is.

comment:15 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: reopenedclosed

(In [5530]) Fixed #4453 -- Allow dots in URL pattern names (although the string in that case is first tried as an import path and only then falls back to being treated as a pattern).

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