Opened 13 years ago
Closed 10 years ago
#16361 closed Bug (fixed)
IGNORABLE_404 functionality should respect APPEND_SLASH setting
Reported by: | Leo Shklovskii | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
I was originally going to file this against IGNORABLE_404_ENDS, but it looks like r16160 rewrote that setting and the way these are configured, however, the bug still remains.
When APPEND_SLASH is enabled, CommonMiddleware issues the redirect on /favicon.ico
and a 404 for /favicon.ico/
ends up showing up in the logs.
If it's a pain to fix this issue given the new implementation, one alternative is to just add a note to the docs telling people to add a //?
to the end of their regex.
Change History (8)
comment:2 by , 13 years ago
Component: | Core (Other) → Documentation |
---|---|
Triage Stage: | Unreviewed → Accepted |
Accepted as a documentation fix.
comment:3 by , 13 years ago
Description: | modified (diff) |
---|
follow-up: 5 comment:4 by , 13 years ago
Why not do a re.sub()?
https://code.djangoproject.com/changeset/16160#file1
return any(pattern.search(re.sub("/$", "", uri)) for pattern in settings.IGNORABLE_404_URLS)
comment:5 by , 13 years ago
Replying to anonymous:
Why not do a re.sub()?
https://code.djangoproject.com/changeset/16160#file1
return any(pattern.search(re.sub("/$", "", uri)) for pattern in settings.IGNORABLE_404_URLS)
The solution is not as simple as that for the case where the pattern actually includes the trailing slash - I don't think you can avoid running over the patterns twice for the case where APPEND_SLASH = True.
In addition, the question is, do we want the trailing slash to be ignored because it might have been added due to CommonMiddleware
and APPEND_SLASH? We don't know that it has been added by CommonMiddleware
. Since it is generally more powerful left as it is, I think we should just add a note to the docs.
comment:6 by , 13 years ago
Another solution is to not append a slash to URLs matching IGNORABLE_404_URLS, because they are expected to be 404s. But I'm not sure it's better than the status quo.
comment:7 by , 12 years ago
I think this problem has essentially gone away because APPEND_SLASH doesn't append a slash to URLs that return a 404 any more.
comment:8 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Marking as fixed per last comment. I don't see any docs to add at this point.
Oops, sorry, that should have been r16160 above.
edit: just fixed in the summary.