Allow Git and Mercurial repositories to be cloned with names in the URI

Summary:
Ref T4175. This allows these URIs to all be valid for Git and Mercurial:

  /diffusion/X/
  /diffusion/X/anything.git
  /diffusion/X/anything/

This mostly already works, it just needed a few tweaks.

Test Plan: Cloned git and hg working copies using HTTP and SSH.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T4175

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

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -551,15 +551,13 @@ final class DiffusionRepositoryController extends DiffusionController {
     switch ($repository->getVersionControlSystem()) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
         $command = csprintf(
-          'git clone %s %s',
-          $uri,
-          $repository->getCloneName());
+          'git clone %s',
+          $uri);
         break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
         $command = csprintf(
-          'hg clone %s %s',
-          $uri,
-          $repository->getCloneName());
+          'hg clone %s',
+          $uri);
         break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
         $command = csprintf(

src/applications/diffusion/controller/DiffusionRepositoryDefaultController.php

@@ -6,6 +6,12 @@ final class DiffusionRepositoryDefaultController extends DiffusionController {
     // NOTE: This controller is just here to make sure we call
     // willBeginExecution() on any /diffusion/X/ URI, so we can intercept
     // `git`, `hg` and `svn` HTTP protocol requests.
-    return new Aphront404Response();
+
+    // If we made it here, it's probably because the user copy-pasted a
+    // clone URI with "/anything.git" at the end into their web browser.
+    // Send them to the canonical repository URI.
+
+    return id(new AphrontRedirectResponse())
+      ->setURI($this->getDiffusionRequest()->getRepository()->getURI());
   }
 }

src/applications/diffusion/controller/DiffusionServeController.php

@@ -51,6 +51,7 @@ final class DiffusionServeController extends DiffusionController {
     if (!preg_match($regex, (string)$uri, $matches)) {
       return null;
     }
+
     return $matches['callsign'];
   }
 
@@ -244,7 +245,7 @@ final class DiffusionServeController extends DiffusionController {
     switch ($repository->getVersionControlSystem()) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
         $service = $request->getStr('service');
-        $path = $this->getRequestDirectoryPath();
+        $path = $this->getRequestDirectoryPath($repository);
         // NOTE: Service names are the reverse of what you might expect, as they
         // are from the point of view of the server. The main read service is
         // "git-upload-pack", and the main write service is "git-receive-pack".
@@ -282,7 +283,7 @@ final class DiffusionServeController extends DiffusionController {
     PhabricatorUser $viewer) {
     $request = $this->getRequest();
 
-    $request_path = $this->getRequestDirectoryPath();
+    $request_path = $this->getRequestDirectoryPath($repository);
     $repository_root = $repository->getLocalPath();
 
     // Rebuild the query string to strip `__magic__` parameters and prevent
@@ -351,10 +352,33 @@ final class DiffusionServeController extends DiffusionController {
     return id(new DiffusionGitResponse())->setGitData($stdout);
   }
 
-  private function getRequestDirectoryPath() {
+  private function getRequestDirectoryPath(PhabricatorRepository $repository) {
     $request = $this->getRequest();
     $request_path = $request->getRequestURI()->getPath();
-    return preg_replace('@^/diffusion/[A-Z]+@', '', $request_path);
+    $base_path = preg_replace('@^/diffusion/[A-Z]+@', '', $request_path);
+
+    // For Git repositories, strip an optional directory component if it
+    // isn't the name of a known Git resource. This allows users to clone
+    // repositories as "/diffusion/X/anything.git", for example.
+    if ($repository->isGit()) {
+      $known = array(
+        'info',
+        'git-upload-pack',
+        'git-receive-pack',
+      );
+
+      foreach ($known as $key => $path) {
+        $known[$key] = preg_quote($path, '@');
+      }
+
+      $known = implode('|', $known);
+
+      if (preg_match('@^/([^/]+)/('.$known.')(/|$)@', $base_path)) {
+        $base_path = preg_replace('@^/([^/]+)@', '', $base_path);
+      }
+    }
+
+    return $base_path;
   }
 
   private function authenticateHTTPRepositoryUser(

src/applications/repository/data/PhabricatorRepositoryURINormalizer.php

@@ -108,6 +108,15 @@ final class PhabricatorRepositoryURINormalizer extends Phobject {
         break;
     }
 
+    // If this is a Phabricator URI, strip it down to the callsign. We mutably
+    // allow you to clone repositories as "/diffusion/X/anything.git", for
+    // example.
+
+    $matches = null;
+    if (preg_match('@^(diffusion/[A-Z]+)@', $path, $matches)) {
+      $path = $matches[1];
+    }
+
     return $path;
   }
 

src/applications/repository/data/__tests__/PhabricatorRepositoryURINormalizerTestCase.php

@@ -16,6 +16,8 @@ final class PhabricatorRepositoryURINormalizerTestCase
       'user@domain.com:path/repo/' => 'path/repo',
       'file:///path/to/local/repo.git' => 'path/to/local/repo',
       '/path/to/local/repo.git' => 'path/to/local/repo',
+      'ssh://something.com/diffusion/X/anything.git' => 'diffusion/X',
+      'ssh://something.com/diffusion/X/' => 'diffusion/X',
     );
 
     $type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;

src/applications/repository/storage/PhabricatorRepository.php

@@ -833,6 +833,12 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
       $uri->setProtocol('ssh');
     }
 
+    if ($this->isGit()) {
+      $uri->setPath($uri->getPath().$this->getCloneName().'.git');
+    } else if ($this->isHg()) {
+      $uri->setPath($uri->getPath().$this->getCloneName().'/');
+    }
+
     $ssh_user = PhabricatorEnv::getEnvConfig('diffusion.ssh-user');
     if ($ssh_user) {
       $uri->setUser($ssh_user);
@@ -861,8 +867,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
       return null;
     }
 
-
     $uri = PhabricatorEnv::getProductionURI($this->getURI());
+    $uri = new PhutilURI($uri);
+
+    if ($this->isGit()) {
+      $uri->setPath($uri->getPath().$this->getCloneName().'.git');
+    } else if ($this->isHg()) {
+      $uri->setPath($uri->getPath().$this->getCloneName().'/');
+    }
 
     return $uri;
   }