The menu_router mysql check in fast_404_validate_path_drupal() is causing us issues.

We sometimes have a row in our menu_router table that is empty - caused by another mischievous module. It shouldn't be there, but when it is, it causes every page on the site to be a 404. Due to the WHERE ? LIKE CONCAT(path, '%') check.

Also, I think we should keep the query should be as similar to the one in fast_404_validate_path_mysql().

CommentFileSizeAuthor
#1 fast_404-2210349-fix_query_in_fast_404_validate_path_drupal.patch3.9 KBLeon Kessler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Leon Kessler’s picture

Leon Kessler CreditAttribution: Leon Kessler commented
Status: Active » Needs review
FileSize
fast_404-2210349-fix_query_in_fast_404_validate_path_drupal.patch3.9 KB
Bladedu’s picture

Bladedu
he/him
Primary language Italian
Location Italy
CreditAttribution: Bladedu commented
Status: Needs review » Needs work 
+++ b/fast_404.inc
@@ -129,10 +129,10 @@ function fast_404_validate_path_drupal() {
+  $sql = "SELECT path FROM {menu_router} WHERE path = :q OR :q LIKE CONCAT(path, '/%')";

This query is wrong. It shouldn't contain a trailing slash "/" in front of the wildcard.

adammalone’s picture

adammalone
Primary language English
Location Sydney
CreditAttribution: adammalone commented

As a nitpick, there are also a bunch of empty lines in your patch that could ideally be gotten rid of to make the patch cleaner. If there are whitespace removals etc to clean up the module, they'd be greatly appreciated although would be a better fit in another issue :)

gapple’s picture

gapple
he/they
Primary language English
CreditAttribution: gapple commented

I think ultimately the mischievous module needs to be fixed.

It looks to me that the lockout that ends up occurring is due to the if ($res) condition, which will fail on an empty string. Doing a stricter check on the return value of DatabaseStatementInterface::fetchField should be sufficient, but will then result in the function always returning true while the empty router entry exists.

adammalone’s picture

adammalone
Primary language English
Location Sydney
CreditAttribution: adammalone commented

I think ultimately the mischievous module needs to be fixed.

^ this