(stable) Promote 2020 Week 37

src/conduit/ArcanistConduitCallFuture.php

@@ -72,15 +72,18 @@ final class ArcanistConduitCallFuture
   }
 
   protected function didReceiveException($exception) {
-    switch ($exception->getErrorCode()) {
+
-      case 'ERR-INVALID-SESSION':
+    if ($exception instanceof ConduitClientException) {
-        if (!$this->getEngine()->getConduitToken()) {
+      switch ($exception->getErrorCode()) {
-          $this->raiseLoginRequired();
+        case 'ERR-INVALID-SESSION':
-        }
+          if (!$this->getEngine()->getConduitToken()) {
-        break;
+            $this->raiseLoginRequired();
-      case 'ERR-INVALID-AUTH':
+          }
-        $this->raiseInvalidAuth();
+          break;
-        break;
+        case 'ERR-INVALID-AUTH':
+          $this->raiseInvalidAuth();
+          break;
+      }
     }
 
     throw $exception;

src/conduit/ConduitFuture.php

@@ -62,9 +62,14 @@ final class ConduitFuture extends FutureProxy {
     }
 
     if ($data['error_code']) {
+      $message = pht(
+        '<%s> %s',
+        $this->conduitMethod,
+        $data['error_info']);
+
       throw new ConduitClientException(
         $data['error_code'],
-        $data['error_info']);
+        $message);
     }
 
     $result = $data['result'];

src/future/FutureIterator.php

@@ -194,7 +194,14 @@ final class FutureIterator
    * @task iterator
    */
   public function next() {
-    $this->key = null;
+    // See T13572. If we preivously resolved and returned a Future, release
+    // it now. This prevents us from holding Futures indefinitely when callers
+    // like FuturePool build long-lived iterators and keep adding new Futures
+    // to them.
+    if ($this->key !== null) {
+      unset($this->futures[$this->key]);
+      $this->key = null;
+    }
 
     $this->updateWorkingSet();
 

src/future/exec/execx.php

@@ -47,7 +47,7 @@ function exec_manual($cmd /* , ... */) {
  */
 function phutil_passthru($cmd /* , ... */) {
   $args = func_get_args();
-  return newv('PhutilExecPassthru', $args)->execute();
+  return newv('PhutilExecPassthru', $args)->resolve();
 }
 
 

src/future/http/HTTPSFuture.php

@@ -132,7 +132,7 @@ final class HTTPSFuture extends BaseHTTPFuture {
    * @return string|false
    */
   public static function loadContent($uri, $timeout = null) {
-    $future = new HTTPSFuture($uri);
+    $future = new self($uri);
     if ($timeout !== null) {
       $future->setTimeout($timeout);
     }
@@ -151,7 +151,8 @@ final class HTTPSFuture extends BaseHTTPFuture {
       throw new Exception(
         pht(
           'Specified download path "%s" already exists, refusing to '.
-          'overwrite.'));
+          'overwrite.',
+          $download_path));
     }
 
     return $this;

src/hardpoint/ArcanistHardpointList.php

@@ -27,7 +27,9 @@ final class ArcanistHardpointList
           pht(
             'Hardpoint (at index "%s") has the same key ("%s") as an earlier '.
             'hardpoint. Each hardpoint must have a key that is unique '.
-            'among hardpoints on the object.'));
+            'among hardpoints on the object.',
+            $idx,
+            $key));
       }
 
       $map[$key] = $hardpoint;

src/land/engine/ArcanistLandEngine.php

@@ -1459,6 +1459,7 @@ abstract class ArcanistLandEngine
             'Merge strategy "%s" specified in "%s" configuration is '.
             'unknown. Supported merge strategies are: %s.',
             $strategy,
+            $this->getStrategyConfigurationKey(),
             $strategy_list));
       }
 

src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php

@@ -82,7 +82,12 @@ final class ArcanistFormattedStringXHPASTLinterRule
       }
 
       $format = $parameters->getChildByIndex($start);
