Problem/Motivation

No warning is given if Drupal 8 is installed without the Apache rewrite module enabled. This results in a seemingly broken installation for users without this module enabled.

Proposed resolution

Use the .htaccess file to rewrite to a new URL with an appended parameter. Installation process will then check for this parameter. If it is present, rewrite module is installed and working properly. If it is not present, then a warning is shown to the user that the rewrite module is not installed and they are given a link to more information on clean URLs.

Remaining tasks

Reviews needed to accept a final form of the patch

User interface changes

New warning shown during install process if rewrite module is not installed.

API changes

None.

Original report by aspilicious

When apache rewrite is disabled you still can install drupal. When installed several key features for a cms like adding content will just fail ;).
Someone at the office had this issue when he wanted to try drupal8. We accidently found the cause of his broken D8 by going through every setting in his config files.

Is it possible to throw a warning when this isn't enabled. And if we add a check can we make sure it works for both apache en other systems like nginx for example (if that is needed for those systems).

Files: 
CommentFileSizeAuthor
#30 drupal-rewrite_failure-1878884-30.patch1.77 KBchris_hall_hu_cheng
PASSED: [[SimpleTest]]: [MySQL] 58,346 pass(es). View
#23 drupal-rewrite_failure-1878884-23.patch1.78 KBSivaji
PASSED: [[SimpleTest]]: [MySQL] 55,922 pass(es). View
#17 drupal-rewrite_failure-1878884-17.patch1.77 KBjoris_lucius
PASSED: [[SimpleTest]]: [MySQL] 50,576 pass(es). View
#15 drupal-rewrite_failure-1878884-15.patch1.72 KBjoris_lucius
PASSED: [[SimpleTest]]: [MySQL] 50,574 pass(es). View
#12 drupal-rewrite_failure-1878884-11.patch1.82 KBjoris_lucius
PASSED: [[SimpleTest]]: [MySQL] 50,602 pass(es). View
#11 drupal-rewrite_failure-1878884-11.patch1.82 KBjoris_lucius
PASSED: [[SimpleTest]]: [MySQL] 50,596 pass(es). View
#7 drupal-rewrite_failure-1878884-7.patch1.46 KBjoris_lucius
FAILED: [[SimpleTest]]: [MySQL] 3,901 pass(es), 590 fail(s), and 589 exception(s). View

Comments

joris_lucius’s picture

@aspilicious I was checking this issue and agree it is not nice that you get a broken D8 after the installation.
However it is still possible to use this installation, see http://drupal.org/node/1659580

Using www.example.com/index.php/node/add it still works.
Is it an idea to check for the apache module and give a warning with an explanation?
If the warning is ignored use the dirty ~index.php so the default installation does not "crash"?

Any other thoughts like switching back (manual or automatic) once the rewrite is enabled is welcome.

chx’s picture

just add back the clean url checker into system_requirements -- request a pseudo css/js which is in fact a menu callback doing the clean url check -- and thus issue a warning.

joris_lucius’s picture

Assigned: Unassigned » joris_lucius

Assigning to myself, planning to have the first patch ready this week.

joris_lucius’s picture

There seems to problem by using the system_requirements.

Like described on http://api.drupal.org/api/drupal/core!modules!system!system.api.php/func... there are 4 different states, since this is a warning REQUIREMENT_WARNING(The requirement failed with a warning) seems to be the most logical. However the code in install.inc where the actual constant is defined says "Requirement severity -- Warning condition; proceed but flag warning."

If I set the severity to "Warning" it just throws an error during the installation and its not possible to continue, here these 2 texts seems to be in conflict.

When the REQUIREMENT_INFO (For info only) is used for the severity it is ignored if there are no other errors/warnings to be found. This renders this option useless in this situation.

An option is to convert all REQUIREMENT_INFO to drupal_set_message to display the message and not break the installation.

Code used in system_requirements():

  if ($phase == 'install' && function_exists('apache_get_modules')) {
    $apache_modules = apache_get_modules();
    if (!in_array('mod_rewrite', $apache_modules)) {
      $requirements['rewrite_module']['title'] = 'Clean-URLs';
      $requirements['rewrite_module']['value'] = 'Lorem Ipsum';
      $requirements['rewrite_module']['severity'] = REQUIREMENT_INFO;
    }
  }

And at the bottom

  if ($phase == 'install') {
    foreach ($requirements as $requirement){
      if (isset($requirement['severity'])) {
        if ($requirement['severity'] == REQUIREMENT_INFO)
          drupal_set_message($requirement['title'] . ': '. $requirement['value'], 'warning');
      }
    }
  }

Is there any best practice to set warnings during the installation process if the requirements are not able to show warnings without stopping the install.

Damien Tournoud’s picture

