Code

Opened 3 years ago

Last modified 14 months ago

#16361 new Bug

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.

Attachments (0)

Change History (7)

comment:1 Changed 3 years ago by Leo

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

Oops, sorry, that should have been r16160 above.

Version 0, edited 3 years ago by Leo (next)

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

  • Description modified (diff)

comment:4 follow-up: Changed 3 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 3 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 3 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 14 months 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.