Let the web server handle missing 'favicon.ico' files.

[jnrbsn]

If a favicon.ico file is missing, the web server should handle it, not Cake.

[jnrbsn]

Some people have legitimate reasons for the actual favicon.ico file being missing. At a previous job of mine, we had some fancy apache config that was serving the same favicon for all virtual hosts, but the actual files did not exist where one might expect.

Also, the current code is kind of wonky anyway. The $_SERVER['PATH_INFO'] only applies when you're requesting index.php/favicon.ico (or if you're rewriting to that), which doesn't really happen. Even if the current code (that conditional) was getting caught, the server would return 200 OK (and an empty page with a text/html content-type) instead of the proper "404 Not Found" like it should.

[markstory]

Won't this change break applications that dynamically generate favicon.ico, and do things like publish data in it? I agree the current PATH_INFO code is wonky though. I think it was forgotten about when we had to give up on PATH_INFO because of mod_cgi servers.

[jnrbsn]

I guess it probably would "break applications that dynamically generate favicon.ico", but is that really a concern? The intended purpose of that conditional makes me think the answer is "no". Plus, the only way this would affect existing applications is if someone copied the .htaccess file from the Cake code.

Unfortunately, these are the only options I can think of for handling missing favicon.ico files:

1. Let Cake throw a MissingControllerException (the current behavior ...but only because the "PATH_INFO code" is not working as intended).
2. Fix the "PATH_INFO code" so that conditional is true, and the return statement is executed. It's bad that this method would actually serve a 200 OK and a text/html content-type. Those things could be fixed too, but it would still "break applications that dynamically generate favicon.ico" as you mentioned.
3. Let the web server handle it (as I'm suggesting). I think this is the best solution for people who are not doing anything with a favicon.ico. It behaves as expected from a HTTP protocol standpoint, and FWIW, it's one less PHP script running every time someone loads a page. For those who are dynamically generating favicon.ico, you could just put a note in the migration guide for the release (but that would probably mean you'd want this pull request for 2.2 and not 2.1).

[ADmad]

We can simply remove the broken code in index.php and leave htaccess unchanged. Adding conditions for handling requests for such special files is wrong imo. Similar to favicon.ico there are also other files like robots.txt which are requests by bots. All these requests should be handled just like any other normal request. If these files are non present, with debug off cake would return a 404 which is appropriate. We should leave it to the developer to add extra rules in .htaccess or handle it in php as needed.

[jnrbsn]

That makes sense to me (I'll probably still edit my .htaccess though).

I was going to add a second commit reverting the change to .htaccess, but I figured you probably wouldn't want to merge it like that, so I made a new commit in a different branch and submitted pull request #607. So I guess you can close this one if you feel the discussion is complete.

[markstory]

Replaced by #607