mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-23 07:12:41 +01:00
Fix a "user@domain:path" protocol handling bug
Summary: In D3063, we stopped converting "user@domain:path" git-style URIs, but this broke the SSH-detection code and I missed that in my test plan because my test case uses natural SSH keys so the omission of SSH flags didn't cause failures. This code is a bit of a mess anyway. Consolidate and refactor it to be a bit simpler, and add test coverage. Test Plan: Ran unit tests. Ran "test_connection.php" in SSH and non-SSH modes, verified SSH modes generated appropriate ssh-agent commands around the git remote commands. Reviewers: vrana, btrahan, tberman Reviewed By: btrahan CC: aran Maniphest Tasks: T1529 Differential Revision: https://secure.phabricator.com/D3093
This commit is contained in:
epriestley
parent
f1eb92a126
commit
d9296638cd
2 changed files with 183 additions and 89 deletions
|
|
@ -16,6 +16,9 @@
|
||||||
* limitations under the License.
|
* limitations under the License.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @task uri Repository URI Management
|
||||||
|
*/
|
||||||
final class PhabricatorRepository extends PhabricatorRepositoryDAO {
|
final class PhabricatorRepository extends PhabricatorRepositoryDAO {
|
||||||
|
|
||||||
const TABLE_PATH = 'repository_path';
|
const TABLE_PATH = 'repository_path';
|
||||||
|
|
@ -70,41 +73,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO {
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getRemoteURI() {
|
|
||||||
$raw_uri = $this->getDetail('remote-uri');
|
|
||||||
if (!$raw_uri) {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
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;
|
|
||||||
}
|
|
||||||
|
|
||||||
$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;
|
|
||||||
}
|
|
||||||
|
|
||||||
throw new Exception(
|
|
||||||
"Repository remote URI '{$raw_uri}' could not be parsed!");
|
|
||||||
}
|
|
||||||
|
|
||||||
public function getLocalPath() {
|
public function getLocalPath() {
|
||||||
return $this->getDetail('local-path');
|
return $this->getDetail('local-path');
|
||||||
}
|
}
|
||||||
|
|
@ -308,64 +276,10 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO {
|
||||||
return (string)$this->sshKeyfile;
|
return (string)$this->sshKeyfile;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function shouldUseSSH() {
|
|
||||||
$uri = new PhutilURI($this->getRemoteURI());
|
|
||||||
$protocol = $uri->getProtocol();
|
|
||||||
if ($this->isSSHProtocol($protocol)) {
|
|
||||||
return (bool)$this->getSSHKeyfile();
|
|
||||||
} else {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public function shouldUseHTTP() {
|
|
||||||
$uri = new PhutilURI($this->getRemoteURI());
|
|
||||||
$protocol = $uri->getProtocol();
|
|
||||||
if ($this->isHTTPProtocol($protocol)) {
|
|
||||||
return (bool)$this->getDetail('http-login');
|
|
||||||
} else {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public function shouldUseSVNProtocol() {
|
|
||||||
$uri = new PhutilURI($this->getRemoteURI());
|
|
||||||
$protocol = $uri->getProtocol();
|
|
||||||
if ($this->isSVNProtocol($protocol)) {
|
|
||||||
return (bool)$this->getDetail('http-login');
|
|
||||||
} else {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
public function getPublicRemoteURI() {
|
|
||||||
$uri = new PhutilURI($this->getRemoteURI());
|
|
||||||
|
|
||||||
// Make sure we don't leak anything if this repo is using HTTP Basic Auth
|
|
||||||
// with the credentials in the URI or something zany like that.
|
|
||||||
$uri->setUser(null);
|
|
||||||
$uri->setPass(null);
|
|
||||||
|
|
||||||
return $uri;
|
|
||||||
}
|
|
||||||
|
|
||||||
public function getURI() {
|
public function getURI() {
|
||||||
return '/diffusion/'.$this->getCallsign().'/';
|
return '/diffusion/'.$this->getCallsign().'/';
|
||||||
}
|
}
|
||||||
|
|
||||||
private function isSSHProtocol($protocol) {
|
|
||||||
return ($protocol == 'ssh' || $protocol == 'svn+ssh');
|
|
||||||
}
|
|
||||||
|
|
||||||
private function isHTTPProtocol($protocol) {
|
|
||||||
return ($protocol == 'http' || $protocol == 'https');
|
|
||||||
}
|
|
||||||
|
|
||||||
private function isSVNProtocol($protocol) {
|
|
||||||
return ($protocol == 'svn');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function isTracked() {
|
public function isTracked() {
|
||||||
return $this->getDetail('tracking-enabled', false);
|
return $this->getDetail('tracking-enabled', false);
|
||||||
}
|
}
|
||||||
|
|
@ -476,4 +390,160 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO {
|
||||||
return $repositories;
|
return $repositories;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* -( Repository URI Management )------------------------------------------ */
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get the remote URI for this repository.
|
||||||
|
*
|
||||||
|
* @return string
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
public function getRemoteURI() {
|
||||||
|
return (string)$this->getRemoteURIObject();
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get the remote URI for this repository, without authentication information.
|
||||||
|
*
|
||||||
|
* @return string Repository URI.
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
public function getPublicRemoteURI() {
|
||||||
|
$uri = $this->getURIObject();
|
||||||
|
|
||||||
|
// Make sure we don't leak anything if this repo is using HTTP Basic Auth
|
||||||
|
// with the credentials in the URI or something zany like that.
|
||||||
|
|
||||||
|
if ($uri instanceof PhutilGitURI) {
|
||||||
|
$uri->setUser(null);
|
||||||
|
} else {
|
||||||
|
$uri->setUser(null);
|
||||||
|
$uri->setPass(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
return (string)$uri;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get the protocol for the repository's remote.
|
||||||
|
*
|
||||||
|
* @return string Protocol, like "ssh" or "git".
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
public function getRemoteProtocol() {
|
||||||
|
$uri = $this->getRemoteURIObject();
|
||||||
|
|
||||||
|
if ($uri instanceof PhutilGitURI) {
|
||||||
|
return 'ssh';
|
||||||
|
} else {
|
||||||
|
return $uri->getProtocol();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get a parsed object representation of the repository's remote URI. This
|
||||||
|
* may be a normal URI (returned as a @{class:PhutilURI}) or a git URI
|
||||||
|
* (returned as a @{class:PhutilGitURI}).
|
||||||
|
*
|
||||||
|
* @return wild A @{class:PhutilURI} or @{class:PhutilGitURI}.
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
private function getRemoteURIObject() {
|
||||||
|
$raw_uri = $this->getDetail('remote-uri');
|
||||||
|
if (!$raw_uri) {
|
||||||
|
return new PhutilURI('');
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!strncmp($raw_uri, '/', 1)) {
|
||||||
|
return new PhutilURI('file://'.$raw_uri);
|
||||||
|
}
|
||||||
|
|
||||||
|
$uri = new PhutilURI($raw_uri);
|
||||||
|
if ($uri->getProtocol()) {
|
||||||
|
if ($this->isSSHProtocol($uri->getProtocol())) {
|
||||||
|
if ($this->getSSHLogin()) {
|
||||||
|
$uri->setUser($this->getSSHLogin());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return $uri;
|
||||||
|
}
|
||||||
|
|
||||||
|
$uri = new PhutilGitURI($raw_uri);
|
||||||
|
if ($uri->getDomain()) {
|
||||||
|
if ($this->getSSHLogin()) {
|
||||||
|
$uri->setUser($this->getSSHLogin());
|
||||||
|
}
|
||||||
|
return $uri;
|
||||||
|
}
|
||||||
|
|
||||||
|
throw new Exception("Remote URI '{$raw_uri}' could not be parsed!");
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Determine if we should connect to the remote using SSH flags and
|
||||||
|
* credentials.
|
||||||
|
*
|
||||||
|
* @return bool True to use the SSH protocol.
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
private function shouldUseSSH() {
|
||||||
|
$protocol = $this->getRemoteProtocol();
|
||||||
|
if ($this->isSSHProtocol($protocol)) {
|
||||||
|
return (bool)$this->getSSHKeyfile();
|
||||||
|
} else {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Determine if we should connect to the remote using HTTP flags and
|
||||||
|
* credentials.
|
||||||
|
*
|
||||||
|
* @return bool True to use the HTTP protocol.
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
private function shouldUseHTTP() {
|
||||||
|
$protocol = $this->getRemoteProtocol();
|
||||||
|
if ($protocol == 'http' || $protocol == 'https') {
|
||||||
|
return (bool)$this->getDetail('http-login');
|
||||||
|
} else {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Determine if we should connect to the remote using SVN flags and
|
||||||
|
* credentials.
|
||||||
|
*
|
||||||
|
* @return bool True to use the SVN protocol.
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
private function shouldUseSVNProtocol() {
|
||||||
|
$protocol = $this->getRemoteProtocol();
|
||||||
|
if ($protocol == 'svn') {
|
||||||
|
return (bool)$this->getDetail('http-login');
|
||||||
|
} else {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Determine if a protocol is SSH or SSH-like.
|
||||||
|
*
|
||||||
|
* @param string A protocol string, like "http" or "ssh".
|
||||||
|
* @return bool True if the protocol is SSH-like.
|
||||||
|
* @task uri
|
||||||
|
*/
|
||||||
|
private function isSSHProtocol($protocol) {
|
||||||
|
return ($protocol == 'ssh' || $protocol == 'svn+ssh');
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -19,6 +19,30 @@
|
||||||
final class PhabricatorRepositoryTestCase
|
final class PhabricatorRepositoryTestCase
|
||||||
extends PhabricatorTestCase {
|
extends PhabricatorTestCase {
|
||||||
|
|
||||||
|
public function testRepositoryURIProtocols() {
|
||||||
|
$tests = array(
|
||||||
|
'/path/to/repo' => 'file',
|
||||||
|
'file:///path/to/repo' => 'file',
|
||||||
|
'ssh://user@domain.com/path' => 'ssh',
|
||||||
|
'git@example.com:path' => 'ssh',
|
||||||
|
'git://git@example.com/path' => 'git',
|
||||||
|
'svn+ssh://example.com/path' => 'svn+ssh',
|
||||||
|
'https://example.com/repo/' => 'https',
|
||||||
|
'http://example.com/' => 'http',
|
||||||
|
'https://user@example.com/' => 'https',
|
||||||
|
);
|
||||||
|
|
||||||
|
foreach ($tests as $uri => $expect) {
|
||||||
|
$repository = new PhabricatorRepository();
|
||||||
|
$repository->setDetail('remote-uri', $uri);
|
||||||
|
|
||||||
|
$this->assertEqual(
|
||||||
|
$expect,
|
||||||
|
$repository->getRemoteProtocol(),
|
||||||
|
"Protocol for '{$uri}'.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public function testBranchFilter() {
|
public function testBranchFilter() {
|
||||||
$git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;
|
$git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue