Page MenuHomePhabricator

D11541.diff
No OneTemporary

D11541.diff

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -7,7 +7,7 @@
*/
return array(
'names' => array(
- 'core.pkg.css' => '04a24e98',
+ 'core.pkg.css' => '8815f87d',
'core.pkg.js' => 'efa12ecc',
'darkconsole.pkg.js' => '8ab24e01',
'differential.pkg.css' => '8af45893',
@@ -125,7 +125,7 @@
'rsrc/css/phui/phui-action-list.css' => '9ee9910a',
'rsrc/css/phui/phui-box.css' => '7b3a2eed',
'rsrc/css/phui/phui-button.css' => '008ba5e2',
- 'rsrc/css/phui/phui-crumbs-view.css' => '3e362700',
+ 'rsrc/css/phui/phui-crumbs-view.css' => '646a8830',
'rsrc/css/phui/phui-document.css' => 'bbeb1890',
'rsrc/css/phui/phui-feed-story.css' => 'c9f3a0b5',
'rsrc/css/phui/phui-fontkit.css' => '9c3d2dce',
@@ -137,7 +137,7 @@
'rsrc/css/phui/phui-info-panel.css' => '27ea50a1',
'rsrc/css/phui/phui-list.css' => '53deb25c',
'rsrc/css/phui/phui-object-box.css' => '0d47b3c8',
- 'rsrc/css/phui/phui-object-item-list-view.css' => '832c58fe',
+ 'rsrc/css/phui/phui-object-item-list-view.css' => '2686a80e',
'rsrc/css/phui/phui-pinboard-view.css' => '3dd4a269',
'rsrc/css/phui/phui-property-list-view.css' => '51480060',
'rsrc/css/phui/phui-remarkup-preview.css' => '19ad512b',
@@ -770,7 +770,7 @@
'phui-calendar-day-css' => 'de035c8a',
'phui-calendar-list-css' => 'c1d0ca59',
'phui-calendar-month-css' => 'a92e47d2',
- 'phui-crumbs-view-css' => '3e362700',
+ 'phui-crumbs-view-css' => '646a8830',
'phui-document-view-css' => 'bbeb1890',
'phui-feed-story-css' => 'c9f3a0b5',
'phui-font-icon-base-css' => '3dad2ae3',
@@ -783,7 +783,7 @@
'phui-info-panel-css' => '27ea50a1',
'phui-list-view-css' => '53deb25c',
'phui-object-box-css' => '0d47b3c8',
- 'phui-object-item-list-view-css' => '832c58fe',
+ 'phui-object-item-list-view-css' => '2686a80e',
'phui-pinboard-view-css' => '3dd4a269',
'phui-property-list-view-css' => '51480060',
'phui-remarkup-preview-css' => '19ad512b',
diff --git a/scripts/repository/commit_hook.php b/scripts/repository/commit_hook.php
--- a/scripts/repository/commit_hook.php
+++ b/scripts/repository/commit_hook.php
@@ -1,6 +1,15 @@
-#!/usr/bin/env php
+#!/usr/bin/env TERM=dumb php
<?php
+// NOTE: Note that we're specifying TERM=dumb above when invoking the PHP
+// interpreter. This suppresses an error which looks like this:
+//
+// No entry for terminal type "unknown";
+// using dumb terminal settings.
+//
+// This arises from somewhere in the PHP startup machinery if TERM is not
+// set to a recognized value.
+
// Commit hooks execute in an unusual context where the environment may be
// unavailable, particularly in SVN. The first parameter to this script is
// either a bare repository identifier ("X"), or a repository identifier
diff --git a/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
@@ -14,9 +14,7 @@
}
protected function executeRepositoryOperations() {
- $args = $this->getArgs();
- $path = head($args->getArg('dir'));
- $repository = $this->loadRepository($path);
+ $repository = $this->getRepository();
// This is a write, and must have write access.
$this->requireWriteAccess();
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
@@ -7,6 +7,12 @@
return parent::writeError($message."\n");
}
+ protected function identifyRepository() {
+ $args = $this->getArgs();
+ $path = head($args->getArg('dir'));
+ return $this->loadRepositoryWithPath($path);
+ }
+
protected function waitForGitClient() {
$io_channel = $this->getIOChannel();
diff --git a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
@@ -14,9 +14,7 @@
}
protected function executeRepositoryOperations() {
- $args = $this->getArgs();
- $path = head($args->getArg('dir'));
- $repository = $this->loadRepository($path);
+ $repository = $this->getRepository();
$command = csprintf('git-upload-pack -- %s', $repository->getLocalPath());
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
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
@@ -24,11 +24,14 @@
));
}
- protected function executeRepositoryOperations() {
+ protected function identifyRepository() {
$args = $this->getArgs();
$path = $args->getArg('repository');
- $repository = $this->loadRepository($path);
+ return $this->loadRepositoryWithPath($path);
+ }
+ protected function executeRepositoryOperations() {
+ $repository = $this->getRepository();
$args = $this->getArgs();
if (!$args->getArg('stdio')) {
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
@@ -8,11 +8,16 @@
public function getRepository() {
if (!$this->repository) {
- throw new Exception('Call loadRepository() before getRepository()!');
+ throw new Exception(pht('Repository is not available yet!'));
}
return $this->repository;
}
+ private function setRepository(PhabricatorRepository $repository) {
+ $this->repository = $repository;
+ return $this;
+ }
+
public function getArgs() {
return $this->args;
}
@@ -33,6 +38,10 @@
return $env;
}
+ /**
+ * Identify and load the affected repository.
+ */
+ abstract protected function identifyRepository();
abstract protected function executeRepositoryOperations();
protected function writeError($message) {
@@ -43,6 +52,12 @@
final public function execute(PhutilArgumentParser $args) {
$this->args = $args;
+ $repository = $this->identifyRepository();
+ $this->setRepository($repository);
+
+ // TODO: Here, we would make a proxying decision, had I implemented
+ // proxying yet.
+
try {
return $this->executeRepositoryOperations();
} catch (Exception $ex) {
@@ -51,7 +66,7 @@
}
}
- protected function loadRepository($path) {
+ protected function loadRepositoryWithPath($path) {
$viewer = $this->getUser();
$regex = '@^/?diffusion/(?P<callsign>[A-Z]+)(?:/|\z)@';
@@ -88,8 +103,6 @@
pht('This repository is not available over SSH.'));
}
- $this->repository = $repository;
-
return $repository;
}
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
@@ -20,6 +20,12 @@
private $internalBaseURI;
private $externalBaseURI;
+ private $peekBuffer;
+ private $command;
+
+ private function getCommand() {
+ return $this->command;
+ }
protected function didConstruct() {
$this->setName('svnserve');
@@ -32,7 +38,107 @@
));
}
+ protected function identifyRepository() {
+ // NOTE: In SVN, we need to read the first few protocol frames before we
+ // can determine which repository the user is trying to access. We're
+ // going to peek at the data on the wire to identify the repository.
+
+ $io_channel = $this->getIOChannel();
+
+ // Before the client will send us the first protocol frame, we need to send
+ // it a connection frame with server capabilities. To figure out the
+ // correct frame we're going to start `svnserve`, read the frame from it,
+ // send it to the client, then kill the subprocess.
+
+ // TODO: This is pretty inelegant and the protocol frame will change very
+ // rarely. We could cache it if we can find a reasonable way to dirty the
+ // cache.
+
+ $command = csprintf('svnserve -t');
+ $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
+ $future = new ExecFuture('%C', $command);
+ $exec_channel = new PhutilExecChannel($future);
+ $exec_protocol = new DiffusionSubversionWireProtocol();
+
+ while (true) {
+ PhutilChannel::waitForAny(array($exec_channel));
+ $exec_channel->update();
+
+ $exec_message = $exec_channel->read();
+ if ($exec_message !== null) {
+ $messages = $exec_protocol->writeData($exec_message);
+ if ($messages) {
+ $message = head($messages);
+ $raw = $message['raw'];
+
+ // Write the greeting frame to the client.
+ $io_channel->write($raw);
+
+ // Kill the subprocess.
+ $future->resolveKill();
+ break;
+ }
+ }
+
+ if (!$exec_channel->isOpenForReading()) {
+ throw new Exception(
+ pht(
+ 'svnserve subprocess exited before emitting a protocol frame.'));
+ }
+ }
+
+ $io_protocol = new DiffusionSubversionWireProtocol();
+ while (true) {
+ PhutilChannel::waitForAny(array($io_channel));
+ $io_channel->update();
+
+ $in_message = $io_channel->read();
+ if ($in_message !== null) {
+ $this->peekBuffer .= $in_message;
+ if (strlen($this->peekBuffer) > (1024 * 1024)) {
+ throw new Exception(
+ pht(
+ 'Client transmitted more than 1MB of data without transmitting '.
+ 'a recognizable protocol frame.'));
+ }
+
+ $messages = $io_protocol->writeData($in_message);
+ if ($messages) {
+ $message = head($messages);
+ $struct = $message['structure'];
+
+ // This is the:
+ //
+ // ( version ( cap1 ... ) url ... )
+ //
+ // The `url` allows us to identify the repository.
+
+ $uri = $struct[2]['value'];
+ $path = $this->getPathFromSubversionURI($uri);
+
+ return $this->loadRepositoryWithPath($path);
+ }
+ }
+
+ if (!$io_channel->isOpenForReading()) {
+ throw new Exception(
+ pht(
+ 'Client closed connection before sending a complete protocol '.
+ 'frame.'));
+ }
+
+ // If the client has disconnected, kill the subprocess and bail.
+ if (!$io_channel->isOpenForWriting()) {
+ throw new Exception(
+ pht(
+ 'Client closed connection before receiving response.'));
+ }
+ }
+ }
+
protected function executeRepositoryOperations() {
+ $repository = $this->getRepository();
+
$args = $this->getArgs();
if (!$args->getArg('tunnel')) {
throw new Exception('Expected `svnserve -t`!');
@@ -48,12 +154,15 @@
$this->inProtocol = new DiffusionSubversionWireProtocol();
$this->outProtocol = new DiffusionSubversionWireProtocol();
- $err = id($this->newPassthruCommand())
+ $this->command = id($this->newPassthruCommand())
->setIOChannel($this->getIOChannel())
->setCommandChannelFromExecFuture($future)
->setWillWriteCallback(array($this, 'willWriteMessageCallback'))
- ->setWillReadCallback(array($this, 'willReadMessageCallback'))
- ->execute();
+ ->setWillReadCallback(array($this, 'willReadMessageCallback'));
+
+ $this->command->setPauseIOReads(true);
+
+ $err = $this->command->execute();
if (!$err && $this->didSeeWrite) {
$this->getRepository()->writeStatusMessage(
@@ -161,6 +270,19 @@
switch ($this->outPhaseCount) {
case 0:
// This is the "greeting", which announces capabilities.
+
+ // We already sent this when we were figuring out which
+ // repository this request is for, so we aren't going to send
+ // it again.
+
+ // Instead, we're going to replay the client's response (which
+ // we also already read).
+
+ $command = $this->getCommand();
+ $command->writeIORead($this->peekBuffer);
+ $command->setPauseIOReads(false);
+
+ $message_raw = null;
break;
case 1:
// This responds to the client greeting, and announces auth.
@@ -203,7 +325,9 @@
}
- $result[] = $message_raw;
+ if ($message_raw !== null) {
+ $result[] = $message_raw;
+ }
}
if (!$result) {
@@ -213,7 +337,7 @@
return implode('', $result);
}
- private function makeInternalURI($uri_string) {
+ private function getPathFromSubversionURI($uri_string) {
$uri = new PhutilURI($uri_string);
$proto = $uri->getProtocol();
@@ -223,11 +347,10 @@
'Protocol for URI "%s" MUST be "svn+ssh".',
$uri_string));
}
-
$path = $uri->getPath();
// Subversion presumably deals with this, but make sure there's nothing
- // skethcy going on with the URI.
+ // sketchy going on with the URI.
if (preg_match('(/\\.\\./)', $path)) {
throw new Exception(
pht(
@@ -235,8 +358,17 @@
$uri_string));
}
- $repository = $this->loadRepository($path);
+ $path = $this->normalizeSVNPath($path);
+ return $path;
+ }
+
+ private function makeInternalURI($uri_string) {
+ $uri = new PhutilURI($uri_string);
+
+ $repository = $this->getRepository();
+
+ $path = $this->getPathFromSubversionURI($uri_string);
$path = preg_replace(
'(^/diffusion/[A-Z]+)',
rtrim($repository->getLocalPath(), '/'),
@@ -246,14 +378,25 @@
$path = rtrim($path, '/');
}
+ // NOTE: We are intentionally NOT removing username information from the
+ // URI. Subversion retains it over the course of the request and considers
+ // two repositories with different username identifiers to be distinct and
+ // incompatible.
+
$uri->setPath($path);
// If this is happening during the handshake, these are the base URIs for
// the request.
if ($this->externalBaseURI === null) {
$pre = (string)id(clone $uri)->setPath('');
- $this->externalBaseURI = $pre.'/diffusion/'.$repository->getCallsign();
- $this->internalBaseURI = $pre.rtrim($repository->getLocalPath(), '/');
+
+ $external_path = '/diffusion/'.$repository->getCallsign();
+ $external_path = $this->normalizeSVNPath($external_path);
+ $this->externalBaseURI = $pre.$external_path;
+
+ $internal_path = rtrim($repository->getLocalPath(), '/');
+ $internal_path = $this->normalizeSVNPath($internal_path);
+ $this->internalBaseURI = $pre.$internal_path;
}
return (string)$uri;
@@ -270,4 +413,12 @@
return $uri;
}
+ private function normalizeSVNPath($path) {
+ // Subversion normalizes redundant slashes internally, so normalize them
+ // here as well to make sure things match up.
+ $path = preg_replace('(/+)', '/', $path);
+
+ return $path;
+ }
+
}
diff --git a/src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php b/src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php
--- a/src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php
+++ b/src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php
@@ -43,6 +43,7 @@
private $execFuture;
private $willWriteCallback;
private $willReadCallback;
+ private $pauseIOReads;
public function setCommandChannelFromExecFuture(ExecFuture $exec_future) {
$exec_channel = new PhutilExecChannel($exec_future);
@@ -78,6 +79,11 @@
$this->errorChannel->write($data);
}
+ public function setPauseIOReads($pause) {
+ $this->pauseIOReads = $pause;
+ return $this;
+ }
+
public function execute() {
$command_channel = $this->commandChannel;
$io_channel = $this->ioChannel;
@@ -140,16 +146,15 @@
$done = !$command_channel->isOpenForReading() &&
$command_channel->isReadBufferEmpty();
- $in_message = $io_channel->read();
- if ($in_message !== null) {
- $in_message = $this->willWriteData($in_message);
+ if (!$this->pauseIOReads) {
+ $in_message = $io_channel->read();
if ($in_message !== null) {
- $command_channel->write($in_message);
+ $this->writeIORead($in_message);
}
}
$out_message = $command_channel->read();
- if ($out_message !== null) {
+ if (strlen($out_message)) {
$out_message = $this->willReadData($out_message);
if ($out_message !== null) {
$io_channel->write($out_message);
@@ -185,6 +190,13 @@
return $err;
}
+ public function writeIORead($in_message) {
+ $in_message = $this->willWriteData($in_message);
+ if (strlen($in_message)) {
+ $this->commandChannel->write($in_message);
+ }
+ }
+
public function willWriteData($message) {
if ($this->willWriteCallback) {
return call_user_func($this->willWriteCallback, $this, $message);

File Metadata

Mime Type
text/plain
Expires
Sat, Jan 18, 6:40 AM (8 h, 23 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7000569
Default Alt Text
D11541.diff (17 KB)

Event Timeline