Fix commit policy stuff and anchor handling

Summary:
See discussion in D5121. Fixes T2615.

This might cause us more issues if anything is loading commit handles without passing a viewer, but I think I tested all of those cases.

Test Plan: Looked at feed, audit, maniphest, diffusion, differential, owners, repositories.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2615

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

src/applications/diffusion/query/DiffusionCommitQuery.php

@@ -4,6 +4,7 @@ final class DiffusionCommitQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
   private $identifiers;
+  private $phids;
 
   /**
    * Load commits by partial or full identifiers, e.g. "rXab82393", "rX1234",
@@ -16,6 +17,11 @@ final class DiffusionCommitQuery
     return $this;
   }
 
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
   public function loadPage() {
     $table = new PhabricatorRepositoryCommit();
     $conn_r = $table->establishConnection('r');
@@ -134,6 +140,13 @@ final class DiffusionCommitQuery
       }
     }
 
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
     return $this->formatWhereClause($where);
   }
 

src/applications/phid/handle/PhabricatorObjectHandleData.php

@@ -47,10 +47,10 @@ final class PhabricatorObjectHandleData {
         return mpull($users, null, 'getPHID');
 
       case PhabricatorPHIDConstants::PHID_TYPE_CMIT:
-        $commit_dao = new PhabricatorRepositoryCommit();
+        $commits = id(new DiffusionCommitQuery())
-        $commits = $commit_dao->putInSet(new LiskDAOSet())->loadAllWhere(
+          ->setViewer($this->viewer)
-          'phid IN (%Ls)',
+          ->withPHIDs($phids)
-          $phids);
+          ->execute();
         return mpull($commits, null, 'getPHID');
 
       case PhabricatorPHIDConstants::PHID_TYPE_TASK:
@@ -329,17 +329,10 @@ final class PhabricatorObjectHandleData {
             $handle->setPHID($phid);
             $handle->setType($type);
 
-            $repository = null;
+            if (empty($objects[$phid])) {
-            if (!empty($objects[$phid])) {
-              $repository = $objects[$phid]->loadOneRelative(
-                new PhabricatorRepository(),
-                'id',
-                'getRepositoryID');
-            }
-
-            if (!$repository) {
               $handle->setName('Unknown Commit');
             } else {
+              $repository = $objects[$phid]->getRepository();
               $commit = $objects[$phid];
               $callsign = $repository->getCallsign();
               $commit_identifier = $commit->getCommitIdentifier();
@@ -358,6 +351,7 @@ final class PhabricatorObjectHandleData {
               $handle->setTimestamp($commit->getEpoch());
               $handle->setComplete(true);
             }
+
             $handles[$phid] = $handle;
           }
           break;

src/infrastructure/markup/PhabricatorMarkupEngine.php

@@ -41,7 +41,7 @@ final class PhabricatorMarkupEngine {
 
   private $objects = array();
   private $viewer;
-  private $version = 5;
+  private $version = 6;
 
 
 /* -(  Markup Pipeline  )---------------------------------------------------- */

src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php

@@ -41,14 +41,14 @@ abstract class PhabricatorRemarkupRuleObject
     $text = $this->getObjectNamePrefix().$id;
     if ($anchor) {
       $matches = null;
-      if (preg_match('@^#(?:comment-)?(\d{1,7})$@', $anchor, $matches)) {
+      if (preg_match('@^(?:comment-)?(\d{1,7})$@', $anchor, $matches)) {
         // Maximum length is 7 because 12345678 could be a file hash in
         // Differential.
-        $href = $href."#comment-".$matches[1];
+        $href = $href.'#comment-'.$matches[1];
-        $text = $text."#".$matches[1];
+        $text = $text.'#'.$matches[1];
       } else {
-        $href = $href.$anchor;
+        $href = $href.'#'.$anchor;
-        $text = $text.$anchor;
+        $text = $text.'#'.$anchor;
       }
     }
 
@@ -92,8 +92,11 @@ abstract class PhabricatorRemarkupRuleObject
       array($this, 'markupObjectEmbed'),
       $text);
 
+    // NOTE: The "(?<!#)" prevents us from linking "#abcdef" or similar. The
+    // "\b" allows us to link "(abcdef)" or similar without linking things
+    // in the middle of words.
     $text = preg_replace_callback(
-      '@\b'.$prefix.'('.$id.')(?:#([-\w\d]+))?\b@',
+      '@(?<!#)\b'.$prefix.'('.$id.')(?:#([-\w\d]+))?\b@',
       array($this, 'markupObjectReference'),
       $text);