Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I *UPDATED* the issue main text so nobody gets confused.
Issue: When you set the 404 page to user/login and you are a logged in user and calling a page that did not exist, Drupal will create an infinite loop.
Drupals standardbehavior on paths which are not found is to save the path in $_GET['destination'] and if set redirect to the page which is set in variable 'site_404' (see code). If that variable returns user/login, the logincode will then realize, that the user is already logged in and will without displaying the suer/login page redirect to the saved original page request from $_GET['destination']. This of cause, was not found in the first place and the loop starts again.
// Check for and return a fast 404 page if configured.
drupal_fast_404();
// Keep old path for reference, and to allow forms to redirect to it.
if (!isset($_GET['destination'])) {
$_GET['destination'] = $_GET['q'];
}
$path = drupal_get_normal_path(variable_get('site_404', ''));
if ($path && $path != $_GET['q']) {
The change to user/login happend in this rows and yes it is our fault. But I think the thinking that, if somebody requests a page which is not there sees the user/login page sound not so bad. We were developing a drupal site which NEEDs to have logged in users.
So this indeed is reproducable with vanilla drupal. Just put user/login in the 404 textfield, save and request the page /foobarblubb as authenticated user. There you have your infinite loop.
Btw. the 404 and 403 fields have no distinction of logged und anonymous 404 pages, so this error might happen to someone else als well. All caused by the rewrite of $_GET['q'] and $_GET['destination'].
A hint below the fields could be usefull or a check in the core. Not sure.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | infinite_404_redirect-1672282-11.patch | 1.46 KB | superspring |
| #4 | infinite_loop.png | 30.01 KB | ro-no-lo |
Comments
Comment #1
ro-no-lo CreditAttribution: ro-no-lo commentedTo clarify: The part that I get redirected to user/login while already logged in is strange and wrong.
Comment #2
pounardNot an issue for this module.
Comment #3
marcingy CreditAttribution: marcingy commentedThis does not sound like a core bug but a badily configured module somewhere. Moving to support request.
Comment #4
ro-no-lo CreditAttribution: ro-no-lo commentedFOUND THE CAUSE
Because it bothered me once again, I debugged it again:
The change to user/login happend in this rows and yes it is our fault. But I think the thinking that, if somebody requests a page which is not there sees the user/login page sound not so bad. We were developing a drupal site which NEEDs to have logged in users.
So this indeed is reproducable with vanilla drupal. Just put user/login in the 404 textfield, save and request the page /foobarblubb as authenticated user. There you have your infinite loop.
Btw. the 404 and 403 fields have no distinction of logged und anonymous 404 pages, so this error might happen to someone else als well. All caused by the rewrite of $_GET['q'] and $_GET['destination'].
A hint below the fields could be usefull or a check in the core. Not sure.
Comment #5
ro-no-lo CreditAttribution: ro-no-lo commentedI changed it back to bug report, so that maybe someone checks if that is indeed an issue who should be avoided in the future - even by accidential input of user/login, which is an absolut valid path in the system for anonymous users as a 404 page.
Feel free to change the category back.
Comment #6
pounardI agree that if this can be reproduced with a vanilla core just by setting the site_404 to 'user/login' this is bug. As this behavior is now known, we should either forbidden this path to be set in this variable, either fix it in order to avoid infinite redirect loops.
Comment #7
ro-no-lo CreditAttribution: ro-no-lo commentedJust thinking:
I belive that Drupal needs a solution, which scales to more paths not just 'user/login'. There might me modules which defines other pathes for the same functionality as 'user/login'. Another approach might be usefull.
In the assumption that '' will allways work or at least should really always work. An infinite loop counter could be added. Which tries the drupal_goto() in case of a 404 for X-times. and if a certain amount is exceeded, it redirects to '' with a message and/or watchdog entry. I looked into the common.inc if I could hack something together real quick, but common.inc#2510 was not as easy to understand for me in that case.
I hope someone with more core router knowledge can do it.
Something like this, but I think thats not that good:
Comment #8
pounardIt's not easy to track redirect loop from the server, it assumes that we need to store data either in the session or another place that is linked to the user currently browsing (which is exactly what is the session for, we could also just use a cookie thought): it may break the lazy session cookie sending algorithm, this kind of session extra usage would leave the user hit pages with no cache where it should deliver a cached page.
Comment #9
ro-no-lo CreditAttribution: ro-no-lo commentedThe code I added worked for my simple example. Might be a good start. Because the code is only in the MENU_NOT_FOUND section added.
Hmm your right. This needs to be user bound and therefore the variable table is not a good idea.
Comment #10
pounardYes because multiple requests from different clients could conflict, and also because variables system performance is critical, and data in this table has a front cache on top of it, rebuild operations have a really negative performance impact, it should not be used for volatile or mutant data.
Comment #10.0
pounardUpdated Entry
Comment #11
superspring CreditAttribution: superspring commentedThis patch prevents infinite redirects by storing which URLs have already gone through when redirecting with 404s.
Comment #13
superspring CreditAttribution: superspring commented#11: infinite_404_redirect-1672282-11.patch queued for re-testing.
Comment #14
marcingy CreditAttribution: marcingy commentedWe can't store 404s in session my opinion as this leads to a state where anonymous users end up with sessions which was purposely removed in d7.
Comment #15
superspring CreditAttribution: superspring commentedHey @marcingy,
To have no sessions what-so-ever this could be rewritten to use cookies. Is this the type of fix that you're looking for?
Comment #16
marcingy CreditAttribution: marcingy commentedCookies are also not ideal either because of implications it also has for proxies.
The issue is caused by
So I would make it so as the variable' site_404 can't be set to user/login. Instead the path of user can be used which will either send the user to their profile page or present a login form as appropriate. The infinte loop will not happen as the access callback allows anyone to access the page. Ie never allow the inifinte redirect to exist in the first place.
Comment #16.0
marcingy CreditAttribution: marcingy commentedRemoved the distracting texts, so the main issue is clearly pointed out.
Comment #25
quietone CreditAttribution: quietone as a volunteer commentedI set the 404 page to user/login and tested by accessing a non existing node as user 1 and as anonymous. There was no infinite loop in Drupal 9.2.x or Drupal 8.9.13 but there was in Drupal 7.79-dev.
Moving to D7 issue queue.