Missing favicon poison and 4.5 compatibility fixes for the upgrade process [#41118]

The administrator is supposed to look into the watchdog logs after the update to find the reasons for any failed query. But the problem is that the missing favicon actually creates a damn lot of watchdog entries, from which it is very hard to find the ones really relevant. Look for a sample 4.5 to 4.7 update (non AJAX version) I included below. There is one entry which is relevant for me (the one from the update script). The others from the missing favicon are irrelevant (and I included one cron run to show that all these are generated by only one update process).

mysql> select SUBSTRING(message, 1, 40) as msg, location, timestamp from watchdog order by timestamp desc limit 98;
+------------------------------------------+-------------------------------+------------+
| msg                                      | location                      | timestamp  |
+------------------------------------------+-------------------------------+------------+
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551173 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551173 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551173 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551172 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551172 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551172 |
| Duplicate entry &#039;rss.xml&#039; for  | /update.php?op=do_update_nojs | 1134551172 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551170 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551170 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551170 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551170 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551169 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551169 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551169 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551169 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551169 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551167 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551167 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551167 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551165 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551165 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551165 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551161 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551161 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551161 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Table &#039;drupalhu.node_revisions&#039 | /favicon.ico                  | 1134551150 |
| Unknown column &#039;theme&#039; in &#03 | /favicon.ico                  | 1134551150 |
| Unknown column &#039;sid&#039; in &#039; | /favicon.ico                  | 1134551150 |
| A cron futása befejeződött.           | /cron.php                     | 1134419522 |
+------------------------------------------+-------------------------------+------------+
98 rows in set (0.03 sec)

Even if we go past the update, the missing favicon poisons the 404 logs with lots of entries. All browsers try to grab the favicon nowadays, even if it is specified nowhere in the HTML code. So Drupal should come with a default favicon file. Which for simplicity should be 0 bytes in size (included as an attachment) :)

Also note that the update process is going to be faster, since there is no need to process these failed 404 page generations on the update pages. Drupal in general will have a lower impact on servers, if the missing favicon is not handled by Drupal's 404 mechanism, since this is too high a price to pay for such a common HTTP request.

Comment	File	Size	Author
#16	Drupal.update.system.fixes.patch	6.24 KB	gábor hojtsy
#14	Drupal.fix.update.process.patch	6.12 KB	gábor hojtsy
#5	Drupal.favicon.add.upgrade.patch	649 bytes	gábor hojtsy
#3	Drupal.favicon.upgrade.patch	641 bytes	gábor hojtsy
	favicon.ico	0 bytes	gábor hojtsy

Comments

Comment #1

killes@www.drop.org commented 14 December 2005 at 09:37

Status:

Reviewed & tested by the community

» Needs review

I disagree. People should simply create themselves a favicon.

Comment #2

gábor hojtsy

he/him

Hungarian

Hungary

commented 14 December 2005 at 09:43

Killes, people will create a favicon if they need one. This correction does not give them any favicon really, just makes all the annoying problems associated with a missing favicon go away. This is not like putting the druplicon back in.

Comment #3

gábor hojtsy

he/him

Hungarian

Hungary

commented 14 December 2005 at 09:48

Status:

Needs review

» Reviewed & tested by the community

Status	File	Size
new	Drupal.favicon.upgrade.patch	641 bytes

Ok, here is a half joking alternative, which is in line with what Killes says. Either this or the zero byte favicon.

Comment #4

killes@www.drop.org commented 14 December 2005 at 09:55

Status:

Reviewed & tested by the community

» Needs work

I'd support the second solution if the text were less silly. Grow up.

Comment #5

gábor hojtsy

he/him

Hungarian

Hungary

commented 14 December 2005 at 10:02

Status:

Needs work

» Reviewed & tested by the community

Status	File	Size
new	Drupal.favicon.add.upgrade.patch	649 bytes

Killes supports instructing users to add a favicon before the upgrade, subtler text included. Note that without this patch the text explicitly instructs users to remove all files, and only unpack what is in the new tarball. If the text is not changed, or a favicon is not included, users are instructed to use a setup which is guaranteed to generate errors in the watchdog tables. IMHO since users are not going to read the UPGRADE.txt (although they obviously should), including a dummy favicon is a better option, but it is now hopefully up to Dries to decide.

Comment #6

dries commented 14 December 2005 at 13:20

Do you have some weird browser plugin/extension/toolbar that tries to grab the favicon.ico for each XMLHttpRequests? I have yet to see such behavior using a stock Firefox.

Does the auto-complete functionality (eg. the AJAX-powered username textfield) trigger favicon.ico requests too?

Maybe it would be better to move the current favicon from the misc-directory to the root-directory?

Comment #7

morbus iff

they/them

commented 14 December 2005 at 13:33

Dries - he said he used the "non-AJAX version" to do the update, which would mean no XMLHTTPRequest. I have no idea what "non-AJAX version" actually means, however.

Comment #8

morbus iff

they/them

commented 14 December 2005 at 13:39

I'm also slightly confused on our use of favicons in general:

