Opened 4 years ago

Closed 12 months ago

#16361 closed Bug (fixed)

IGNORABLE_404 functionality should respect APPEND_SLASH setting

Reported by: Leo 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 aaugustin)

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 Changed 4 years ago by Leo

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Oops, sorry, that should have been r16160 above.

edit: just fixed in the summary.

Last edited 4 years ago by aaugustin (previous) (diff)

comment:2 Changed 4 years ago by lukeplant

  • Component changed from Core (Other) to Documentation
  • Triage Stage changed from Unreviewed to Accepted

Accepted as a documentation fix.

comment:3 Changed 4 years ago by aaugustin

  • Description modified (diff)

comment:4 follow-up: Changed 4 years ago by 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)

comment:5 in reply to: ↑ 4 Changed 4 years ago by lukeplant

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 Changed 4 years ago by aaugustin

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 Changed 2 years ago by aaugustin

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 Changed 12 months ago by timgraham

  • Resolution set to fixed
  • Status changed from new to closed

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