Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#11412 closed defect (bug) (duplicate)

Discard requests for favicon.ico

Reported by: Denis-de-Bernardy Owned by:
Priority: normal Milestone:
Component: Performance Version: 2.9
Severity: normal Keywords:
Cc:

Description

WP should not load in its entirety on favicon.ico requests, unless a plugin allows it to.

We should add the needed RewriteCond to the .htaccess statements. Or at the very least catch that sort of request early on (while we parse the request, for instance), apply filters in case a plugin wants to step in, and bail accordingly.

As things stand, every page load on a site without a favicon generates a 404 error that gets logged -- and on each of these, WP loads in its entirety, and hopelessly queries the database in an effort to canonically redirect to something that might work.

Attachments (2)

favicon.patch (89 bytes) - added by miqrogroove 3 years ago.
Problem solved :P
favicon-srs.patch (1.3 KB) - added by miqrogroove 3 years ago.
But on that note, I'm guessing the wordpress.org icon is more likely to receive approval.

Download all attachments as: .zip

Change History (16)

If it were up to me, I would commit a zero-byte favicon.ico to trunk and not touch a darn thing. +1 forward thinking though.

Problem solved :P

But on that note, I'm guessing the wordpress.org icon is more likely to receive approval.

if a user has has own favicon file, he probable doesn't want the WP favicon to overwrite his own. so this ought to be fixed at the php level. :-)

Personally, I'm picturing something like... at the very beginning of the main request:

http://core.trac.wordpress.org/browser/trunk/wp-includes/classes.php?rev=11930#L488

if preg_match favicon_request
  then if $file = apply_filters('wp_favicon, false) !== false
   then
    send appropriate headers
    readfile($file);
   endif
    else
   send send 404 headers
  endif
  die
endif

And #3426

One should be closed as duplicate of the other, probably.

Denis you're getting a little ahead of yourself there. The -f rule in .htaccess should make the readfile pattern.. well.. not needed.

comment:6 follow-up: ↓ 7   miqrogroove3 years ago

How about, in the installer/upgrader, check for file existence, then create the zero byte file on the fly.

comment:7 in reply to: ↑ 6   Denis-de-Bernardy3 years ago

Replying to miqrogroove:

How about, in the installer/upgrader, check for file existence, then create the zero byte file on the fly.

I like that idea...

we might as well add a WP icon in this case, though. :-P

Potential alternative solution, for those who have permalinks enabled:

RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_URI} /favicon.ico$
RewriteRule . $home_path/wp-includes/favicon.ico [L]

others would need to hook into a favicon hook as suggested above.

If it was really necessary to use mod_rewrite, this would be the correct syntax:

RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^favicon\.ico$ wp-includes/favicon.ico [L]

actually, the should be removed, and the $home_path is needed in the event the site is in a subfolder.

RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule favicon\.ico$ $home_path/wp-includes/favicon.ico [L]

I guess it depends how strict you want to be about duplicate content issues. I prefer to anchor when possible so that programs like Google Toolbar get 404'd when they fetch invalid paths.

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

This is a duplicate of: #3426

  • Milestone 3.0 deleted
Note: See TracTickets for help on using tickets.