Add a configuration warning when memory_limit will limit file uploads

Summary: Fixes T6011. See that task for discussion. We can detect when `memory_limit` will be the limiting factor for drag-and-drop uploads and warn administrators about it.

Test Plan: Fiddled configuration values and hit, then resolved, the issue.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6011

Differential Revision: https://secure.phabricator.com/D10413

src/applications/config/check/PhabricatorSetupCheckStorage.php

@@ -2,6 +2,9 @@
 
 final class PhabricatorSetupCheckStorage extends PhabricatorSetupCheck {
 
+  /**
+   * @phutil-external-symbol class PhabricatorStartup
+   */
   protected function executeChecks() {
     $upload_limit = PhabricatorEnv::getEnvConfig('storage.upload-size-limit');
     if (!$upload_limit) {
@@ -16,8 +19,64 @@ final class PhabricatorSetupCheckStorage extends PhabricatorSetupCheck {
         ->setName(pht('Upload Limit Not Yet Configured'))
         ->setMessage($message)
         ->addPhabricatorConfig('storage.upload-size-limit');
+    } else {
+      $memory_limit = PhabricatorStartup::getOldMemoryLimit();
+      if ($memory_limit && ((int)$memory_limit > 0)) {
+        $memory_limit_bytes = phutil_parse_bytes($memory_limit);
+        $memory_usage_bytes = memory_get_usage();
+        $upload_limit_bytes = phutil_parse_bytes($upload_limit);
+
+        $available_bytes = ($memory_limit_bytes - $memory_usage_bytes);
+
+        if ($upload_limit_bytes > $available_bytes) {
+          $summary = pht(
+            'Your PHP memory limit is configured in a way that may prevent '.
+            'you from uploading large files.');
+
+          $message = pht(
+            'When you upload a file via drag-and-drop or the API, the entire '.
+            'file is buffered into memory before being written to permanent '.
+            'storage. Phabricator needs memory available to store these '.
+            'files while they are uploaded, but PHP is currently configured '.
+            'to limit the available memory.'.
+            "\n\n".
+            'Your Phabricator %s is currently set to a larger value (%s) than '.
+            'the amount of available memory (%s) that a PHP process has '.
+            'available to use, so uploads via drag-and-drop and the API will '.
+            'hit the memory limit before they hit other limits.'.
+            "\n\n".
+            '(Note that the application itself must also fit in available '.
+            'memory, so not all of the memory under the memory limit is '.
+            'available for buffering file uploads.)'.
+            "\n\n".
+            "The easiest way to resolve this issue is to set %s to %s in your ".
+            "PHP configuration, to disable the memory limit. There is ".
+            "usually little or no value to using this option to limit ".
+            "Phabricator process memory.".
+            "\n\n".
+            "You can also increase the limit, or decrease %s, or ignore this ".
+            "issue and accept that these upload mechanisms will be limited ".
+            "in the size of files they can handle.",
+            phutil_tag('tt', array(), 'storage.upload-size-limit'),
+            phutil_format_bytes($upload_limit_bytes),
+            phutil_format_bytes($available_bytes),
+            phutil_tag('tt', array(), 'memory_limit'),
+            phutil_tag('tt', array(), '-1'),
+            phutil_tag('tt', array(), 'storage.upload-size-limit'));
+
+          $this
+            ->newIssue('php.memory_limit.upload')
+            ->setName(pht('Memory Limit Restricts File Uploads'))
+            ->setSummary($summary)
+            ->setMessage($message)
+            ->addPHPConfig('memory_limit')
+            ->addPHPConfigOriginalValue('memory_limit', $memory_limit)
+            ->addPhabricatorConfig('storage.upload-size-limit');
+        }
+      }
     }
 
+
     $local_path = PhabricatorEnv::getEnvConfig('storage.local-disk.path');
     if (!$local_path) {
       return;

src/applications/config/issue/PhabricatorSetupIssue.php

@@ -16,6 +16,7 @@ final class PhabricatorSetupIssue {
   private $phpConfig = array();
   private $commands = array();
   private $mysqlConfig = array();
+  private $originalPHPConfigValues = array();
 
   public function addCommand($command) {
     $this->commands[] = $command;
@@ -82,6 +83,28 @@ final class PhabricatorSetupIssue {
     return $this;
   }
 
+  /**
+   * Set an explicit value to display when showing the user PHP configuration
+   * values.
+   *
+   * If Phabricator has changed a value by the time a config issue is raised,
+   * you can provide the original value here so the UI makes sense. For example,
+   * we alter `memory_limit` during startup, so if the original value is not
+   * provided it will look like it is always set to `-1`.
+   *
+   * @param string PHP configuration option to provide a value for.
+   * @param string Explicit value to show in the UI.
+   * @return this
+   */
+  public function addPHPConfigOriginalValue($php_config, $value) {
+    $this->originalPHPConfigValues[$php_config] = $value;
+    return $this;
+  }
+
+  public function getPHPConfigOriginalValue($php_config, $default = null) {
+    return idx($this->originalPHPConfigValues, $php_config, $default);
+  }
+
   public function getPHPConfig() {
     return $this->phpConfig;
   }

src/applications/config/view/PhabricatorSetupIssueView.php

@@ -26,7 +26,7 @@ final class PhabricatorSetupIssueView extends AphrontView {
 
     $configs = $issue->getPHPConfig();
     if ($configs) {
-      $description[] = $this->renderPHPConfig($configs);
+      $description[] = $this->renderPHPConfig($configs, $issue);
     }
 
     $configs = $issue->getMySQLConfig();
@@ -243,7 +243,7 @@ final class PhabricatorSetupIssueView extends AphrontView {
       ));
   }
 
-  private function renderPHPConfig(array $configs) {
+  private function renderPHPConfig(array $configs, $issue) {
     $table_info = phutil_tag(
       'p',
       array(),
@@ -253,7 +253,9 @@ final class PhabricatorSetupIssueView extends AphrontView {
 
     $dict = array();
     foreach ($configs as $key) {
-      $dict[$key] = ini_get($key);
+      $dict[$key] = $issue->getPHPConfigOriginalValue(
+        $key,
+        ini_get($key));
     }
 
     $table = $this->renderValueTable($dict);

support/PhabricatorStartup.php

@@ -41,6 +41,7 @@ final class PhabricatorStartup {
   private static $globals = array();
   private static $capturingOutput;
   private static $rawInput;
+  private static $oldMemoryLimit;
 
   // TODO: For now, disable rate limiting entirely by default. We need to
   // iterate on it a bit for Conduit, some of the specific score levels, and
@@ -310,6 +311,7 @@ final class PhabricatorStartup {
    */
   private static function setupPHP() {
     error_reporting(E_ALL | E_STRICT);
+    self::$oldMemoryLimit = ini_get('memory_limit');
     ini_set('memory_limit', -1);
 
     // If we have libxml, disable the incredibly dangerous entity loader.
@@ -318,6 +320,14 @@ final class PhabricatorStartup {
     }
   }
 
+
+  /**
+   * @task validation
+   */
+  public static function getOldMemoryLimit() {
+    return self::$oldMemoryLimit;
+  }
+
   /**
    * @task validation
    */