-      if ($format->getTypeName() != 'n_STRING_SCALAR') {
+      if (!$format->isConstantString()) {
+
+        // TODO: When this parameter is not a constant string, the call may
+        // be unsafe. We should make some attempt to warn about this for
+        // "qsprintf()" and other security-sensitive functions.
+
         continue;
       }
 

src/lint/linter/xhpast/rules/__tests__/formatted-string/formatted-string.lint-test

@@ -11,12 +11,28 @@ fprintf(null, 'x');
 queryfx(null, 'x', 'y');
 
 foobar(null, null, '%s');
+
+pht('x %s y');
+pht('x %s y'.'z');
+
+pht(<<<HEREDOC
+a b c
+HEREDOC
+  );
+
+pht(<<<HEREDOC
+a %s c
+HEREDOC
+  );
 ~~~~~~~~~~
 error:3:1:XHP54:Formatted String
 error:7:1:XHP54:Formatted String
 error:8:1:XHP54:Formatted String
 error:11:1:XHP54:Formatted String
 error:13:1:XHP54:Formatted String
+error:15:1:XHP54:Formatted String
+error:16:1:XHP54:Formatted String
+error:23:1:XHP54:Formatted String
 ~~~~~~~~~~
 ~~~~~~~~~~
 {

src/parser/xhpast/api/XHPASTNode.php

@@ -33,7 +33,9 @@ final class XHPASTNode extends AASTNode {
         return $this->getChildByIndex(0)->evalStatic();
         break;
       case 'n_STRING_SCALAR':
-        return (string)$this->getStringLiteralValue();
+        return phutil_string_cast($this->getStringLiteralValue());
+      case 'n_HEREDOC':
+        return phutil_string_cast($this->getStringLiteralValue());
       case 'n_NUMERIC_SCALAR':
         $value = $this->getSemanticString();
         if (preg_match('/^0x/i', $value)) {
@@ -186,31 +188,51 @@ final class XHPASTNode extends AASTNode {
   }
 
   public function getStringLiteralValue() {
-    if ($this->getTypeName() != 'n_STRING_SCALAR') {
+    $type_name = $this->getTypeName();
-      return null;
+
+    if ($type_name === 'n_HEREDOC') {
+      $value = $this->getSemanticString();
+      $value = phutil_split_lines($value);
+      $value = array_slice($value, 1, -1);
+      $value = implode('', $value);
+
+      // Strip the final newline from value, this isn't part of the string
+      // literal.
+      $value = preg_replace('/(\r|\n|\r\n)\z/', '', $value);
+
+      return $this->newStringLiteralFromSemanticString($value);
     }
 
-    $value = $this->getSemanticString();
+    if ($type_name === 'n_STRING_SCALAR') {
-    $type  = $value[0];
+      $value = $this->getSemanticString();
-    $value = preg_replace('/^b?[\'"]|[\'"]$/i', '', $value);
+      $type  = $value[0];
-    $esc   = false;
+      $value = preg_replace('/^b?[\'"]|[\'"]$/i', '', $value);
-    $len   = strlen($value);
-    $out   = '';
 
-    if ($type == "'") {
+      if ($type == "'") {
-      // Single quoted strings treat everything as a literal except "\\" and
+        // Single quoted strings treat everything as a literal except "\\" and
-      // "\'".
+        // "\'".
-      return str_replace(
+        return str_replace(
-        array('\\\\', '\\\''),
+          array('\\\\', '\\\''),
-        array('\\',   "'"),
+          array('\\',   "'"),
-        $value);
+          $value);
+      }
+
+      return $this->newStringLiteralFromSemanticString($value);
     }
 
+    return null;
+  }
+
+  private function newStringLiteralFromSemanticString($value) {
     // Double quoted strings treat "\X" as a literal if X isn't specifically
     // a character which needs to be escaped -- e.g., "\q" and "\'" are
     // literally "\q" and "\'". stripcslashes() is too aggressive, so find
     // all these under-escaped backslashes and escape them.
 
+    $len = strlen($value);
+    $esc = false;
+    $out = '';
+
     for ($ii = 0; $ii < $len; $ii++) {
       $c = $value[$ii];
       if ($esc) {

src/repository/api/ArcanistMercurialAPI.php

@@ -778,7 +778,7 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
                 pht(
                   "'%s' has been amended with 'Differential Revision:', ".
                   "as specified by '%s' in your %s 'base' configuration.",
-                  '.'.
+                  '.',
                   $rule,
                   $source));
               // NOTE: This should be safe because Mercurial doesn't support

src/runtime/ArcanistRuntime.php

@@ -553,6 +553,7 @@ final class ArcanistRuntime {
             'workflow in a given toolset must have a unique name.',
             get_class($workflow),
             get_class($map[$key]),
+            $key,
             get_class($toolset),
             $toolset->getToolsetKey()));
       }

src/toolset/ArcanistAliasEngine.php

@@ -72,8 +72,8 @@ final class ArcanistAliasEngine
           pht(
             'Configuration source ("%s") defines an invalid alias, which '.
             'will be ignored: %s',
-            $alias->getConfigurationSource()->getSourceDisplayName()),
+            $alias->getConfigurationSource()->getSourceDisplayName(),
-            $exception->getMessage());
+            $exception->getMessage()));
     }
 
     $command = array_shift($argv);

src/workflow/ArcanistCloseRevisionWorkflow.php

@@ -35,8 +35,7 @@ EOTEXT
         'help' => pht(
           "Close only if the repository is untracked and the revision is ".
           "accepted. Continue even if the close can't happen. This is a soft ".
-          "version of '' used by other workflows.",
+          "version of 'close-revision' used by other workflows."),
-          'close-revision'),
       ),
       'quiet' => array(
         'help' => pht('Do not print a success message.'),