Opened 16 years ago
Closed 16 years ago
#9921 closed (duplicate)
request.urlconf incorrect behavoir
Reported by: | strelnikovdmitrij | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | 1.0 |
Severity: | Keywords: | request.urlconf | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
if dynamically load urls file through request.urlconf django will not append trailing slash
so if I will have rule
(r'^(?P<user>[a-zA-Z0-9-]{4,20})/$', 'user')
, it have to call it
http://site.com/user/
only,
http://site.com/user(wihtout slash) will not work and django will try load ROOT_URLCONF from settings file.
If I will have the same rules in ROOT_URLCONF file it will work correctly.
May be its happen due bad parsing hostname. Im using subdomains..so I have x.site.com - load x.urls to request.urlconf, y.site.com - load y.urls to request.urlconf. Prepend project name -project.x.urls doesnt solve problem. File is load correctly, but didnt append slash. Global append settings - On. Maybe bug in cache systems.
Don't know :(
Using latest stable 1.0.2 django, python 2.5, windows xp sp3.
Dmitrij
Change History (11)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
comment:3 by , 16 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Resolution: | duplicate |
Status: | closed → reopened |
SOLVED :
in django\middleware\common.py file near by line 55 (not sure, I've change some sources)
if settings.APPEND_SLASH and (not old_url[1].endswith('/')): if (not _is_valid_path(request.path_info) '''and''' _is_valid_path("%s/" % request.path_info)): new_url[1] = new_url[1] + '/'
have to be
if settings.APPEND_SLASH and (not old_url[1].endswith('/')): print "not end with /" if (not _is_valid_path(request.path_info) '''&''' _is_valid_path("%s/" % request.path_info)): new_url[1] = new_url[1] + '/' print "new_url[1] = %s " % new_url[1] if settings.DEBUG and request.method == 'POST':
for more info see the Python doc describing different between logical and arithmetical operators or look through folowing code executed in standart python shell ;)
>>> t = True >>> f = False >>> not t and f False >>> not t & f True
comment:4 by , 16 years ago
Resolution: | → duplicate |
---|---|
Status: | reopened → closed |
comment:5 by , 16 years ago
Resolution: | duplicate |
---|---|
Status: | closed → reopened |
comment:6 by , 16 years ago
I do not see how changing from using boolean to arithmetic/bitwise "AND" (but not "NOT") in code where we are dealing with boolean values could possibly be correct.
Given the precedence (http://docs.python.org/reference/expressions.html#summary) of boolean "AND" and "NOT", the code as written today is saying:
if (the path we have been asked to locate is not resolvable) and (adding a slash to the end it makes it resolvable) then (proceed with settings things up so we'll do a redirect to the added-slash version of the url)
When you change the boolean "AND" to a bitwise one, but leave the boolean "NOT", the precedence changes (bitwise "AND" has higher precedence than boolean "NOT") and the code then says:
if not ("the path as given is resolvable" and "adding a slash to the end makes it resolvable") then (proceed with settings things up so we'll do a redirect to the added-slash version of the url)
which means that any not-resolvable-as-originally-specified path will get redirected to the added-slash version, which is incorrect. We only want to do the redirect when adding the slash actually results in a resolvable url.
I'm not re-closing as a dupe, because I'm not sure what it's supposed to be a dupe of. reverse() is not involved here. I do see there are two other tickets (#3530 and #5034) with other cases where request.urlconf
is not being taken into account when it should be. James tried to get those consolidated into one but that doesn't seem to have stuck. It does seem like we have a general problem with request.urlconf
not being considered in all the places it should be, and it would be good if they were all found and fixed together rather than piecemeal (higher likelihood of doing it in a consistent fashion), but I don't really know enough about the code involved here to say whether one consistent fix is a reasonable goal or if these really all need to be fixed individually as they are tripped over.
comment:7 by , 16 years ago
thanks for reply and detail describing ;)
I've gone through code again :)
The result:
my changes goes back ;)
now look..
common.py
process_request(self, request): ... if (not _is_valid_path(request.path_info) and _is_valid_path("%s/" % request.path_info)): new_url[1] = new_url[1] + '/' ...
going to _is_valid_path(...
def _is_valid_path(path): try: urlresolvers.resolve(path) return True except urlresolvers.Resolver404: return False
going to urlresolvers.resolve(path)..
(file urlresolvers.py)
def resolve(path, urlconf=None): #urlconf is always None (I hope) return get_resolver(urlconf).resolve(path)
going to get_resolver(urlconf) # urlconf = None
def get_resolver(urlconf): if urlconf is None: from django.conf import settings urlconf = settings.ROOT_URLCONF #get default settings return RegexURLResolver(r'^/', urlconf) get_resolver = memoize(get_resolver, _resolver_cache, 1)
I think this should be better..and no operators will changed ;)
common.py !!! + means added
def process_request(self, request): ... host = request.get_host() old_url = [host, request.path] new_url = old_url[:] + urlconf = None + ... if (not _is_valid_path(request.path_info, +urlconf+) and _is_valid_path("%s/" % request.path_info, +urlconf+)): new_url[1] = new_url[1] + '/'
going to _is_valid_path..
def _is_valid_path(path, +urlconf=None+): try: urlresolvers.resolve(path, +urlconf+) return True except urlresolvers.Resolver404: return False
and urlresolvers.resolve will get correct urlconf from request, if there is no urlconf it will be getted from settings ;)
Any better ideas?
Thanks.
Dmitrij
comment:8 by , 16 years ago
sorry, forgot one piece of code
if settings.APPEND_SLASH and (not old_url[1].endswith('/')): #default None + if hasattr(request, 'urlconf'): + urlconf = request.urlconf #if not (_is_valid_path(request.path_info) and _is_valid_path("%s/" % request.path_info)): if (not _is_valid_path(request.path_info, + urlconf +) and _is_valid_path("%s/" % request.path_info, + urlconf +)): new_url[1] = new_url[1] + '/'
comment:10 by , 16 years ago
Smiley I agree with your assment, the issue is COMMON_MIDDLWEAR tries to see if it can reverse the url with a / appended. This is why I originally marked this as a dupe, it's a symptom, not a cause.
Marking as a duplicate, this is because reverse() doesn't take request.urlconf into account.