Django

Code

Ticket #4453 (closed: fixed)

Opened 1 year ago

Last modified 1 year ago

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

Reported by: forest@alittletooquiet.net Assigned to: jacob
Milestone: Component: Uncategorized
Version: SVN Keywords: url reverse dot
Cc: Triage Stage: Accepted
Has patch: 0 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

reverse_helper.txt (4.8 kB) - added by Todd O'Bryan <toddobryan@mac.com> on 06/16/07 00:34:01.
patch against django.core.urlresolvers

Change History

06/01/07 09:58:36 changed by anonymous

  • keywords set to url reverse dot.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

06/11/07 01:05:45 changed by SmileyChris

  • status changed from new to closed.
  • resolution set to wontfix.

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.

06/11/07 05:02:25 changed by mtredinnick

  • status changed from closed to reopened.
  • resolution deleted.

It's a bug.

06/11/07 17:05:51 changed by SmileyChris

  • stage changed from Unreviewed to Accepted.

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

06/11/07 20:19:19 changed by Forest Bond <forest@alittletooquiet.net>

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.

06/16/07 00:33:11 changed by Todd O'Bryan <toddobryan@mac.com>

  • summary changed from url pattern name can contain dots, but reverse('pattern.name.with.dots') will always fail to 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.

06/16/07 00:34:01 changed by Todd O'Bryan <toddobryan@mac.com>

  • attachment reverse_helper.txt added.

patch against django.core.urlresolvers

06/16/07 00:35:22 changed by Todd O'Bryan <toddobryan@mac.com>

  • has_patch set to 1.
  • component changed from Uncategorized to Template system.

06/16/07 00:57:29 changed by SmileyChris

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

My patch in #2977 would also address this.

06/16/07 06:27:48 changed by Forest Bond <forest@alittletooquiet.net>

  • component changed from Template system to Uncategorized.
  • summary changed from django.core.url_resolvers.reverse_url doesn't reverse escaped characters to 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.

06/16/07 06:29:21 changed by Forest Bond <forest@alittletooquiet.net>

  • has_patch deleted.

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

06/16/07 09:25:15 changed by Todd O'Bryan <toddobryan@mac.com>

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?

06/16/07 22:16:11 changed by Todd O'Bryan <toddobryan@mac.com>

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 ) 06/17/07 06:59:59 changed by Forest Bond <forest@alittletooquiet.net>

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 ) 06/17/07 07:00:43 changed by Forest Bond <forest@alittletooquiet.net>

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

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

Thanks *for* sticking around, that is.

06/24/07 23:34:34 changed by mtredinnick

  • status changed from reopened to closed.
  • resolution set to fixed.

(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).


Add/Change #4453 (url pattern name can contain dots, but reverse('pattern.name.with.dots') will always fail)




Change Properties
Action