IGNORABLE_404 functionality should respect APPEND_SLASH setting

[Leo Shklovskii]

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.

[Leo Shklovskii]

Oops, sorry, that should have been r16160 above.

[Luke Plant]

Accepted as a documentation fix.

[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)

[Luke Plant]

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.

[Aymeric Augustin]

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.

[Aymeric Augustin]

I think this problem has essentially gone away because APPEND_SLASH doesn't append a slash to URLs that return a 404 any more.

[Tim Graham]

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