a) We ship with "use default shortcut icon" enabled.
b) I disable that checkbox.
c) In Bluemarine, I now get <link rel="shortcut icon" href="" type="image/x-icon" />.
d) Which seems to short-circuit any attempts by Firefox/OS X to fetch /favicon.ico.

I would suspect that this does NOT happen in update.php, since it's themeless, but I'm still a bit concerned about the "non-AJAX version" of 4.7 that Goba is talking about.

Comment #9

dries commented 14 December 2005 at 13:44

Sorry, I missed the part about using the non-AJAX version.

To fix the non-AJAX version we can emit a link-tag in the head-tag?

Not tested:

Index: update.php
===================================================================
RCS file: /cvs/drupal/drupal/update.php,v
retrieving revision 1.166
diff -r1.166 update.php
416a417
>   drupal_set_html_head('<link rel="shortcut icon" href="'. check_url(theme_get_setting('favicon')) .'" type="image/x-icon" />');

Comment #10

morbus iff

they/them

commented 14 December 2005 at 13:54

I think that's probably a "cleaner" approach than shipping a 0 byte favicon (I could lean either way on that issue, for reasons that'd take me a few paragraphs to iterate). But, there's no theme in update.php, so it should really just be <link rel="shortcut icon" href="" type="image/x-icon" />, no?

Comment #11

dries commented 14 December 2005 at 13:59

Status:

Reviewed & tested by the community

» Needs work

Comment #12

drumm

he/him

NY, US

commented 14 December 2005 at 23:49

The favicon goes to the generic logs. This is rather annoying, but not necessarily an update issue.

The maintenance theme, which is what update uses, should use minimal favicon settings since retrieving the custom favicon might result in a worse error in the future if we ever change how it is stored.

The link from update to the logs could be a deep link to filtered logs. I expect only showing PHP errors would be ideal.

Comment #13

dries commented 15 December 2005 at 12:04

So I guess the solution is:

Make the maintenance theme use the favicon.ico in misc (without using the theme settings).
Improve the link to the watchdog.
Remove t() functions from update.php

Comment #14

gábor hojtsy

he/him

Hungarian

Hungary

commented 18 December 2005 at 12:06

Title:

Missing favicon poisons update logs

» Missing favicon poison and 4.5 compatibility fixes for the upgrade process

Status	File	Size
new	Drupal.fix.update.process.patch	6.12 KB

Ok, here is a patch, which includes (almost all of) the fixes requested by Dries, some of which came up in the 4.5 compatibility issue (which I am marking as duplicate in favor of this one).

The patch:
- Links to the default favicon in the misc folder
- Removes all t()-s from the update file
- Includes the 4.5 compatibility fix

It does not however include an improved link to the watchdog logs, because I looked into how watchdog works, and it turned out that it uses the Form API, which seems to only allow for _POST['edit'] to contain the information needed for the setting of the filter (look into _form_builder), so we cannot put it into a link. I was unable to find out, how one does multimethod (GET/POST) form handlers. Anyone more familiar with the situation can improve this patch, but let me stress, that this patch fixes critical issues. The update does not even start now if you have a translation because of the t() issues.

Comment #15

gábor hojtsy

he/him

Hungarian

Hungary

commented 26 December 2005 at 16:32

Priority:	Normal	» Critical
Status:	Needs work	» Needs review

Under my continued testing of the 4.7.0 upgrade (from our 4.5 Drupal.hu database), I also found that a recent performance fix of changing is_array() calls to isset() calls made the revisions updates broken. The browser just hangs on the update regardless whether you use the AJAX or refresh based versions, since the revision updates are never processed. The critical change is easily noticed at the bottom of this diff

   while ($node = db_fetch_object($result)) {
     $revisions = unserialize($node->revisions);
-    if (is_array($revisions) && count($revisions) > 0) {
+    if (!$revisions && is_array($revisions) && count($revisions) > 0) {

The added line checks for a condition, which is impossible to meet: that some variable is not evaluated as true, although it is an array and has items in it. Since this update never finishes until the revisions are processed and the revisions are not processed due to this faulty update, the update process hangs, and never ever completes.

So the updated patch now fixes the following problems:

The update now starts even if you have a localization installed
The update does not emit extra error messages due to the missing favicon
The update does include the 4.5 watchdog update, so error messages generated throughout the process are actually saved.
The update process does not hang on forever, trying to update revisions.

I tested this patch on the Drupal.hu (4.5) database, and it works excellently, fixing multiple critical issues. Please test, please apply.

Comment #16

gábor hojtsy

he/him

Hungarian

Hungary

commented 26 December 2005 at 16:34

Status	File	Size
new	Drupal.update.system.fixes.patch	6.24 KB

Oh, the patch.

Comment #17

dries commented 26 December 2005 at 17:20

Status:

Needs review

» Fixed

Managed to reproduce the problems. Committed to HEAD. Thanks, Goba!

Comment #18

(not verified) commented 9 January 2006 at 17:20

Status:

Fixed

» Closed (fixed)

Missing favicon poison and 4.5 compatibility fixes for the upgrade process

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Comment #16

Comment #17

Comment #18

News items

Our community

Documentation

Drupal code base

Governance of community