I guess this is the same problem as #932636: Drupal *always* installs with clean URLs: because we have a ErrorDocument in .htaccess, even if mod_rewrite is not working, all GET requests are kind of correctly served by Drupal, but *not* any other methods, so all forms submissions will fail.

joris_lucius’s picture

@Damien Tournoud That seems not be true, I am still able to do form submissions with mod_rewrite disabled using index.php/~

Looking into D7 and D6 now to help find a good solution, because D8 is still working without it and the install should not be stopped.

joris_lucius’s picture

Status: Active » Needs review
FileSize
1.46 KB
FAILED: [[SimpleTest]]: [MySQL] 3,901 pass(es), 590 fail(s), and 589 exception(s). View

Included is my first patch for Drupal.
It checks if the rewrite is functioning during the installation, if not it gives an error.

This issue is discussed on IRC, D8 is still functioning even if the rewriting fails https://drupal.org/node/1183208

chx said that fixing the $base_url in the settings.php is also an option to work it.
But settings this url to www.example.com/index.php will break the css/js and does not make www.example.com go to www.example.com/index.php. The user still enters a "broken" website.

I am not an expert on .htaccess and cannot think of any good fix for this.

Any review or support on this issue is welcome.

Status: Needs review » Needs work

The last submitted patch, drupal-rewrite_failure-1878884-7.patch, failed testing.

Nick_vh’s picture

