Fix some issues caught by HipHop, and work around some issues

caused by HipHop.

externals/phpmailer/class.phpmailer-lite.php

@@ -370,7 +370,7 @@ class PHPMailerLite {
    */
   private function AddAnAddress($kind, $address, $name = '') {
     if (!preg_match('/^(to|cc|bcc|ReplyTo)$/', $kind)) {
-      echo 'Invalid recipient array: ' . kind;
+      echo 'Invalid recipient array: ' . $kind;
       return false;
     }
     $address = trim($address);

externals/xhprof/xhprof_lib.php

@@ -261,7 +261,7 @@ function xhprof_aggregate_runs($xhprof_runs_impl, $runs,
   $bad_runs = array();
   foreach($runs as $idx => $run_id) {
 
-    $raw_data = $xhprof_runs_impl->get_run($run_id, $source, $description);
+    $raw_data = $xhprof_runs_impl->get_run($run_id, $source, '?');
 
     // use the first run to derive what metrics to aggregate on.
     if ($idx == 0) {
@@ -283,7 +283,7 @@ function xhprof_aggregate_runs($xhprof_runs_impl, $runs,
     }
 
     if ($use_script_name) {
-      $page = $description;
+      $page = '?';
 
       // create a fake function '__script::$page', and have and edge from
       // main() to '__script::$page'. We will also need edges to transfer
@@ -589,7 +589,7 @@ function xhprof_prune_run($raw_data, $prune_percent) {
 
   $prune_threshold = (($main_info[$prune_metric] * $prune_percent) / 100.0);
 
-  init_metrics($raw_data, null, null, false);
+//  init_metrics($raw_data, null, null, false);
   $flat_info = xhprof_compute_inclusive_times($raw_data);
 
   foreach ($raw_data as $parent_child => $info) {

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -212,27 +212,22 @@ class AphrontDefaultApplicationConfiguration
         '<code>'.phutil_escape_html((string)$ex).'</code>'.
       '</div>';
 
-    if ($this->getRequest()->isAjax()) {
-      $dialog = new AphrontDialogView();
-      $dialog
-        ->setTitle('Exception!')
-        ->setClass('aphront-exception-dialog')
-        ->setUser($this->getRequest()->getUser())
-        ->appendChild($content)
-        ->addCancelButton('/');
-
-      $response = new AphrontDialogResponse();
-      $response->setDialog($dialog);
-
-      return $response;
+    $user = $this->getRequest()->getUser();
+    if (!$user) {
+      // If we hit an exception very early, we won't have a user.
+      $user = new PhabricatorUser();
     }
 
-    $view = new PhabricatorStandardPageView();
-    $view->setRequest($this->getRequest());
-    $view->appendChild($content);
+    $dialog = new AphrontDialogView();
+    $dialog
+      ->setTitle('Exception!')
+      ->setClass('aphront-exception-dialog')
+      ->setUser($user)
+      ->appendChild($content)
+      ->addCancelButton('/');
 
-    $response = new AphrontWebpageResponse();
-    $response->setContent($view->render());
+    $response = new AphrontDialogResponse();
+    $response->setDialog($dialog);
 
     return $response;
   }

src/aphront/default/configuration/__init__.php

@@ -12,6 +12,7 @@ phutil_require_module('phabricator', 'aphront/response/ajax');
 phutil_require_module('phabricator', 'aphront/response/dialog');
 phutil_require_module('phabricator', 'aphront/response/webpage');
 phutil_require_module('phabricator', 'applications/base/controller/404');
+phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'view/dialog');
 phutil_require_module('phabricator', 'view/page/failure');
 phutil_require_module('phabricator', 'view/page/standard');

src/applications/auth/controller/oauth/PhabricatorOAuthLoginController.php

@@ -65,21 +65,30 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController {
         'code'          => $code,
       );
 
+      $post_data = http_build_query($query_data);
+      $post_length = strlen($post_data);
+
       $stream_context = stream_context_create(
         array(
           'http' => array(
             'method'  => 'POST',
-            'header'  => 'Content-type: application/x-www-form-urlencoded',
-            'content' => http_build_query($query_data),
+            'header'  =>
+              "Content-Type: application/x-www-form-urlencoded\r\n".
+              "Content-Length: {$post_length}\r\n",
+            'content' => $post_data,
           ),
         ));
 
       $stream = fopen($auth_uri, 'r', false, $stream_context);
 
-      $meta = stream_get_meta_data($stream);
-      $response = stream_get_contents($stream);
+      $response = false;
+      $meta = null;
+      if ($stream) {
+        $meta = stream_get_meta_data($stream);
+        $response = stream_get_contents($stream);
+        fclose($stream);
+      }
 
-      fclose($stream);
 
       if ($response === false) {
         return $this->buildErrorResponse(new PhabricatorOAuthFailureView());
@@ -127,7 +136,6 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController {
       $user_id);
 
     if ($current_user->getPHID()) {
-
       if ($known_oauth) {
         if ($known_oauth->getUserID() != $current_user->getID()) {
           $dialog = new AphrontDialogView();
@@ -285,11 +293,19 @@ class PhabricatorOAuthLoginController extends PhabricatorAuthController {
           $request->setCookie('phsid', $session_key);
           return id(new AphrontRedirectResponse())->setURI('/');
         } catch (AphrontQueryDuplicateKeyException $exception) {
-          $key = $exception->getDuplicateKey();
-          if ($key == 'userName') {
+
+          $same_username = id(new PhabricatorUser())->loadOneWhere(
+            'userName = %s',
+            $user->getUserName());
+
+          $same_email = id(new PhabricatorUser())->loadOneWhere(
+            'email = %s',
+            $user->getEmail());
+
+          if ($same_username) {
             $e_username = 'Duplicate';
-            $errors[] = 'That username is not unique.';
-          } else if ($key == 'email') {
+            $errors[] = 'That username or email is not unique.';
+          } else if ($same_email) {
             $e_email = 'Duplicate';
             $errors[] = 'That email is not unique.';
           } else {

src/applications/conduit/method/differential/markcommitted/ConduitAPI_differential_markcommitted_Method.php

@@ -60,8 +60,7 @@ class ConduitAPI_differential_markcommitted_Method extends ConduitAPIMethod {
     $editor = new DifferentialCommentEditor(
       $revision,
       $revision->getAuthorPHID(),
-      DifferentialAction::ACTION_COMMIT,
-      $inline_comments = array());
+      DifferentialAction::ACTION_COMMIT);
     $editor->save();
 
     $revision->setStatus(DifferentialRevisionStatus::COMMITTED);

src/applications/differential/data/revisionlist/DifferentialRevisionListData.php

@@ -165,7 +165,7 @@ class DifferentialRevisionListData {
 
         $this->revisions = $rev->loadAllFromArray($data);
         break;
-      case self::QUERY_BY_PHID:
+      case self::QUERY_PHIDS:
         $this->revisions = $this->loadAllWhere(
           'revision.phid in (%Ls)',
           $this->ids);
@@ -233,8 +233,11 @@ class DifferentialRevisionListData {
   }
 
   private function loadAllOpenWithCCs(array $ccphids) {
+    $rev = new DifferentialRevision();
+
     $revision = new DifferentialRevision();
     $data = queryfx_all(
+      $rev->establishConnection('r'),
       'SELECT revision.* FROM %T revision
         JOIN %T relationship ON relationship.revisionID = revision.id
           AND relationship.relation = %s

src/applications/differential/parser/changeset/DifferentialChangesetParser.php

@@ -516,7 +516,7 @@ class DifferentialChangesetParser {
         $changeset->getTableName().'_parse_cache',
         $this->changesetID,
         $cache);
-    } catch (QueryException $ex) {
+    } catch (AphrontQueryException $ex) {
       // TODO: uhoh
     }
   }

src/applications/maniphest/view/tasklist/ManiphestTaskListView.php

@@ -35,7 +35,7 @@ class ManiphestTaskListView extends AphrontView {
 
     $views = array();
     foreach ($this->tasks as $task) {
-      $view = new ManiphestTaskSummaryView($task);
+      $view = new ManiphestTaskSummaryView();
       $view->setTask($task);
       $view->setHandles($this->handles);
       $views[] = $view->render();

src/applications/maniphest/view/transactionlist/ManiphestTransactionListView.php

@@ -72,7 +72,7 @@ class ManiphestTransactionListView extends AphrontView {
     }
 
     foreach ($groups as $group) {
-      $view = new ManiphestTransactionDetailView($transaction);
+      $view = new ManiphestTransactionDetailView();
       $view->setTransactionGroup($group);
       $view->setHandles($this->handles);
       $view->setMarkupEngine($this->markupEngine);

src/applications/repository/controller/create/PhabricatorRepositoryCreateController.php

@@ -69,14 +69,10 @@ class PhabricatorRepositoryCreateController extends PhabricatorController {
           return id(new AphrontRedirectResponse())
             ->setURI('/repository/edit/'.$repository->getID().'/');
 
-        } catch (PhabricatorQueryDuplicateKeyException $ex) {
-          if ($ex->getDuplicateKey() == 'callsign') {
-            $e_callsign = 'Duplicate';
-            $errors[] = 'Callsign must be unique. Another repository already '.
-                        'uses that callsign.';
-          } else {
-            throw $ex;
-          }
+        } catch (AphrontQueryDuplicateKeyException $ex) {
+          $e_callsign = 'Duplicate';
+          $errors[] = 'Callsign must be unique. Another repository already '.
+                      'uses that callsign.';
         }
       }
     }

src/storage/connection/base/AphrontDatabaseConnection.php

@@ -36,15 +36,21 @@ abstract class AphrontDatabaseConnection {
   abstract public function escapeStringForLikeClause($string);
 
   public function queryData($pattern/*, $arg, $arg, ... */) {
+    if (false) {
+      // Workaround for the HPHP workaround: ensure we include this module
+      // since we really are using the function.
+      queryfx($this, $pattern);
+    }
+
     $args = func_get_args();
     array_unshift($args, $this);
-    return call_user_func_array('queryfx_all', $args);
+    return hphp_workaround_call_user_func_array('queryfx_all', $args);
   }
 
   public function query($pattern/*, $arg, $arg, ... */) {
     $args = func_get_args();
     array_unshift($args, $this);
-    return call_user_func_array('queryfx', $args);
+    return hphp_workaround_call_user_func_array('queryfx', $args);
   }
 
   // TODO: Probably need to reset these when we catch a connection exception

src/storage/connection/mysql/AphrontMySQLDatabaseConnection.php

@@ -234,12 +234,11 @@ class AphrontMySQLDatabaseConnection extends AphrontDatabaseConnection {
       case 1205: // Lock wait timeout exceeded
         throw new AphrontQueryRecoverableException("#{$errno}: {$error}");
       case 1062: // Duplicate Key
-        $matches = null;
-        $key = null;
-        if (preg_match('/for key \'(.*)\'$/', $error, $matches)) {
-          $key = $matches[1];
-        }
-        throw new AphrontQueryDuplicateKeyException($key, "{$errno}: {$error}");
+        // NOTE: In some versions of MySQL we get a key name back here, but
+        // older versions just give us a key index ("key 2") so it's not
+        // portable to parse the key out of the error and attach it to the
+        // exception.
+        throw new AphrontQueryDuplicateKeyException("{$errno}: {$error}");
       default:
         // TODO: 1064 is syntax error, and quite terrible in production.
         throw new AphrontQueryException("#{$errno}: {$error}");

src/storage/exception/duplicatekey/AphrontQueryDuplicateKeyException.php

@@ -21,14 +21,4 @@
  */
 class AphrontQueryDuplicateKeyException extends AphrontQueryException {
 
-  private $duplicateKey;
-
-  public function getDuplicateKey() {
-    return $this->duplicateKey;
-  }
-
-  public function __construct($duplicate_key, $message) {
-    $this->duplicateKey = $duplicate_key;
-    parent::__construct($message);
-  }
 }

src/storage/queryfx/queryfx.php

@@ -20,9 +20,15 @@
  * @group storage
  */
 function queryfx(AphrontDatabaseConnection $conn, $sql/*, ... */) {
+  if (false) {
+    // Workaround for the HPHP workaround: ensure we include this module
+    // since we really are using the function.
+    qsprintf($conn, $sql);
+  }
+
   $argv = func_get_args();
-  $query = call_user_func_array('qsprintf', $argv);
-  return $conn->executeRawQuery($query);
+  $query = hphp_workaround_call_user_func_array('qsprintf', $argv);
+  $conn->executeRawQuery($query);
 }
 
 /**
@@ -30,7 +36,7 @@ function queryfx(AphrontDatabaseConnection $conn, $sql/*, ... */) {
  */
 function vqueryfx($conn, $sql, $argv) {
   array_unshift($argv, $conn, $sql);
-  return call_user_func_array('queryfx', $argv);
+  hphp_workaround_call_user_func_array('queryfx', $argv);
 }
 
 /**
@@ -38,8 +44,8 @@ function vqueryfx($conn, $sql, $argv) {
  */
 function queryfx_all($conn, $sql/*, ... */) {
   $argv = func_get_args();
-  $ret = call_user_func_array('queryfx', $argv);
-  return $conn->selectAllResults($ret);
+  hphp_workaround_call_user_func_array('queryfx', $argv);
+  return $conn->selectAllResults();
 }
 
 /**
@@ -47,7 +53,7 @@ function queryfx_all($conn, $sql/*, ... */) {
  */
 function queryfx_one($conn, $sql/*, ... */) {
   $argv = func_get_args();
-  $ret = call_user_func_array('queryfx_all', $argv);
+  $ret = hphp_workaround_call_user_func_array('queryfx_all', $argv);
   if (count($ret) > 1) {
     throw new AphrontQueryCountException(
       'Query returned more than one row.');
@@ -59,6 +65,6 @@ function queryfx_one($conn, $sql/*, ... */) {
 
 function vqueryfx_all($conn, $sql, array $argv) {
   array_unshift($argv, $conn, $sql);
-  $ret = call_user_func_array('queryfx', $argv);
-  return $conn->selectAllResults($ret);
+  hphp_workaround_call_user_func_array('queryfx', $argv);
+  return $conn->selectAllResults();
 }

src/view/layout/headsup/actionlist/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'infrastructure/celerity/api');
 phutil_require_module('phabricator', 'view/base');
 
 

src/view/page/standard/PhabricatorStandardPageView.php

@@ -152,9 +152,12 @@ class PhabricatorStandardPageView extends AphrontPageView {
 
     $login_stuff = null;
     $request = $this->getRequest();
+    $user = null;
     if ($request) {
       $user = $request->getUser();
-      if ($user->getPHID()) {
+      // NOTE: user may not be set here if we caught an exception early
+      // in the execution workflow.
+      if ($user && $user->getPHID()) {
         $login_stuff =
           'Logged in as '.phutil_render_tag(
             'a',
@@ -203,16 +206,19 @@ class PhabricatorStandardPageView extends AphrontPageView {
       }
       $foot_links[] = $link;
     }
-    // This ends up very early in tab order at the top of the page and there's
-    // a bunch of junk up there anyway, just shove it down here.
-    $foot_links[] = phabricator_render_form(
-      $user,
-      array(
-        'action' => '/logout/',
-        'method' => 'post',
-        'style'  => 'display: inline',
-      ),
-      '<button class="link">Logout</button>');
+
+    if ($user && $user->getPHID()) {
+      // This ends up very early in tab order at the top of the page and there's
+      // a bunch of junk up there anyway, just shove it down here.
+      $foot_links[] = phabricator_render_form(
+        $user,
+        array(
+          'action' => '/logout/',
+          'method' => 'post',
+          'style'  => 'display: inline',
+        ),
+        '<button class="link">Logout</button>');
+    }
 
     $foot_links = implode(' &middot; ', $foot_links);
 

webroot/index.php

@@ -19,6 +19,11 @@
 error_reporting(E_ALL | E_STRICT);
 
 $env = getenv('PHABRICATOR_ENV'); // Apache
+if (!$env) {
+  if (isset($_ENV['PHABRICATOR_ENV'])) {
+    $env = $_ENV['PHABRICATOR_ENV'];
+  }
+}
 
 if (!$env) {
   phabricator_fatal_config_error(
@@ -172,3 +177,11 @@ function phabricator_fatal_config_error($msg) {
   die();
 }
 
+/**
+ * Workaround for HipHop bug, see Facebook Task #503624.
+ */
+function hphp_workaround_call_user_func_array($func, array $array) {
+  $f = new ReflectionFunction($func);
+  return $f->invokeArgs($array);
+}
+