Fix 404 handling without clean URLs

Doesn't apply anymore.

Steven what's up with this?

I can't reproduce this on HEAD(5.0) or 4.7.

After testing with a disabled mod_rewrite I'm fairly certain this has been fixed already.

I think I will have to re open this. As it is kind of broken with D7 now.

I did an install of drupal 7 alpha 7 on a server with no mod rewite. It looked like everything was fine, and the install wen't ok. Pages were showing ok.

BUT.

Forms would not submit, and every page was returning a 404 code, but with the correct content for the page. I only realised abuot the 404 code as I had firebug on.

This is kind of an inciduous and evil bug, as I didn't realise that mod rewrite was disabled and clean urls looked like they were working, it was only after hacking around with the rewite rules to find out what was going on that I realised that it was the 404 handler which was causing the issue. But that was the best part of an afternoon spent trying to work out what was up.

I am bumping to major as it can break Drupal.

It would be nice to see if anyone else can recreate.

Looking into this a little.

I think this is more prevalent as request_path() will parse a clean URL, even if clean urls are off. So if index.php gets called by the 404 handler instead of mod_rewite it can look like the site is working.

I can see a couple of ways to stop this, none ideal
1. request_path could honor clean urls, but this would make testing if clean urls work problematic
2. We could make the 404 handler in .htaccess conditional on mod rewrite being enabled, but as mod rewrite handles non existent files would this do anything?.
3. We could check in Drupal (somehow) to see if we are being called from the 404 handler or mod rewrite.
4. We could have the 404 handler go to a file other than index.php so we can flag that this is a file not found.

I have looked into this some more and created a patch. It occurs when a server becomes misconfigured and clean urls no longer work. To reproduce is easy.

On your server turn off mod rewrite, then try to log in and disable clean urls.

You won't be able to, as the 404 handler doesn’t pass on form variables, as well as forcing a 404 return code for each request.

This essentially makes a Drupal site unusable, and it is not obvious what has happened, or even that things are broken until you try to submit a form.

The patch will check to see if what appears to Drupal to be a valid URL is also considered so by the web browser, if not, it will alert the user and turn clean URLs off.

Updating patch to get rid of tabs

Patch seems to fix as expected. Couple things though.

1. the drupal_set_message probably isn't necessary.
2. meens should be means
3. The behavior still exists if clean_urls are turned off and you hit a clean url. You should probably separate the variable set from the redirect. This might also affect the watchdog.
4. There should be spaces after commas for all your function arguments. watchdog and drupal_set_message are missing them.

@neclimdul Thanks for the review. Points 2,3 & 4 have been addressed.

I'm not sure about point 1, it may be useful to alert someone to the fact that something has just broken as it isn't immediately apparent unless you are keeping an eye on the URLs.

Well, that may be but your average site visitor doesn't care or understand what that means. That's what the watchdog does. I don't know if we have this sort of failure logic documented anywhere but it is consistent with handling other sorts of failures in core.

#17: drupal-13594-3.patch queued for re-testing.

The patch in #17 works as expected.

Another way to reproduce this behaviour and test the patch is by setting up a fresh D7 install with clean urls enabled. Then turn the RewriteEngine off in your .htaccess

# Various rewrite rules.
<IfModule mod_rewrite.c>
  RewriteEngine off

The site returns "307 Temporary Redirect", clean urls are deactivated.

Is #718848: 404 when enabling overlay module when Short URLs are not supported. a duplicate of this?

I'm not sure since this isn't directly associated with the Overlay module. That said, the problem with Overlay may be symptom and that would make it a duplicate.

Why don't you try the patch and see if it fixes the problem? I don't think there was anything keeping this from being RTBC other than I wasn't comfortable forcing that message to sit visitors.

Can you image if you saw something like that on Facebook? Some user would be like "Short URLs? WTF and why do I care?" Then there would be a post on thedailywtf and we'd all laugh at them.

I removed the set_message and some whitespaces from JeremyFrench's patch #17.

I have a brand-new install of D7 and I ran into this issue. I checked this site for issues related to a 404 message. I found #886182: Editing Path in Page Redirects to admin/content 404 and that led me here.

I applied the patch from #23 and the issue has indeed been fixed. I'm hoping this makes its way back into core.

For this issue there is a common question "Are clean URLs enabled?" Personally, I have no idea. When I go to Administration » Configuration » Search and metadata » Clean URLs, and run the test, the page refreshes, but there is no success/fail message. Looking at the site with a non-registered user I don't see clean URLs, so I'm assuming it's turned off. So where do we turn it on and how can we really verify that it's on or off?

Thanks!

[EDIT] It looks like the problem persists if the admin overlay is not in use, but when overlay is in use, the 404 does not occur. I'm not 100% sure of the cause/effect situation here but will post if I can narrow it down.

Patch looks good. Wondering whether we could test this in any way (hacks).

I couldn't think of a way to automate the testing.

If you want to test manually it is fairly straightforward (from memory). Install a site on a server with mod rewrite working. Then disable mod rewrite in your Apache config (and restart).

Without the patch the site will look like it is working, but you will be unable to login (or do any post form submission).

With the patch clean urls should be turned off and login should then work.

So the test would be to change the same things that the "clean urls" test changes (thus simulating that it once passed), and then try to log in. If the login succeeds, the test passes.

Reading over this patch, I can immediately see a DrupalWTF coming out of it. If I made an accidental change to .htaccess or Apache that busted clean URLs, I'd probably notice quickly. In this hypothetical situation, I'd appreciate Drupal changing to non-clean URLs temporarily, but if I then corrected the issue with .htaccess/Apache my expectation would be that clean URLs would start working again immediately. This situation could go terribly wrong when dealing with multi-site situations where you'd have to go through dozens of sites and manually re-enable clean URLs on every one of them.

Additionally, most users have no idea what Clean URLs are at all. They don't know what they're called or why they wouldn't come back on immediately after correcting the problem with their server. I'd suggest that we make this change temporary (memory-only) by setting $GLOBALS['conf']['clean_urls'] = 0; before the call to drupal_goto() rather than saving the change to the database. "Automatically" changing configuration and leaving the user with no idea how to turn it back on is a hundred support requests just waiting to happen.

Additionally the watchdog message is incredibly vague (couldn't we at least add a link to the Clean URLs configuration page?) and should describe how to remedy the problem rather than making a vague statement. I'd suggest something like this:

The Clean URLs option was enabled but the server does not seem to support rewriting URLs. Check your server configuration to ensure rewriting URLs is supported or permanently disable Clean URLs under the <a href="!url">Clean URLs setting page</a>.

@quicksketch thanks for digging this up and reviewing!

That is a very nice watchdog message! I'm for changing it.

I'm not sure about the not setting the variable though. If we don't there will be a swarm of watchdog messages until the site administrator figures out there's a problem.

I'm thinking that we would need a separate variable. To say that clean urls had been fixed, with a periodic check to enable again if they are working.

I'm also not sure if form submissions would be passed to pages if they are 302 redirected (I haven't tested though).