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: | 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)
Change History (16)
comment:1 by , 17 years ago
Keywords: | url reverse dot added |
---|
comment:2 by , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:4 by , 17 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I'm still a bit confused, but Malcolm's smarter than me. Accepted it is.
comment:5 by , 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 , 17 years ago
Summary: | url pattern name can contain dots, but reverse('pattern.name.with.dots') will always fail → django.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.
comment:7 by , 17 years ago
Component: | Uncategorized → Template system |
---|---|
Has patch: | set |
comment:8 by , 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 , 17 years ago
Component: | Template system → Uncategorized |
---|---|
Summary: | django.core.url_resolvers.reverse_url doesn't reverse escaped characters → url 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 , 17 years ago
Has patch: | unset |
---|
* Please ignore patch attached to this ticket, as it fixes the wrong bug. *
comment:11 by , 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 , 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.
follow-up: 14 comment:13 by , 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 :)
comment:14 by , 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 , 17 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
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.