Page MenuHomePhabricator
Files F14093174

D7981.diff
No OneTemporary
Actions

D7981.diff
View Options

Index: src/__phutil_library_map__.php
===================================================================
--- src/__phutil_library_map__.php
+++ src/__phutil_library_map__.php
@@ -1881,6 +1881,8 @@
'PhabricatorRepositoryTransaction' => 'applications/repository/storage/PhabricatorRepositoryTransaction.php',
'PhabricatorRepositoryTransactionQuery' => 'applications/repository/query/PhabricatorRepositoryTransactionQuery.php',
'PhabricatorRepositoryType' => 'applications/repository/constants/PhabricatorRepositoryType.php',
+ 'PhabricatorRepositoryURINormalizer' => 'applications/repository/data/PhabricatorRepositoryURINormalizer.php',
+ 'PhabricatorRepositoryURINormalizerTestCase' => 'applications/repository/data/__tests__/PhabricatorRepositoryURINormalizerTestCase.php',
'PhabricatorRepositoryURITestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php',
'PhabricatorRepositoryVCSPassword' => 'applications/repository/storage/PhabricatorRepositoryVCSPassword.php',
'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
@@ -4551,6 +4553,8 @@
'PhabricatorRepositoryTestCase' => 'PhabricatorTestCase',
'PhabricatorRepositoryTransaction' => 'PhabricatorApplicationTransaction',
'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+ 'PhabricatorRepositoryURINormalizer' => 'Phobject',
+ 'PhabricatorRepositoryURINormalizerTestCase' => 'PhabricatorTestCase',
'PhabricatorRepositoryURITestCase' => 'PhabricatorTestCase',
'PhabricatorRepositoryVCSPassword' => 'PhabricatorRepositoryDAO',
'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine',
Index: src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
===================================================================
--- src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
+++ src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
@@ -719,7 +719,17 @@
$valid = false;
$exists = false;
} else {
- $valid = self::isSameGitOrigin($remote_uri, $expect_remote);
+ $normal_type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;
+
+ $remote_normal = id(new PhabricatorRepositoryURINormalizer(
+ $normal_type_git,
+ $remote_uri))->getNormalizedPath();
+
+ $expect_normal = id(new PhabricatorRepositoryURINormalizer(
+ $normal_type_git,
+ $expect_remote))->getNormalizedPath();
+
+ $valid = ($remote_normal == $expect_normal);
$exists = true;
}
@@ -768,48 +778,6 @@
}
}
-
-
- /**
- * @task git
- */
- public static function isSameGitOrigin($remote, $expect) {
- $remote_path = self::getPathFromGitURI($remote);
- $expect_path = self::getPathFromGitURI($expect);
-
- $remote_match = self::executeGitNormalizePath($remote_path);
- $expect_match = self::executeGitNormalizePath($expect_path);
-
- return ($remote_match == $expect_match);
- }
-
- 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 "/" and ".git", so similar paths correctly match.
-
- $path = trim($path, '/');
- $path = preg_replace('/\.git$/', '', $path);
- return $path;
- }
-
-
private function pushToMirrors(PhabricatorRepository $repository) {
if (!$repository->canMirror()) {
return;
Index: src/applications/repository/data/PhabricatorRepositoryURINormalizer.php
===================================================================
--- /dev/null
+++ src/applications/repository/data/PhabricatorRepositoryURINormalizer.php
@@ -0,0 +1,98 @@
+<?php
+
+/**
+ * Normalize repository URIs. For example, these URIs are generally equivalent
+ * and all point at the same repository:
+ *
+ * ssh://user@host/repo
+ * ssh://user@host/repo/
+ * ssh://user@host:22/repo
+ * user@host:/repo
+ * ssh://user@host/repo.git
+ *
+ * This class can be used to normalize URIs like this, in order to detect
+ * alternate spellings of the same repository URI. In particular, the
+ * @{method:getNormalizedPath} method will return:
+ *
+ * repo
+ *
+ * ...for all of these URIs. Generally, usage looks like this:
+ *
+ * $norm_a = new PhabricatorRepositoryURINormalizer($type, $uri_a);
+ * $norm_b = new PhabricatorRepositoryURINormalizer($type, $uri_b);
+ *
+ * if ($norm_a->getNormalizedPath() == $norm_b->getNormalizedPath()) {
+ * // URIs appear to point at the same repository.
+ * } else {
+ * // URIs are very unlikely to be the same repository.
+ * }
+ *
+ * Because a repository can be hosted at arbitrarly many arbitrary URIs, there
+ * is no way to completely prevent false negatives by only examining URIs
+ * (that is, repositories with totally different URIs could really be the same).
+ * However, normalization is relatively agressive and false negatives should
+ * be rare: if normalization says two URIs are different repositories, they
+ * probably are.
+ *
+ * @task normal Normalizing URIs
+ */
+final class PhabricatorRepositoryURINormalizer extends Phobject {
+
+ const TYPE_GIT = 'git';
+ private $type;
+ private $uri;
+
+ public function __construct($type, $uri) {
+ switch ($type) {
+ case self::TYPE_GIT:
+ break;
+ default:
+ throw new Exception(pht('Unknown URI type "%s"!'));
+ }
+
+ $this->type = $type;
+ $this->uri = $uri;
+ }
+
+
+/* -( Normalizing URIs )--------------------------------------------------- */
+
+
+ /**
+ * @task normal
+ */
+ public function getPath() {
+ switch ($this->type) {
+ case self::TYPE_GIT:
+ $uri = new PhutilURI($this->uri);
+ if ($uri->getProtocol()) {
+ return $uri->getPath();
+ }
+
+ $uri = new PhutilGitURI($this->uri);
+ if ($uri->getDomain()) {
+ return $uri->getPath();
+ }
+
+ return $this->uri;
+ }
+ }
+
+
+ /**
+ * @task normal
+ */
+ public function getNormalizedPath() {
+ $path = $this->getPath();
+ $path = trim($path, '/');
+
+ switch ($this->type) {
+ case self::TYPE_GIT:
+ $path = preg_replace('/\.git$/', '', $path);
+ break;
+ }
+
+ return $path;
+ }
+
+}
Index: src/applications/repository/data/__tests__/PhabricatorRepositoryURINormalizerTestCase.php
===================================================================
--- /dev/null
+++ src/applications/repository/data/__tests__/PhabricatorRepositoryURINormalizerTestCase.php
@@ -0,0 +1,31 @@
+<?php
+
+final class PhabricatorRepositoryURINormalizerTestCase
+ extends PhabricatorTestCase {
+
+ public function testGitURINormalizer() {
+ $cases = array(
+ 'ssh://user@domain.com/path.git' => 'path',
+ 'https://user@domain.com/path.git' => 'path',
+ 'git@domain.com:path.git' => 'path',
+ 'ssh://user@gitserv002.com/path.git' => 'path',
+ 'ssh://htaft@domain.com/path.git' => 'path',
+ 'ssh://user@domain.com/bananas.git' => 'bananas',
+ 'git@domain.com:bananas.git' => 'bananas',
+ 'user@domain.com:path/repo' => 'path/repo',
+ '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',
+ );
+
+ $type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;
+
+ foreach ($cases as $input => $expect) {
+ $normal = new PhabricatorRepositoryURINormalizer($type_git, $input);
+ $this->assertEqual(
+ $expect,
+ $normal->getNormalizedPath(),
+ pht('Normalized path for "%s".', $input));
+ }
+ }
+}
Index: src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php
===================================================================
--- src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php
+++ src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php
@@ -24,88 +24,4 @@
$this->assertEqual(array(), $refs);
}
- public function testExecuteGitVerifySameOrigin() {
- $cases = array(
- array(
- 'ssh://user@domain.com/path.git',
- 'ssh://user@domain.com/path.git',
- true,
- 'Identical paths should pass.',
- ),
- array(
- 'ssh://user@domain.com/path.git',
- 'https://user@domain.com/path.git',
- true,
- 'Protocol changes should pass.',
- ),
- array(
- 'ssh://user@domain.com/path.git',
- 'git@domain.com:path.git',
- true,
- 'Git implicit SSH should pass.',
- ),
- array(
- 'ssh://user@gitserv001.com/path.git',
- 'ssh://user@gitserv002.com/path.git',
- true,
- 'Domain changes should pass.',
- ),
- array(
- 'ssh://alincoln@domain.com/path.git',
- 'ssh://htaft@domain.com/path.git',
- true,
- 'User/auth changes should pass.',
- ),
- array(
- 'ssh://user@domain.com/apples.git',
- 'ssh://user@domain.com/bananas.git',
- false,
- 'Path changes should fail.',
- ),
- array(
- 'ssh://user@domain.com/apples.git',
- 'git@domain.com:bananas.git',
- false,
- 'Git implicit SSH path changes should fail.',
- ),
- array(
- 'user@domain.com:path/repo.git',
- 'user@domain.com:path/repo',
- true,
- 'Optional .git extension should not prevent matches.',
- ),
- array(
- 'user@domain.com:path/repo/',
- 'user@domain.com:path/repo',
- true,
- 'Optional trailing slash should not prevent matches.',
- ),
- array(
- 'file:///path/to/local/repo.git',
- 'file:///path/to/local/repo.git',
- true,
- 'file:// protocol should be supported.',
- ),
- array(
- '/path/to/local/repo.git',
- 'file:///path/to/local/repo.git',
- true,
- 'Implicit file:// protocol should be recognized.',
- ),
- );
-
- foreach ($cases as $case) {
- list($remote, $config, $expect, $message) = $case;
-
- $this->assertEqual(
- $expect,
- PhabricatorRepositoryPullLocalDaemon::isSameGitOrigin(
- $remote,
- $config,
- '(a test case)'),
- "Verification that '{$remote}' and '{$config}' are the same origin ".
- "had a different outcome than expected: {$message}");
- }
- }
-
}

File Metadata

Mime Type
text/plain
Expires
Tue, Nov 26, 9:57 AM (7 h, 59 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6788951
Default Alt Text
D7981.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility