Issue #951644 by catch, David_Rothstein, aschiwi, xjm: Fixed Requirement warnings (e.g. for PHP memory limit) are not shown on install or update unless there is a requirement error also.

4 files changed, 150 insertions, 19 deletions

diff --git a/includes/install.core.inc b/includes/install.core.inc
index a74dfdf0f..094c9b871 100644
--- a/includes/install.core.inc
+++ b/includes/install.core.inc
@@ -710,8 +710,10 @@ function install_display_output($output, $install_state) {
  *
  * @return
  *   A themed status report, or an exception if there are requirement errors.
- *   Otherwise, no output is returned, so that the next task can be run
- *   in the same page request.
+ *   If there are only requirement warnings, a themed status report is shown
+ *   initially, but the user is allowed to bypass it by providing 'continue=1'
+ *   in the URL. Otherwise, no output is returned, so that the next task can be
+ *   run in the same page request.
  */
 function install_verify_requirements(&$install_state) {
   // Check the installation requirements for Drupal and this profile.
@@ -723,22 +725,30 @@ function install_verify_requirements(&$install_state) {
   // Check the severity of the requirements reported.
   $severity = drupal_requirements_severity($requirements);
 
-  if ($severity == REQUIREMENT_ERROR) {
+  // If there are errors, always display them. If there are only warnings, skip
+  // them if the user has provided a URL parameter acknowledging the warnings
+  // and indicating a desire to continue anyway. See drupal_requirements_url().
+  if ($severity == REQUIREMENT_ERROR || ($severity == REQUIREMENT_WARNING && empty($install_state['parameters']['continue']))) {
     if ($install_state['interactive']) {
       drupal_set_title(st('Requirements problem'));
       $status_report = theme('status_report', array('requirements' => $requirements));
-      $status_report .= st('Check the error messages and <a href="!url">proceed with the installation</a>.', array('!url' => check_url(request_uri())));
+      $status_report .= st('Check the error messages and <a href="!url">proceed with the installation</a>.', array('!url' => check_url(drupal_requirements_url($severity))));
       return $status_report;
     }
     else {
-      // Throw an exception showing all unmet requirements.
+      // Throw an exception showing any unmet requirements.
       $failures = array();
       foreach ($requirements as $requirement) {
+        // Skip warnings altogether for non-interactive installations; these
+        // proceed in a single request so there is no good opportunity (and no
+        // good method) to warn the user anyway.
         if (isset($requirement['severity']) && $requirement['severity'] == REQUIREMENT_ERROR) {
           $failures[] = $requirement['title'] . ': ' . $requirement['value'] . "\n\n" . $requirement['description'];
         }
       }
-      throw new Exception(implode("\n\n", $failures));
+      if (!empty($failures)) {
+        throw new Exception(implode("\n\n", $failures));
+      }
     }
   }
 }
diff --git a/includes/install.inc b/includes/install.inc
index 516e14618..6411f8f19 100644
--- a/includes/install.inc
+++ b/includes/install.inc
@@ -999,7 +999,6 @@ function drupal_install_fix_file($file, $mask, $message = TRUE) {
   }
 }
 
-
 /**
  * Send the user to a different installer page.
  *
@@ -1017,6 +1016,68 @@ function install_goto($path) {
 }
 
 /**
+ * Returns the URL of the current script, with modified query parameters.
+ *
+ * This function can be called by low-level scripts (such as install.php and
+ * update.php) and returns the URL of the current script. Existing query
+ * parameters are preserved by default, but new ones can optionally be merged
+ * in.
+ *
+ * This function is used when the script must maintain certain query parameters
+ * over multiple page requests in order to work correctly. In such cases (for
+ * example, update.php, which requires the 'continue=1' parameter to remain in
+ * the URL throughout the update process if there are any requirement warnings
+ * that need to be bypassed), using this function to generate the URL for links
+ * to the next steps of the script ensures that the links will work correctly.
+ *
+ * @param $query
+ *   (optional) An array of query parameters to merge in to the existing ones.
+ *
+ * @return
+ *   The URL of the current script, with query parameters modified by the
+ *   passed-in $query. The URL is not sanitized, so it still needs to be run
+ *   through check_url() if it will be used as an HTML attribute value.
+ *
+ * @see drupal_requirements_url()
+ */
+function drupal_current_script_url($query = array()) {
+  $uri = $_SERVER['SCRIPT_NAME'];
+  $query = array_merge(drupal_get_query_parameters(), $query);
+  if (!empty($query)) {
+    $uri .= '?' . drupal_http_build_query($query);
+  }
+  return $uri;
+}
+
+/**
+ * Returns a URL for proceeding to the next page after a requirements problem.
+ *
+ * This function can be called by low-level scripts (such as install.php and
+ * update.php) and returns a URL that can be used to attempt to proceed to the
+ * next step of the script.
+ *
+ * @param $severity
+ *   The severity of the requirements problem, as returned by
+ *   drupal_requirements_severity().
+ *
+ * @return
+ *   A URL for attempting to proceed to the next step of the script. The URL is
+ *   not sanitized, so it still needs to be run through check_url() if it will
+ *   be used as an HTML attribute value.
+ *
+ * @see drupal_current_script_url()
+ */
+function drupal_requirements_url($severity) {
+  $query = array();
+  // If there are no errors, only warnings, append 'continue=1' to the URL so
+  // the user can bypass this screen on the next page load.
+  if ($severity == REQUIREMENT_WARNING) {
+    $query['continue'] = 1;
+  }
+  return drupal_current_script_url($query);
+}
+
+/**
  * Functional equivalent of t(), used when some systems are not available.
  *
  * Used during the install process, when database, theme, and localization
diff --git a/modules/system/system.test b/modules/system/system.test
index a75153f6a..a00f51e26 100644
--- a/modules/system/system.test
+++ b/modules/system/system.test
@@ -2086,7 +2086,7 @@ class UpdateScriptFunctionalTest extends DrupalWebTestCase {
   }
 
   function setUp() {
-    parent::setUp();
+    parent::setUp('update_script_test');
     $this->update_url = $GLOBALS['base_url'] . '/update.php';
     $this->update_user = $this->drupalCreateUser(array('administer software updates'));
   }
@@ -2123,6 +2123,49 @@ class UpdateScriptFunctionalTest extends DrupalWebTestCase {
   }
 
   /**
+   * Tests that requirements warnings and errors are correctly displayed.
+   */
+  function testRequirements() {
+    $this->drupalLogin($this->update_user);
+
+    // If there are no requirements warnings or errors, we expect to be able to
+    // go through the update process uninterrupted.
+    $this->drupalGet($this->update_url, array('external' => TRUE));
+    $this->drupalPost(NULL, array(), t('Continue'));
+    $this->assertText(t('No pending updates.'), t('End of update process was reached.'));
+
+    // If there is a requirements warning, we expect it to be initially
+    // displayed, but clicking the link to proceed should allow us to go
+    // through the rest of the update process uninterrupted. (First run this
+    // test with pending updates to make sure they can be run successfully;
+    // then try again without pending updates to make sure that works too.)
+    variable_set('update_script_test_requirement_type', REQUIREMENT_WARNING);
+    drupal_set_installed_schema_version('update_script_test', drupal_get_installed_schema_version('update_script_test') - 1);
+    $this->drupalGet($this->update_url, array('external' => TRUE));
+    $this->assertText('This is a requirements warning provided by the update_script_test module.');
+    $this->clickLink('try again');
+    $this->assertNoText('This is a requirements warning provided by the update_script_test module.');
+    $this->drupalPost(NULL, array(), t('Continue'));
+    $this->drupalPost(NULL, array(), t('Apply pending updates'));
+    $this->assertText(t('The update_script_test_update_7000() update was executed successfully.'), t('End of update process was reached.'));
+    $this->drupalGet($this->update_url, array('external' => TRUE));
+    $this->assertText('This is a requirements warning provided by the update_script_test module.');
+    $this->clickLink('try again');
+    $this->assertNoText('This is a requirements warning provided by the update_script_test module.');
+    $this->drupalPost(NULL, array(), t('Continue'));
+    $this->assertText(t('No pending updates.'), t('End of update process was reached.'));
+
+    // If there is a requirements error, it should be displayed even after
+    // clicking the link to proceed (since the problem that triggered the error
+    // has not been fixed).
+    variable_set('update_script_test_requirement_type', REQUIREMENT_ERROR);
+    $this->drupalGet($this->update_url, array('external' => TRUE));
+    $this->assertText('This is a requirements error provided by the update_script_test module.');
+    $this->clickLink('try again');
+    $this->assertText('This is a requirements error provided by the update_script_test module.');
+  }
+
+  /**
    * Tests the effect of using the update script on the theme system.
    */
   function testThemeSystem() {
diff --git a/update.php b/update.php
index 23ffa577f..5e771a824 100644
--- a/update.php
+++ b/update.php
@@ -245,7 +245,8 @@ function update_info_page() {
   $output .= "<li>Install your new files in the appropriate location, as described in the handbook.</li>\n";
   $output .= "</ol>\n";
   $output .= "<p>When you have performed the steps above, you may proceed.</p>\n";
-  $output .= '<form method="post" action="update.php?op=selection&amp;token=' . $token . '"><p><input type="submit" value="Continue" class="form-submit" /></p></form>';
+  $form_action = check_url(drupal_current_script_url(array('op' => 'selection', 'token' => $token)));
+  $output .= '<form method="post" action="' . $form_action . '"><p><input type="submit" value="Continue" class="form-submit" /></p></form>';
   $output .= "\n";
   return $output;
 }
@@ -316,20 +317,26 @@ function update_extra_requirements($requirements = NULL) {
 }
 
 /**
- * Check update requirements and report any errors.
+ * Check update requirements and report any errors or (optionally) warnings.
+ *
+ * @param $skip_warnings
+ *   (optional) If set to TRUE, requirement warnings will be ignored, and a
+ *   report will only be issued if there are requirement errors. Defaults to
+ *   FALSE.
  */
-function update_check_requirements() {
+function update_check_requirements($skip_warnings = FALSE) {
   // Check requirements of all loaded modules.
   $requirements = module_invoke_all('requirements', 'update');
   $requirements += update_extra_requirements();
   $severity = drupal_requirements_severity($requirements);
 
-  // If there are issues, report them.
-  if ($severity == REQUIREMENT_ERROR) {
+  // If there are errors, always display them. If there are only warnings, skip
+  // them if the caller has indicated they should be skipped.
+  if ($severity == REQUIREMENT_ERROR || ($severity == REQUIREMENT_WARNING && !$skip_warnings)) {
     update_task_list('requirements');
     drupal_set_title('Requirements problem');
     $status_report = theme('status_report', array('requirements' => $requirements));
-    $status_report .= 'Check the error messages and <a href="' . check_url(request_uri()) . '">try again</a>.';
+    $status_report .= 'Check the error messages and <a href="' . check_url(drupal_requirements_url($severity)) . '">try again</a>.';
     print theme('update_page', array('content' => $status_report));
     exit();
   }
@@ -385,8 +392,9 @@ if (empty($op) && update_access_allowed()) {
   // Set up theme system for the maintenance page.
   drupal_maintenance_theme();
 
-  // Check the update requirements for Drupal.
-  update_check_requirements();
+  // Check the update requirements for Drupal. Only report on errors at this
+  // stage, since the real requirements check happens further down.
+  update_check_requirements(TRUE);
 
   // Redirect to the update information page if all requirements were met.
   install_goto('update.php?op=info');
@@ -418,8 +426,12 @@ if (update_access_allowed()) {
 
   update_fix_compatibility();
 
-  // Check the update requirements for all modules.
-  update_check_requirements();
+  // Check the update requirements for all modules. If there are warnings, but
+  // no errors, skip reporting them if the user has provided a URL parameter
+  // acknowledging the warnings and indicating a desire to continue anyway. See
+  // drupal_requirements_url().
+  $skip_warnings = !empty($_GET['continue']);
+  update_check_requirements($skip_warnings);
 
   $op = isset($_REQUEST['op']) ? $_REQUEST['op'] : '';
   switch ($op) {
@@ -433,7 +445,12 @@ if (update_access_allowed()) {
 
     case 'Apply pending updates':
       if (isset($_GET['token']) && $_GET['token'] == drupal_get_token('update')) {
-        update_batch($_POST['start'], $base_url . '/update.php?op=results', $base_url . '/update.php');
+        // Generate absolute URLs for the batch processing (using $base_root),
+        // since the batch API will pass them to url() which does not handle
+        // update.php correctly by default.
+        $batch_url = $base_root . drupal_current_script_url();
+        $redirect_url = $base_root . drupal_current_script_url(array('op' => 'results'));
+        update_batch($_POST['start'], $redirect_url, $batch_url);
         break;
       }