diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -46,6 +46,53 @@ } public function getRoutes() { + $repository_routes = array( + '/' => array( + '' => 'DiffusionRepositoryController', + 'repository/(?P.*)' => 'DiffusionRepositoryController', + 'change/(?P.*)' => 'DiffusionChangeController', + 'history/(?P.*)' => 'DiffusionHistoryController', + 'browse/(?P.*)' => 'DiffusionBrowseController', + 'lastmodified/(?P.*)' => 'DiffusionLastModifiedController', + 'diff/' => 'DiffusionDiffController', + 'tags/(?P.*)' => 'DiffusionTagListController', + 'branches/(?P.*)' => 'DiffusionBranchTableController', + 'refs/(?P.*)' => 'DiffusionRefTableController', + 'lint/(?P.*)' => 'DiffusionLintController', + 'commit/(?P[a-z0-9]+)/branches/' + => 'DiffusionCommitBranchesController', + 'commit/(?P[a-z0-9]+)/tags/' + => 'DiffusionCommitTagsController', + 'commit/(?P[a-z0-9]+)/edit/' + => 'DiffusionCommitEditController', + 'manage/(?:(?P[^/]+)/)?' + => 'DiffusionRepositoryManagePanelsController', + 'uri/' => array( + 'view/(?P[0-9]\d*)/' => 'DiffusionRepositoryURIViewController', + 'disable/(?P[0-9]\d*)/' + => 'DiffusionRepositoryURIDisableController', + $this->getEditRoutePattern('edit/') + => 'DiffusionRepositoryURIEditController', + 'credential/(?P[0-9]\d*)/(?Pedit|remove)/' + => 'DiffusionRepositoryURICredentialController', + ), + 'edit/' => array( + 'activate/' => 'DiffusionRepositoryEditActivateController', + 'dangerous/' => 'DiffusionRepositoryEditDangerousController', + 'delete/' => 'DiffusionRepositoryEditDeleteController', + 'update/' => 'DiffusionRepositoryEditUpdateController', + 'testautomation/' => 'DiffusionRepositoryTestAutomationController', + ), + 'pathtree/(?P.*)' => 'DiffusionPathTreeController', + ), + + // NOTE: This must come after the rules above; it just gives us a + // catch-all for serving repositories over HTTP. We must accept requests + // without the trailing "/" because SVN commands don't necessarily + // include it. + '(?:/.*)?' => 'DiffusionRepositoryDefaultController', + ); + return array( '/(?:'. 'r(?P[A-Z]+)'. @@ -54,6 +101,9 @@ ')(?P[a-f0-9]+)' => 'DiffusionCommitController', + '/source/(?P[^/.]+)(?P\.git)?' + => $repository_routes, + '/diffusion/' => array( $this->getQueryRoutePattern() => 'DiffusionRepositoryListController', @@ -63,57 +113,8 @@ '(?:query/(?P[^/]+)/)?' => 'DiffusionPushLogListController', 'view/(?P\d+)/' => 'DiffusionPushEventViewController', ), - '(?:'. - '(?P[A-Z]+)'. - '|'. - '(?P[1-9]\d*)'. - ')/' => array( - '' => 'DiffusionRepositoryController', - - 'repository/(?P.*)' => 'DiffusionRepositoryController', - 'change/(?P.*)' => 'DiffusionChangeController', - 'history/(?P.*)' => 'DiffusionHistoryController', - 'browse/(?P.*)' => 'DiffusionBrowseController', - 'lastmodified/(?P.*)' => 'DiffusionLastModifiedController', - 'diff/' => 'DiffusionDiffController', - 'tags/(?P.*)' => 'DiffusionTagListController', - 'branches/(?P.*)' => 'DiffusionBranchTableController', - 'refs/(?P.*)' => 'DiffusionRefTableController', - 'lint/(?P.*)' => 'DiffusionLintController', - 'commit/(?P[a-z0-9]+)/branches/' - => 'DiffusionCommitBranchesController', - 'commit/(?P[a-z0-9]+)/tags/' - => 'DiffusionCommitTagsController', - 'commit/(?P[a-z0-9]+)/edit/' - => 'DiffusionCommitEditController', - 'manage/(?:(?P[^/]+)/)?' - => 'DiffusionRepositoryManagePanelsController', - 'uri/' => array( - 'view/(?P[0-9]\d*)/' => 'DiffusionRepositoryURIViewController', - 'disable/(?P[0-9]\d*)/' - => 'DiffusionRepositoryURIDisableController', - $this->getEditRoutePattern('edit/') - => 'DiffusionRepositoryURIEditController', - 'credential/(?P[0-9]\d*)/(?Pedit|remove)/' - => 'DiffusionRepositoryURICredentialController', - ), - 'edit/' => array( - 'activate/' => 'DiffusionRepositoryEditActivateController', - 'dangerous/' => 'DiffusionRepositoryEditDangerousController', - 'delete/' => 'DiffusionRepositoryEditDeleteController', - 'update/' => 'DiffusionRepositoryEditUpdateController', - 'testautomation/' => 'DiffusionRepositoryTestAutomationController', - ), - 'pathtree/(?P.*)' => 'DiffusionPathTreeController', - ), - - // NOTE: This must come after the rule above; it just gives us a - // catch-all for serving repositories over HTTP. We must accept - // requests without the trailing "/" because SVN commands don't - // necessarily include it. - '(?:(?P[A-Z]+)|(?P[1-9]\d*))'. - '(?:/.*)?' - => 'DiffusionRepositoryDefaultController', + '(?P[A-Z]+)' => $repository_routes, + '(?P[1-9]\d*)' => $repository_routes, 'inline/' => array( 'edit/(?P[^/]+)/' => 'DiffusionInlineCommentController', diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -90,6 +90,11 @@ protected function getRepositoryIdentifierFromRequest( AphrontRequest $request) { + $short_name = $request->getURIData('repositoryShortName'); + if (strlen($short_name)) { + return $short_name; + } + $identifier = $request->getURIData('repositoryCallsign'); if (strlen($identifier)) { return $identifier; diff --git a/src/applications/diffusion/controller/DiffusionLastModifiedController.php b/src/applications/diffusion/controller/DiffusionLastModifiedController.php --- a/src/applications/diffusion/controller/DiffusionLastModifiedController.php +++ b/src/applications/diffusion/controller/DiffusionLastModifiedController.php @@ -16,6 +16,7 @@ $drequest = $this->getDiffusionRequest(); $paths = $request->getStr('paths'); + try { $paths = phutil_json_decode($paths); } catch (PhutilJSONParserException $ex) { diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -88,6 +88,13 @@ } } + // If the request was for a path like "/source/libphutil.git" but the + // repository is not a Git repository, reject the request. + $type_git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT; + if ($request->getURIData('dotgit') && ($vcs !== $type_git)) { + return null; + } + return $vcs; } @@ -607,7 +614,9 @@ $request = $this->getRequest(); $request_path = $request->getRequestURI()->getPath(); - $info = PhabricatorRepository::parseRepositoryServicePath($request_path); + $info = PhabricatorRepository::parseRepositoryServicePath( + $request_path, + $repository->getVersionControlSystem()); $base_path = $info['path']; // For Git repositories, strip an optional directory component if it diff --git a/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php b/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php --- a/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php +++ b/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php @@ -15,7 +15,9 @@ } protected function identifyRepository() { - return $this->loadRepositoryWithPath($this->getLFSPathArgument()); + return $this->loadRepositoryWithPath( + $this->getLFSPathArgument(), + PhabricatorRepositoryType::REPOSITORY_TYPE_GIT); } private function getLFSPathArgument() { diff --git a/src/applications/diffusion/ssh/DiffusionGitSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitSSHWorkflow.php --- a/src/applications/diffusion/ssh/DiffusionGitSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionGitSSHWorkflow.php @@ -17,7 +17,9 @@ protected function identifyRepository() { $args = $this->getArgs(); $path = head($args->getArg('dir')); - return $this->loadRepositoryWithPath($path); + return $this->loadRepositoryWithPath( + $path, + PhabricatorRepositoryType::REPOSITORY_TYPE_GIT); } protected function waitForGitClient() { diff --git a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php --- a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php @@ -27,7 +27,9 @@ protected function identifyRepository() { $args = $this->getArgs(); $path = $args->getArg('repository'); - return $this->loadRepositoryWithPath($path); + return $this->loadRepositoryWithPath( + $path, + PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL); } protected function executeRepositoryOperations() { diff --git a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php --- a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php @@ -161,18 +161,19 @@ } } - protected function loadRepositoryWithPath($path) { + protected function loadRepositoryWithPath($path, $vcs) { $viewer = $this->getUser(); - $info = PhabricatorRepository::parseRepositoryServicePath($path); + $info = PhabricatorRepository::parseRepositoryServicePath($path, $vcs); if ($info === null) { throw new Exception( pht( - 'Unrecognized repository path "%s". Expected a path like "%s" '. - 'or "%s".', + 'Unrecognized repository path "%s". Expected a path like "%s", '. + '"%s", or "%s".', $path, '/diffusion/X/', - '/diffusion/123/')); + '/diffusion/123/', + '/source/thaumaturgy.git')); } $identifier = $info['identifier']; diff --git a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php --- a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php @@ -117,7 +117,9 @@ $uri = $struct[2]['value']; $path = $this->getPathFromSubversionURI($uri); - return $this->loadRepositoryWithPath($path); + return $this->loadRepositoryWithPath( + $path, + PhabricatorRepositoryType::REPOSITORY_TYPE_SVN); } } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -563,6 +563,11 @@ } public function getURI() { + $short_name = $this->getRepositorySlug(); + if (strlen($short_name)) { + return "/source/{$short_name}/"; + } + $callsign = $this->getCallsign(); if (strlen($callsign)) { return "/diffusion/{$callsign}/"; @@ -573,7 +578,7 @@ } public function getPathURI($path) { - return $this->getURI().$path; + return $this->getURI().ltrim($path, '/'); } public function getCommitURI($identifier) { @@ -586,14 +591,22 @@ return "/R{$id}:{$identifier}"; } - public static function parseRepositoryServicePath($request_path) { + public static function parseRepositoryServicePath($request_path, $vcs) { + // NOTE: In Mercurial over SSH, the path will begin without a leading "/", // so we're matching it optionally. + if ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT) { + $maybe_git = '(?:\\.git)?'; + } else { + $maybe_git = null; + } + $patterns = array( '(^'. - '(?P/?diffusion/(?P[A-Z]+|[0-9]\d*))'. - '(?P(?:/.*)?)'. + '(?P/?(?:diffusion|source)/(?P[^/.]+))'. + $maybe_git. + '(?P(?:/|.*)?)'. '\z)', ); @@ -624,28 +637,15 @@ public function getCanonicalPath($request_path) { $standard_pattern = '(^'. - '(?P/diffusion/)'. + '(?P/(?:diffusion|source)/)'. '(?P[^/]+)'. '(?P(?:/.*)?)'. '\z)'; $matches = null; if (preg_match($standard_pattern, $request_path, $matches)) { - $prefix = $matches['prefix']; - - $callsign = $this->getCallsign(); - if ($callsign) { - $identifier = $callsign; - } else { - $identifier = $this->getID(); - } - $suffix = $matches['suffix']; - if (!strlen($suffix)) { - $suffix = '/'; - } - - return $prefix.$identifier.$suffix; + return $this->getPathURI($suffix); } $commit_pattern = @@ -724,18 +724,6 @@ return $this->getCommitURI($commit); } - - $identifier = $this->getID(); - - $callsign = $this->getCallsign(); - if ($callsign !== null) { - $identifier = $callsign; - } - - if (strlen($identifier)) { - $identifier = phutil_escape_uri_path_component($identifier); - } - if (strlen($path)) { $path = ltrim($path, '/'); $path = str_replace(array(';', '$'), array(';;', '$$'), $path); @@ -766,13 +754,13 @@ case 'lint': case 'pathtree': case 'refs': - $uri = "/diffusion/{$identifier}/{$action}/{$path}{$commit}{$line}"; + $uri = $this->getPathURI("/{$action}/{$path}{$commit}{$line}"); break; case 'branch': if (strlen($path)) { - $uri = "/diffusion/{$identifier}/repository/{$path}"; + $uri = $this->getPathURI("/repository/{$path}"); } else { - $uri = "/diffusion/{$identifier}/"; + $uri = $this->getPathURI(); } break; case 'external': @@ -2108,9 +2096,6 @@ $has_callsign = ($this->getCallsign() !== null); $has_shortname = ($this->getRepositorySlug() !== null); - // TODO: For now, never enable these because they don't work yet. - $has_shortname = false; - $identifier_map = array( PhabricatorRepositoryURI::BUILTIN_IDENTIFIER_CALLSIGN => $has_callsign, PhabricatorRepositoryURI::BUILTIN_IDENTIFIER_SHORTNAME => $has_shortname, diff --git a/src/applications/repository/storage/PhabricatorRepositoryURI.php b/src/applications/repository/storage/PhabricatorRepositoryURI.php --- a/src/applications/repository/storage/PhabricatorRepositoryURI.php +++ b/src/applications/repository/storage/PhabricatorRepositoryURI.php @@ -125,8 +125,8 @@ $other_uris = $repository->getURIs(); $identifier_value = array( - self::BUILTIN_IDENTIFIER_CALLSIGN => 3, - self::BUILTIN_IDENTIFIER_SHORTNAME => 2, + self::BUILTIN_IDENTIFIER_SHORTNAME => 3, + self::BUILTIN_IDENTIFIER_CALLSIGN => 2, self::BUILTIN_IDENTIFIER_ID => 1, );