install_drupal(): call to install_display_output() results in SQL syntax error [#898634]

In install_drupal(), when an exception occurs in the try block, it is caught and displayed to the user using install_display_output().

install_display_output() winds up calling menu_get_item() where the path $ancestor array is found empty but nevertheless used in a query, which leads to an invalid SQL query (WHERE path in ()).

This behavior is triggered when the installation profile contains errors.

An abbreviated debug back trace looking back from menu_get_item() looks something like this:

install_display_output() uses
template_preprocess_maintenance_page() to render the error page. Then a call to
drupal_get_title() retrieves the active title with
menu_get_active_title() which in turn calls
menu_get_item().

Full debug back trace is attached.

Screenshot of the error message:
http://skitch.com/alexbarth/dwxr2/re-installation-error

Full error message:

An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows. Path: http://localhost/rw/install.php?profile=myprofile&locale=en&id=1&op=do StatusText: Service unavailable (with message) ResponseText: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ') ORDER BY fit DESC LIMIT 0, 1' at line 1: SELECT * FROM {menu_router} WHERE path IN () ORDER BY fit DESC LIMIT 0, 1; Array ( ) in menu_get_item() (line 426 of /opt/local/apache2/htdocs/rw/includes/menu.inc).

Patch coming.

Comment	File	Size	Author
#23	fail_fail_fail.patch	534 bytes	catch
#23
#23	pass_pass_pass.patch	1.79 KB	catch
#23
#11	move_menu_rebuild.patch	1.26 KB	chx
#11
#7	install.patch	849 bytes	catch
#7
#5	898634-install_display_output.patch	959 bytes	catch
#5
#1	898634-1_install_drupal.patch	671 bytes	alex_b
#1
	install_display_output_debug.txt	11.32 KB	alex_b

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

alex_b CreditAttribution: alex_b commented 31 August 2010 at 17:04

Assigned:	alex_b	» Unassigned
Status:	Active	» Needs review

File	Size
898634-1_install_drupal.patch	671 bytes

Here is a first shot at the patch. It exits menu_get_item() in case the $ancestor is empty. Seems reasonable as there can't be a menu item if there are no ancestors in the path. I am definitely not sure about this approach though and would appreciate guidance.

With this patch applied, I get a useful error message: http://skitch.com/alexbarth/dwq5p/installing-drupal-drupal

BTW, to reproduce this error, just add book module right after 'block' in the minimal installation profile:

name = Minimal
description = Start with only a few modules enabled.
version = VERSION
core = 7.x
dependencies[] = block
dependencies[] = book
dependencies[] = dblog
files[] = minimal.profile

Comment #2

Damien Tournoud CreditAttribution: Damien Tournoud commented 31 August 2010 at 17:07

Would it be possible to prevent the maintenance page from getting there in the first place?

Comment #3

alex_b CreditAttribution: alex_b commented 31 August 2010 at 19:44

Would it be possible to prevent the maintenance page from getting there in the first place?

Good question. I don't know.

Comment #4

catch

he/him

English

CreditAttribution: catch commented 9 September 2010 at 02:48

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

We could also stop template_preprocess_maintenance_page() calling drupal_get_title() during the install process (or do a drupal_set_title() which would stop it hitting th Or having the installer do a drupal_set_title() would also prevent it trying to find a menu item. e menu system)

Also I don't think this is critical, it makes it (even more) annoying to debug an install profile with errors in it, but that's not in itself a critical bug.

Comment #5

catch

he/him

English

CreditAttribution: catch commented 9 September 2010 at 02:55

Status:

Needs work

» Needs review

File	Size
898634-install_display_output.patch	959 bytes

Here's a patch doing the drupal_set_title(), untested.

Comment #6

David_Rothstein CreditAttribution: David_Rothstein commented 9 September 2010 at 04:19

Status:

Needs review

» Needs work

I haven't tested it either, but I know it won't work because there's no 'interactive_task' in the installer - it's just 'interactive' :)

Also, isn't this code in the wrong place? Looks like this would set the title on all installer pages, not just error pages. I'm guessing this would need to go inside install_drupal() itself, when the exception/error is caught. And even then, since this error seems to occur during an AJAX request as part of the batch, I'm not sure the title would ever be displayed (although I guess we don't care that much).

I think the approach of displaying a custom title makes sense, but it also seems reasonable to try to fix the underlying menu system bug. menu_get_item() really shouldn't die unexpectedly like that, should it?

Comment #7

catch

he/him

English

CreditAttribution: catch commented 9 September 2010 at 07:04

File	Size
install.patch	849 bytes

doh, that's where I meant to put it, don't know what I was thinking when I actually wrote the patch.

I'm trying to think if I've seen this error anywhere other than installation, possibly when there was an error during a menu_rebuild() you could run into the same thing.

Comment #8

alex_b CreditAttribution: alex_b commented 9 September 2010 at 13:46

I think the approach of displaying a custom title makes sense, but it also seems reasonable to try to fix the underlying menu system bug. menu_get_item() really shouldn't die unexpectedly like that, should it?

That was also my thinking. Is there an inherent problem with #1?

Comment #9

David_Rothstein CreditAttribution: David_Rothstein commented 23 September 2010 at 04:36

Came across a related issue with a similar patch as #1: #910400: SQL error in menu_get_items when menu items has no ancestors and router item is not in cache.

