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 Aymeric Augustin)

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:1 by Leo Shklovskii, 13 years ago

Oops, sorry, that should have been r16160 above.

Version 0, edited 13 years ago by Leo Shklovskii (next)

comment:2 by Luke Plant, 13 years ago

Component: Core (Other)Documentation
Triage Stage: UnreviewedAccepted

Accepted as a documentation fix.

comment:3 by Aymeric Augustin, 13 years ago

Description: modified (diff)

comment:4 by anonymous, 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)

in reply to:  4 comment:5 by Luke Plant, 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 Aymeric Augustin, 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 Aymeric Augustin, 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 Tim Graham, 10 years ago

Resolution: fixed
Status: newclosed

Marking as fixed per last comment. I don't see any docs to add at this point.

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