revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-24 06:20:56 +01:00
Code Issues Releases Wiki Activity

Stop trasforming "scp-style" URIs into normal URIs in Git

Browse source 
Summary:
See D3033, T1529. We currently transform "scp-style" `user@host:path` URIs into normal `ssh://user@host/path` URIs. This is undesirable for two reasons:

  - The paths aren't always equivalent. They are for GitHub, which is why I missed this originally, but in the general case the ":path" is resolved relatively and the "/path" is resolved absolutely. So this transformation can break things.
  - It confuses users, who do not think of "git@host:path" URIs as SSH URIs even though the SSH protocol is implied.

So stop using them, and just use the "git@host:path" URIs instead. This is a bit messy since we have some validation built up on top of URIs. Hopefully we can get rid of more of this in the future as we simplify repository management.

Test Plan: Unit tests cover this stuff pretty well. Made a new git repository with a "git@host:path" style URI and did pull/discover on it, verified the right URI was used.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1529

Differential Revision: https://secure.phabricator.com/D3036
This commit is contained in:
epriestley 2012-07-23 12:05:03 -07:00
parent 81fb6aade2
commit 839f3df9c2
3 changed files with 46 additions and 81 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
View file

@ -724,11 +724,8 @@ final class PhabricatorRepositoryPullLocalDaemon
* @task git
*/
public static function executeGitVerifySameOrigin($remote, $expect, $where) {
$remote_uri = PhabricatorRepository::newPhutilURIFromGitURI($remote);
$expect_uri = PhabricatorRepository::newPhutilURIFromGitURI($expect);
$remote_path = $remote_uri->getPath();
$expect_path = $expect_uri->getPath();
$remote_path = self::getPathFromGitURI($remote);
$expect_path = self::getPathFromGitURI($expect);
$remote_match = self::executeGitNormalizePath($remote_path);
$expect_match = self::executeGitNormalizePath($expect_path);
@ -743,14 +740,28 @@ final class PhabricatorRepositoryPullLocalDaemon
}
}
private static function getPathFromGitURI($raw_uri) {
$uri = new PhutilURI($raw_uri);
if ($uri->getProtocol()) {
return $uri->getPath();
}
$uri = new PhutilGitURI($raw_uri);
if ($uri->getDomain()) {
return $uri->getPath();
}
return $raw_uri;
}
/**
* @task git
*/
private static function executeGitNormalizePath($path) {
// Strip away trailing "/" and ".git", so similar paths correctly match.
// Strip away "/" and ".git", so similar paths correctly match.
$path = rtrim($path, '/');
$path = trim($path, '/');
$path = preg_replace('/\.git$/', '', $path);
return $path;
}

67
src/applications/repository/storage/PhabricatorRepository.php
View file

@ -70,99 +70,88 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO {
));
}
public static function newPhutilURIFromGitURI($raw_uri) {
$uri = new PhutilURI($raw_uri);
if (!$uri->getProtocol()) {
if (strpos($raw_uri, '/') === 0) {
// If the URI starts with a '/', it's an implicit file:// URI on the
// local disk.
$uri = new PhutilURI('file://'.$raw_uri);
} else if (strpos($raw_uri, ':') !== false) {
// If there's no protocol (git implicit SSH) but the URI has a colon,
// it's a git implicit SSH URI. Reformat the URI to be a normal URI.
// These git URIs look like "user@domain.com:path" instead of
// "ssh://user@domain/path".
list($domain, $path) = explode(':', $raw_uri, 2);
$uri = new PhutilURI('ssh://'.$domain.'/'.$path);
} else {
throw new Exception("The Git URI '{$raw_uri}' could not be parsed.");
}
}
return $uri;
}
public function getRemoteURI() {
$raw_uri = $this->getDetail('remote-uri');
if (!$raw_uri) {
return null;
}
$vcs = $this->getVersionControlSystem();
$is_git = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT);
if ($is_git) {
$uri = self::newPhutilURIFromGitURI($raw_uri);
} else {
$uri = new PhutilURI($raw_uri);
if (strpos($raw_uri, '/') === 0) {
// If the URI starts with a '/', it's an implicit file:// URI on the
// local disk.
$uri = new PhutilURI('file://'.$raw_uri);
return (string)$uri;
}
if ($this->isSSHProtocol($uri->getProtocol())) {
$uri = new PhutilURI($raw_uri);
if ($uri->getProtocol()) {
if ($this->isSSHProtocol($uri->getProtocol())) {
if ($this->getSSHLogin()) {
$uri->setUser($this->getSSHLogin());
}
}
return (string)$uri;
}
$uri = new PhutilGitURI($raw_uri);
if ($uri->getDomain()) {
if ($this->getSSHLogin()) {
$uri->setUser($this->getSSHLogin());
}
return (string)$uri;
}
return (string)$uri;
throw new Exception(
"Repository remote URI '{$raw_uri}' could not be parsed!");
}
public function getLocalPath() {
return $this->getDetail('local-path');
}
public function execRemoteCommand($pattern /*, $arg, ... */) {
public function execRemoteCommand($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatRemoteCommand($args);
return call_user_func_array('exec_manual', $args);
}
public function execxRemoteCommand($pattern /*, $arg, ... */) {
public function execxRemoteCommand($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatRemoteCommand($args);
return call_user_func_array('execx', $args);
}
public function getRemoteCommandFuture($pattern /*, $arg, ... */) {
public function getRemoteCommandFuture($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatRemoteCommand($args);
return newv('ExecFuture', $args);
}
public function passthruRemoteCommand($pattern /*, $arg, ... */) {
public function passthruRemoteCommand($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatRemoteCommand($args);
return call_user_func_array('phutil_passthru', $args);
}
public function execLocalCommand($pattern /*, $arg, ... */) {
public function execLocalCommand($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatLocalCommand($args);
return call_user_func_array('exec_manual', $args);
}
public function execxLocalCommand($pattern /*, $arg, ... */) {
public function execxLocalCommand($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatLocalCommand($args);
return call_user_func_array('execx', $args);
}
public function getLocalCommandFuture($pattern /*, $arg, ... */) {
public function getLocalCommandFuture($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatLocalCommand($args);
return newv('ExecFuture', $args);
}
public function passthruLocalCommand($pattern /*, $arg, ... */) {
public function passthruLocalCommand($pattern /* , $arg, ... */) {
$args = func_get_args();
$args = $this->formatLocalCommand($args);
return call_user_func_array('phutil_passthru', $args);

35
src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php
View file

@ -19,41 +19,6 @@
final class PhabricatorRepositoryTestCase
extends PhabricatorTestCase {
public function testParseGitURI() {
static $map = array(
'ssh://user@domain.com/path.git' => 'ssh://user@domain.com/path.git',
'user@domain.com:path.git' => 'ssh://user@domain.com/path.git',
'/path/to/local/repo.git' => 'file:///path/to/local/repo.git',
);
foreach ($map as $raw => $expect) {
$uri = PhabricatorRepository::newPhutilURIFromGitURI($raw);
$this->assertEqual(
$expect,
(string)$uri,
"Normalized Git URI '{$raw}'");
}
}
public function testParseBadGitURI() {
$junk = array(
'herp derp moon balloon',
);
foreach ($junk as $garbage) {
$ex = null;
try {
$uri = PhabricatorRepository::newPhutilURIFromGitURI($garbage);
} catch (Exception $caught) {
$ex = $caught;
}
$this->assertEqual(
true,
(bool)$ex,
'Expect exception when parsing garbage.');
}
}
public function testBranchFilter() {
$git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;