It sounds to me like something along those lines is the correct fix.

Comment #10

alex_b CreditAttribution: alex_b commented 9 October 2010 at 23:00

Component:	install system	» menu system
Status:	Needs work	» Needs review

Note: the error can't be reproduced as described in #1 anymore. But pretty much any fatal error when installing modules or in hook_install() seem to provoke it.

For instance, add throw new Exception('This is broken'); to the end of minimal_install() and install. The error message won't say that the Test Exception, but report the PDOException.

#7 fixes the problem. (It has btw the exact same effect on the resulting error message like #1: http://skitch.com/alexbarth/d4sks/installing-drupal-drupal).

From my point of view this is RTBC. In light of #9 this should probably be run by menu system maintainers, hence setting this to NR and menu system.

Comment #11

chx CreditAttribution: chx commented 10 October 2010 at 06:49

File	Size
move_menu_rebuild.patch	1.26 KB

How about this?

Comment #12

chx CreditAttribution: chx commented 10 October 2010 at 06:55

On a second thought #7 is good too but is it enough? I somewhat think that both patches will be needed.

Comment #13

alex_b CreditAttribution: alex_b commented 10 October 2010 at 16:08

Status:

Needs review

» Reviewed & tested by the community

#11 fixes the reported problem. I like the approach as it solidifies menu_get_item().

IMO, "Errors during installation." message is not strictly needed from a UI point of view, hence I would avoid the extra code - see: http://skitch.com/alexbarth/d4a38/installing-drupal-drupal

The messages would actually need some better formatting, but that's a different story.

I'd say #11 is RTBC.

Comment #14

chx CreditAttribution: chx commented 10 October 2010 at 17:04

I would say let's wait for pwolanin before committing.

Comment #15

pwolanin CreditAttribution: pwolanin commented 10 October 2010 at 18:32

There will be some minor overhead to this change (a few extra calls per page to variable_get()), but it seems like a reasonable change if it makes the overall code more robust.

Comment #16

kattekrab CreditAttribution: kattekrab commented 24 October 2010 at 00:13

Error
The website encountered an unexpected error. Please try again later.

Error message
PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ') ORDER BY fit DESC LIMIT 0, 1' at line 1: SELECT * FROM {menu_router} WHERE path IN () ORDER BY fit DESC LIMIT 0, 1; Array ( ) in menu_get_item() (line 426 of /home/cats/Sites/drupal/includes/menu.inc).

Trying to install D7 B2 - got this error.

Using the 'standard' install profile - entered all the database credentials then clicked next and that's what happened. @cafuego suggests that this might be helpful: http://pastebin.com/qNGV0W4S - that's a list of tables that had been created at that point. The actual error seems to have been caused by the lack of ancestors on the menu item.

Somewhat strangely though after clicking back and doing it again, I progressed to the next screen. However after entering site details and maintainer account details I get the WSOD. When I try to view the site front page, I get page not found.

This is on Ubuntu 10.04 with PHP 5.3.2 and MariaDB.

The same problem does NOT occur when installing on Ubuntu 8.04 with PHP 5.2.10

Comment #17

kattekrab CreditAttribution: kattekrab commented 24 October 2010 at 21:50

Version:	7.x-dev	» 7.0-beta2
Status:	Reviewed & tested by the community	» Needs work

update against D7 Beta-2

Comment #18

David_Rothstein CreditAttribution: David_Rothstein commented 25 October 2010 at 12:47

Version:	7.0-beta2	» 7.x-dev
Status:	Needs work	» Reviewed & tested by the community

The patch still applies - setting this back to its original status.

Comment #19

kattekrab CreditAttribution: kattekrab commented 25 October 2010 at 21:14

That patch didn't fix my issue - so am re-opening http://drupal.org/node/910400

See - http://drupal.org/node/910400#comment-3622670

Comment #20

David_Rothstein CreditAttribution: David_Rothstein commented 26 October 2010 at 04:43

Status:

Reviewed & tested by the community

» Needs review

Sorry, somehow I only noticed your comment in #17, not #16.

I can't reproduce that error myself, but it does seem like the patch fails to protect against a path that has no ancestors. This could happen if menu_rebuild() gets called early enough and does not result in the 'menu_masks' variable actually getting populated...

Something like a combination of the patches in #1 and #11 might be reasonable. There can't be any good reason for this function to ever attempt a query that uses IN() under conditions when we know it will fail.

Comment #21

boombatower CreditAttribution: boombatower commented 4 January 2011 at 08:19

#11 fixes the issue when an install profile calls taxonomy_term_save() when creating forums.

Comment #22

sun

German

Karlsruhe

CreditAttribution: sun commented 7 June 2011 at 00:58

Version:	7.x-dev	» 8.x-dev
Issue tags:		+Needs backport to D7

Comment #23

catch

he/him

English

CreditAttribution: catch commented 5 July 2011 at 13:40

File	Size
pass_pass_pass.patch	1.79 KB

fail_fail_fail.patch	534 bytes

I'm not sure why this was marked down from RTBC. There is no indication from kattekrab's post that they ran through the whole process again with the patch applied, I don't think it can be expected to fix an update that's already hosed, so this extra case is 'needs more info' until we have more information IMO.

The installer issue is fixed by #11, and the patch makes sense in itself.

I'm uploading a test that should fail without the patch applied, then #11 plus the test.