+++ b/core/modules/system/system.installundefined
@@ -55,6 +55,13 @@ function system_requirements($phase) {
+    $requirements['rewrite_module']['value'] = $t('Your system configuration does not currently support this feature. The <a href="http://drupal.org/node/15365">handbook page on Clean URLs</a> has additional troubleshooting information.');

Are you sure this is the best way to expose a link in a $t value? I think you'd need to wrap it in an l function.

$t('text !link.', array('!link' => l($t('description'), 'www.test.com')));

+++ b/core/modules/system/system.installundefined
@@ -55,6 +55,13 @@ function system_requirements($phase) {
+  ¶

minor but spacing issue

Aside of that, I don't know enough of what you are trying to do, but I hope it helps :)

chx’s picture

This misses a check of $install_state['interactive'] -- the testing system uses install_drupal to create the 'child' Drupal being tested and this requirement error aborts that.

joris_lucius’s picture

FileSize
1.82 KB
PASSED: [[SimpleTest]]: [MySQL] 50,596 pass(es). View

New patch with $install_state, $t fix and test with $requirements['description']

joris_lucius’s picture

Status: Needs work » Needs review
FileSize
1.82 KB
PASSED: [[SimpleTest]]: [MySQL] 50,602 pass(es). View

Never mind this comment, setting the status in this one queued the last comment for testing.

Nick_vh’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/system.installundefined
@@ -55,6 +56,14 @@ function system_requirements($phase) {
+  ¶

you still have the spacing issue ;-)

chx’s picture

-- Note: the rewrite rule and basing the check on it is my idea so please don't blame poor @joris_lucius if you find it very hacky.

joris_lucius’s picture

Status: Needs work » Needs review
FileSize
1.72 KB
PASSED: [[SimpleTest]]: [MySQL] 50,574 pass(es). View

Hopefully the final patch.
Fixed the whitespace and made the code shorter, also removed the description placeholder.

joris_lucius’s picture

Issue tags: +Needs manual testing

Test passed, still needs manual testing because it affects the UI installer.

chx, swentel and other IRC support thanks for the help on my first core patch!

joris_lucius’s picture

FileSize
1.77 KB
PASSED: [[SimpleTest]]: [MySQL] 50,576 pass(es). View

Put the 2 globals on the same line, nothing else.

tstoeckler’s picture

Status: Needs review » Needs work

The $t issue is not fixed correctly. Please replace:

$t('Your system configuration does not currently support this feature. The !link has additional troubleshooting information.', array('!link' => l($t('handbook page on Clean URLs'), 'http://drupal.org/node/15365')))

with

$t('Your system configuration does not currently support this feature. The handbook page on Clean URLs has additional troubleshooting information.', array('@url' => url('http://drupal.org/node/15365')))

(Also, though this is minor: Shouldn't it be "clean URLs" instead of "Clean URLs"?)

Damien Tournoud’s picture

Status: Needs work » Closed (duplicate)

I don't understand, is this just a workaround for #1546082: Follow-up to variable_get('clean_url') removal?

Using Drupal on server setups that do not support rewriting is still supported. The wording of the change request that removed the clean_url variable is very clear.

chx’s picture

Status: Closed (duplicate) » Needs review

Nope. Drupal does not work without clean URLs now. If it will we can demote this to a warning. I think a warning is useful.

aspilicious’s picture

drupal forms (node edit forms etc) don't work without clean url's, it maybe works but there is no way you can tell why your nodes don't get saved. You get a 404 on a post or something like that.

Nothing in the report overview told us what the problem was.

chx’s picture

Tagging -- this needs a much better issue summary. And, again, it's possible this is a stopgap issue but even so it's worth doing IMNSHO.

Sivaji’s picture

FileSize
1.78 KB
PASSED: [[SimpleTest]]: [MySQL] 55,922 pass(es). View

Re #1:
> Using www.example.com/index.php/node/add it still works.

Is there any other way to know this. Because I encountered this bug, searched in Google, located this issue and your comment to know it works this way.

I too think this needs fix. I have re-rolled the patch #17, with proper use of link in $t().

> (Also, though this is minor: Shouldn't it be "clean URLs" instead of "Clean URLs"?)

This is already "Clean URLs"

RyanPrice’s picture

Updated issue summary

katy5289’s picture

I tested the patch and the error message comes up saying
Error - Clean URLs
Your system configuration does not currently support this feature. The handbook page on Clean URLs has additional troubleshooting information.

I will attach a screenshot later

needs further code review

katy5289’s picture

FileSize
44.03 KB

Adding screenshot. When mod_rewrite is turned off, this error message displays.
Error - Clean URLs
Your system configuration does not currently support this feature. The handbook page on Clean URLs has additional troubleshooting information.

xjm’s picture

Issue tags: -Needs manual testing

Untagging per #25 and #26.

xjm’s picture

amateescu’s picture

Status: Needs review » Needs work
+++ b/.htaccess
@@ -115,6 +115,10 @@ DirectoryIndex index.php index.html index.htm
+  # Rewrite the install.php to check during installation if
+  # the mod_rewrite is working
+  RewriteRule ^core/install.php core/install.php?rewrite=ok [QSA,L]

+++ b/core/modules/system/system.install
@@ -55,6 +55,13 @@ function system_requirements($phase) {
+  if ($phase == 'install' && $install_state['interactive'] && !isset($_GET['rewrite'])) {
+    $requirements['rewrite_module']['title'] = $t('Clean URLs');
+    $requirements['rewrite_module']['value'] = $t('Your system configuration does not currently support this feature. The <a href="@link">handbook page on Clean URLs</a> has additional troubleshooting information.', array('@link' => 'http://drupal.org/node/15365'));
+    $requirements['rewrite_module']['severity'] = REQUIREMENT_ERROR;

Since the 'rewrite' query string is only added in .htaccess, we also need to restrict the requirements check to Apache.

Otherwise, with the current patch, Drupal won't be installable on any other server than Apache with mod_rewrite enabled.

chris_hall_hu_cheng’s picture

Assigned: joris_lucius » chris_hall_hu_cheng
Status: Needs work » Needs review
FileSize
1.77 KB
PASSED: [[SimpleTest]]: [MySQL] 58,346 pass(es). View

Hi all,
I started testing Drupal 8 installs and ran foul this briefly. The 23 patch didn't work as out of date, get_t() function no longer needed etc. etc. so here is a new one.

I degraded to warning because there is still a danger it will act as a blocker to somebody on a non-standard setup and warnings do let you continue if you click through.

I added a check for 'Apache' contained in the server software, again for non-standard setups not using .htaccess.

I went along with the general conclusions of this thread as I don't know all the related issues, there are probably better ways and other areas that can be fixed but I believe that this makes things better as a lot of default setups don't have rewrites enabled, in 7 and below it worked (then then you kicked yourself and enabled rewrites to get clean urls). In Drupal 8 it is potentially confusion.

I am going to look at editing the documentation linked to in the warning next as that doesn't clarify the position for Drupal 8.

Edit: forgot to mention anything needs changing and I will attempt to do that as promptly as possible.

chris_hall_hu_cheng’s picture

Any chance this will get looked or should I stop worrying about it, was going to look at updating documentation, need for other issues (the requirements processes on install isn't clear when you have just a warning for example). Chances are this will go stale again soon though.

chris_hall_hu_cheng’s picture

Have a better understanding of this now, still think it is important, wrote a little about it here:
http://running-on-drupal8.co.uk/blog/drupal8-install-clean-urls

going to leave it a week or so then raise a new issue with better patch, might get some attention.

chris_hall_hu_cheng’s picture

Status: Needs review » Closed (won't fix)

I assume this is dead and with so many comments and confusion is no longer much use so have closed and attempted to raise a clearer issue #2094985: Drupal 8 install does not warn if Clean Urls are not supported I picked the best status I could apply.

chris_hall_hu_cheng’s picture

Issue summary: View changes

Updating issue summary

geerlingguy’s picture

Issue summary: View changes
Status: Closed (won't fix) » Closed (duplicate)
Related issues: +#2094985: Drupal 8 install does not warn if Clean Urls are not supported

If work is moving over to #2094985: Drupal 8 install does not warn if Clean Urls are not supported, this should be duplicate, not wontfix.

YesCT’s picture