D7553.diff
No OneTemporary
Actions

Size

23 KB

Referenced Files

None

Subscribers

None

D7553.diff
View Options

	Index: scripts/ssh/ssh-exec.php
	===================================================================
	--- scripts/ssh/ssh-exec.php
	+++ scripts/ssh/ssh-exec.php
	@@ -61,7 +61,7 @@

	$workflows = array(
	new ConduitSSHWorkflow(),
	-
	+ new DiffusionSSHMercurialServeWorkflow(),
	new DiffusionSSHGitUploadPackWorkflow(),
	new DiffusionSSHGitReceivePackWorkflow(),
	);
	Index: src/__phutil_library_map__.php
	===================================================================
	--- src/__phutil_library_map__.php
	+++ src/__phutil_library_map__.php
	@@ -543,6 +543,10 @@
	'DiffusionSSHGitReceivePackWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitReceivePackWorkflow.php',
	'DiffusionSSHGitUploadPackWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitUploadPackWorkflow.php',
	'DiffusionSSHGitWorkflow' => 'applications/diffusion/ssh/DiffusionSSHGitWorkflow.php',
	+ 'DiffusionSSHMercurialServeWorkflow' => 'applications/diffusion/ssh/DiffusionSSHMercurialServeWorkflow.php',
	+ 'DiffusionSSHMercurialWireClientProtocolChannel' => 'applications/diffusion/ssh/DiffusionSSHMercurialWireClientProtocolChannel.php',
	+ 'DiffusionSSHMercurialWireTestCase' => 'applications/diffusion/ssh/__tests__/DiffusionSSHMercurialWireTestCase.php',
	+ 'DiffusionSSHMercurialWorkflow' => 'applications/diffusion/ssh/DiffusionSSHMercurialWorkflow.php',
	'DiffusionSSHWorkflow' => 'applications/diffusion/ssh/DiffusionSSHWorkflow.php',
	'DiffusionServeController' => 'applications/diffusion/controller/DiffusionServeController.php',
	'DiffusionSetPasswordPanel' => 'applications/diffusion/panel/DiffusionSetPasswordPanel.php',
	@@ -2808,6 +2812,10 @@
	'DiffusionSSHGitReceivePackWorkflow' => 'DiffusionSSHGitWorkflow',
	'DiffusionSSHGitUploadPackWorkflow' => 'DiffusionSSHGitWorkflow',
	'DiffusionSSHGitWorkflow' => 'DiffusionSSHWorkflow',
	+ 'DiffusionSSHMercurialServeWorkflow' => 'DiffusionSSHMercurialWorkflow',
	+ 'DiffusionSSHMercurialWireClientProtocolChannel' => 'PhutilProtocolChannel',
	+ 'DiffusionSSHMercurialWireTestCase' => 'PhabricatorTestCase',
	+ 'DiffusionSSHMercurialWorkflow' => 'DiffusionSSHWorkflow',
	'DiffusionSSHWorkflow' => 'PhabricatorSSHWorkflow',
	'DiffusionServeController' => 'DiffusionController',
	'DiffusionSetPasswordPanel' => 'PhabricatorSettingsPanel',
	Index: src/applications/diffusion/controller/DiffusionServeController.php
	===================================================================
	--- src/applications/diffusion/controller/DiffusionServeController.php
	+++ src/applications/diffusion/controller/DiffusionServeController.php
	@@ -228,40 +228,8 @@
	case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
	$cmd = $request->getStr('cmd');
	if ($cmd == 'batch') {
	- // For "batch" we get a "cmds" argument like
	- //
	- // heads ;known nodes=
	- //
	- // We need to examine the commands (here, "heads" and "known") to
	- // make sure they're all read-only.
	-
	- $args = $this->getMercurialArguments();
	- $cmds = idx($args, 'cmds');
	- if ($cmds) {
	-
	- // NOTE: Mercurial has some code to escape semicolons, but it does
	- // not actually function for command separation. For example, these
	- // two batch commands will produce completely different results (the
	- // former will run the lookup; the latter will fail with a parser
	- // error):
	- //
	- // lookup key=a:xb;lookup key=z* 0
	- // lookup key=a:;b;lookup key=z* 0
	- // ^
	- // \|
	- // +-- Note semicolon.
	- //
	- // So just split unconditionally.
	-
	- $cmds = explode(';', $cmds);
	- foreach ($cmds as $sub_cmd) {
	- $name = head(explode(' ', $sub_cmd, 2));
	- if (!DiffusionMercurialWireProtocol::isReadOnlyCommand($name)) {
	- return false;
	- }
	- }
	- return true;
	- }
	+ $cmds = idx($this->getMercurialArguments(), 'cmds');
	+ return DiffusionMercurialWireProtocol::isReadOnlyBatchCommand($cmds);
	}
	return DiffusionMercurialWireProtocol::isReadOnlyCommand($cmd);
	case PhabricatorRepositoryType::REPOSITORY_TYPE_SUBVERSION:
	Index: src/applications/diffusion/protocol/DiffusionMercurialWireProtocol.php
	===================================================================
	--- src/applications/diffusion/protocol/DiffusionMercurialWireProtocol.php
	+++ src/applications/diffusion/protocol/DiffusionMercurialWireProtocol.php
	@@ -59,4 +59,44 @@
	return isset($read_only[$command]);
	}

	+ public static function isReadOnlyBatchCommand($cmds) {
	+ if (!strlen($cmds)) {
	+ // We expect a "batch" command to always have a "cmds" string, so err
	+ // on the side of caution and throw if we don't get any data here. This
	+ // either indicates a mangled command from the client or a programming
	+ // error in our code.
	+ throw new Exception("Expected nonempty 'cmds' specification!");
	+ }
	+
	+ // For "batch" we get a "cmds" argument like:
	+ //
	+ // heads ;known nodes=
	+ //
	+ // We need to examine the commands (here, "heads" and "known") to make sure
	+ // they're all read-only.
	+
	+ // NOTE: Mercurial has some code to escape semicolons, but it does not
	+ // actually function for command separation. For example, these two batch
	+ // commands will produce completely different results (the former will run
	+ // the lookup; the latter will fail with a parser error):
	+ //
	+ // lookup key=a:xb;lookup key=z* 0
	+ // lookup key=a:;b;lookup key=z* 0
	+ // ^
	+ // \|
	+ // +-- Note semicolon.
	+ //
	+ // So just split unconditionally.
	+
	+ $cmds = explode(';', $cmds);
	+ foreach ($cmds as $sub_cmd) {
	+ $name = head(explode(' ', $sub_cmd, 2));
	+ if (!self::isReadOnlyCommand($name)) {
	+ return false;
	+ }
	+ }
	+
	+ return true;
	+ }
	+
	}
	Index: src/applications/diffusion/ssh/DiffusionSSHMercurialServeWorkflow.php
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/DiffusionSSHMercurialServeWorkflow.php
	@@ -0,0 +1,106 @@
	+<?php
	+
	+final class DiffusionSSHMercurialServeWorkflow
	+ extends DiffusionSSHMercurialWorkflow {
	+
	+ protected $didSeeWrite;
	+
	+ public function didConstruct() {
	+ $this->setName('hg');
	+ $this->setArguments(
	+ array(
	+ array(
	+ 'name' => 'repository',
	+ 'short' => 'R',
	+ 'param' => 'repo',
	+ ),
	+ array(
	+ 'name' => 'stdio',
	+ ),
	+ array(
	+ 'name' => 'command',
	+ 'wildcard' => true,
	+ ),
	+ ));
	+ }
	+
	+ public function getRequestPath() {
	+ return $this->getArgs()->getArg('repository');
	+ }
	+
	+ protected function executeRepositoryOperations(
	+ PhabricatorRepository $repository) {
	+
	+ $args = $this->getArgs();
	+
	+ if (!$args->getArg('stdio')) {
	+ throw new Exception("Expected `hg ... --stdio`!");
	+ }
	+
	+ if ($args->getArg('command') !== array('serve')) {
	+ throw new Exception("Expected `hg ... serve`!");
	+ }
	+
	+ $future = new ExecFuture(
	+ 'hg -R %s serve --stdio',
	+ $repository->getLocalPath());
	+
	+ $io_channel = $this->getIOChannel();
	+
	+ $protocol_channel = new DiffusionSSHMercurialWireClientProtocolChannel(
	+ $io_channel);
	+
	+ $err = id($this->newPassthruCommand())
	+ ->setIOChannel($protocol_channel)
	+ ->setCommandChannelFromExecFuture($future)
	+ ->setWillWriteCallback(array($this, 'willWriteMessageCallback'))
	+ ->execute();
	+
	+ // TODO: It's apparently technically possible to communicate errors to
	+ // Mercurial over SSH by writing a special "\n<error>\n-\n" string. However,
	+ // my attempt to implement that resulted in Mercurial closing the socket and
	+ // then hanging, without showing the error. This might be an issue on our
	+ // side (we need to close our half of the socket?), or maybe the code
	+ // for this in Mercurial doesn't actually work, or maybe something else
	+ // is afoot. At some point, we should look into doing this more cleanly.
	+ // For now, when we, e.g., reject writes for policy reasons, the user will
	+ // see "abort: unexpected response: empty string" after the diagnostically
	+ // useful, e.g., "remote: This repository is read-only over SSH." message.
	+
	+ if (!$err && $this->didSeeWrite) {
	+ $repository->writeStatusMessage(
	+ PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE,
	+ PhabricatorRepositoryStatusMessage::CODE_OKAY);
	+ }
	+
	+ return $err;
	+ }
	+
	+ public function willWriteMessageCallback(
	+ PhabricatorSSHPassthruCommand $command,
	+ $message) {
	+
	+ $command = $message['command'];
	+
	+ // Check if this is a readonly command.
	+
	+ $is_readonly = false;
	+ if ($command == 'batch') {
	+ $cmds = idx($message['arguments'], 'cmds');
	+ if (DiffusionMercurialWireProtocol::isReadOnlyBatchCommand($cmds)) {
	+ $is_readonly = true;
	+ }
	+ } else if (DiffusionMercurialWireProtocol::isReadOnlyCommand($command)) {
	+ $is_readonly = true;
	+ }
	+
	+ if (!$is_readonly) {
	+ $this->requireWriteAccess();
	+ $this->didSeeWrite = true;
	+ }
	+
	+ // If we're good, return the raw message data.
	+ return $message['raw'];
	+ }
	+
	+}
	Index: src/applications/diffusion/ssh/DiffusionSSHMercurialWireClientProtocolChannel.php
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/DiffusionSSHMercurialWireClientProtocolChannel.php
	@@ -0,0 +1,217 @@
	+<?php
	+
	+final class DiffusionSSHMercurialWireClientProtocolChannel
	+ extends PhutilProtocolChannel {
	+
	+ private $buffer = '';
	+ private $state = 'command';
	+ private $expectArgumentCount;
	+ private $argumentName;
	+ private $expectBytes;
	+ private $command;
	+ private $arguments;
	+ private $raw;
	+
	+ protected function encodeMessage($message) {
	+ return $message;
	+ }
	+
	+ private function initializeState($last_command = null) {
	+ if ($last_command == 'unbundle') {
	+ $this->command = '<raw-data>';
	+ $this->state = 'data-length';
	+ } else {
	+ $this->state = 'command';
	+ }
	+ $this->expectArgumentCount = null;
	+ $this->expectBytes = null;
	+ $this->command = null;
	+ $this->argumentName = null;
	+ $this->arguments = array();
	+ $this->raw = '';
	+ }
	+
	+ private function readProtocolLine() {
	+ $pos = strpos($this->buffer, "\n");
	+
	+ if ($pos === false) {
	+ return null;
	+ }
	+
	+ $line = substr($this->buffer, 0, $pos);
	+
	+ $this->raw .= $line."\n";
	+ $this->buffer = substr($this->buffer, $pos + 1);
	+
	+ return $line;
	+ }
	+
	+ private function readProtocolBytes() {
	+ if (strlen($this->buffer) < $this->expectBytes) {
	+ return null;
	+ }
	+
	+ $bytes = substr($this->buffer, 0, $this->expectBytes);
	+ $this->raw .= $bytes;
	+ $this->buffer = substr($this->buffer, $this->expectBytes);
	+
	+ return $bytes;
	+ }
	+
	+ private function newMessageAndResetState() {
	+ $message = array(
	+ 'command' => $this->command,
	+ 'arguments' => $this->arguments,
	+ 'raw' => $this->raw,
	+ );
	+ $this->initializeState($this->command);
	+ return $message;
	+ }
	+
	+ private function newDataMessage($bytes) {
	+ $message = array(
	+ 'command' => '<raw-data>',
	+ 'raw' => strlen($bytes)."\n".$bytes,
	+ );
	+ return $message;
	+ }
	+
	+ protected function decodeStream($data) {
	+ $this->buffer .= $data;
	+
	+ $out = array();
	+ $messages = array();
	+
	+ while (true) {
	+ if ($this->state == 'command') {
	+ $this->initializeState();
	+
	+ // We're reading a command. It looks like:
	+ //
	+ // <command>
	+
	+ $line = $this->readProtocolLine();
	+ if ($line === null) {
	+ break;
	+ }
	+
	+ $this->command = $line;
	+ $this->state = 'arguments';
	+ } else if ($this->state == 'arguments') {
	+
	+ // Check if we're still waiting for arguments.
	+ $args = DiffusionMercurialWireProtocol::getCommandArgs($this->command);
	+ $have = array_select_keys($this->arguments, $args);
	+ if (count($have) == count($args)) {
	+ // We have all the arguments. Emit a message and read the next
	+ // command.
	+ $messages[] = $this->newMessageAndResetState();
	+ } else {
	+ // We're still reading arguments. They can either look like:
	+ //
	+ // <name> <length(value)>
	+ // <value>
	+ // ...
	+ //
	+ // ...or like this:
	+ //
	+ // * <count>
	+ // <name1> <length(value1)>
	+ // <value1>
	+ // ...
	+
	+ $line = $this->readProtocolLine();
	+ if ($line === null) {
	+ break;
	+ }
	+
	+ list($arg, $size) = explode(' ', $line, 2);
	+ $size = (int)$size;
	+
	+ if ($arg != '*') {
	+ $this->expectBytes = $size;
	+ $this->argumentName = $arg;
	+ $this->state = 'value';
	+ } else {
	+ $this->arguments['*'] = array();
	+ $this->expectArgumentCount = $size;
	+ $this->state = 'argv';
	+ }
	+ }
	+ } else if ($this->state == 'value' \|\| $this->state == 'argv-value') {
	+
	+ // We're reading the value of an argument. We just need to wait for
	+ // the right number of bytes to show up.
	+
	+ $bytes = $this->readProtocolBytes();
	+ if ($bytes === null) {
	+ break;
	+ }
	+
	+ if ($this->state == 'argv-value') {
	+ $this->arguments['*'][$this->argumentName] = $bytes;
	+ $this->state = 'argv';
	+ } else {
	+ $this->arguments[$this->argumentName] = $bytes;
	+ $this->state = 'arguments';
	+ }
	+
	+
	+ } else if ($this->state == 'argv') {
	+
	+ // We're reading a variable number of arguments. We need to wait for
	+ // the arguments to arrive.
	+
	+ if ($this->expectArgumentCount) {
	+ $line = $this->readProtocolLine();
	+ if ($line === null) {
	+ break;
	+ }
	+
	+ list($arg, $size) = explode(' ', $line, 2);
	+ $size = (int)$size;
	+
	+ $this->expectBytes = $size;
	+ $this->argumentName = $arg;
	+ $this->state = 'argv-value';
	+
	+ $this->expectArgumentCount--;
	+ } else {
	+ $this->state = 'arguments';
	+ }
	+ } else if ($this->state == 'data-length') {
	+ $line = $this->readProtocolLine();
	+ if ($line === null) {
	+ break;
	+ }
	+ $this->expectBytes = (int)$line;
	+ if (!$this->expectBytes) {
	+ $messages[] = $this->newDataMessage('');
	+ $this->initializeState();
	+ } else {
	+ $this->state = 'data-bytes';
	+ }
	+ } else if ($this->state == 'data-bytes') {
	+ $bytes = substr($this->buffer, 0, $this->expectBytes);
	+ $this->buffer = substr($this->buffer, strlen($bytes));
	+ $this->expectBytes -= strlen($bytes);
	+
	+ $messages[] = $this->newDataMessage($bytes);
	+
	+ if (!$this->expectBytes) {
	+ // We've finished reading this chunk, so go read the next chunk.
	+ $this->state = 'data-length';
	+ } else {
	+ // We're waiting for more data, and have read everything available
	+ // to us so far.
	+ break;
	+ }
	+ } else {
	+ throw new Exception("Bad parser state '{$this->state}'!");
	+ }
	+ }
	+
	+ return $messages;
	+ }
	+
	+}
	Index: src/applications/diffusion/ssh/DiffusionSSHMercurialWorkflow.php
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/DiffusionSSHMercurialWorkflow.php
	@@ -0,0 +1,5 @@
	+<?php
	+
	+abstract class DiffusionSSHMercurialWorkflow extends DiffusionSSHWorkflow {
	+
	+}
	Index: src/applications/diffusion/ssh/__tests__/DiffusionSSHMercurialWireTestCase.php
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/__tests__/DiffusionSSHMercurialWireTestCase.php
	@@ -0,0 +1,58 @@
	+<?php
	+
	+final class DiffusionSSHMercurialWireTestCase
	+ extends PhabricatorTestCase {
	+
	+ public function testMercurialClientWireProtocolParser() {
	+ $data = dirname(__FILE__).'/hgwiredata/';
	+ $dir = Filesystem::listDirectory($data, $include_hidden = false);
	+ foreach ($dir as $file) {
	+ $raw = Filesystem::readFile($data.$file);
	+ $raw = explode("\n~~~~~~~~~~\n", $raw, 2);
	+ $this->assertEqual(2, count($raw));
	+ $expect = json_decode($raw[1], true);
	+ $this->assertEqual(true, is_array($expect), $file);
	+
	+ $this->assertParserResult($expect, $raw[0], $file);
	+ }
	+ }
	+
	+ private function assertParserResult(array $expect, $input, $file) {
	+ list($x, $y) = PhutilSocketChannel::newChannelPair();
	+ $xp = new DiffusionSSHMercurialWireClientProtocolChannel($x);
	+
	+ $y->write($input);
	+ $y->flush();
	+ $y->closeWriteChannel();
	+
	+ $messages = array();
	+ for ($ii = 0; $ii < count($expect); $ii++) {
	+ try {
	+ $messages[] = $xp->waitForMessage();
	+ } catch (Exception $ex) {
	+ // This is probably the parser not producing as many messages as
	+ // we expect. Log the exception, but continue to the assertion below
	+ // since that will often be easier to diagnose.
	+ phlog($ex);
	+ break;
	+ }
	+ }
	+
	+ $this->assertEqual($expect, $messages, $file);
	+
	+ // Now, make sure the channel doesn't have more messages than we expect.
	+ // Specifically, it should throw when we try to read another message.
	+ $caught = null;
	+ try {
	+ $xp->waitForMessage();
	+ } catch (Exception $ex) {
	+ $caught = $ex;
	+ }
	+
	+ $this->assertEqual(
	+ true,
	+ ($caught instanceof Exception),
	+ "No extra messages for '{$file}'.");
	+ }
	+
	+}
	Index: src/applications/diffusion/ssh/__tests__/hgwiredata/batch.txt
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/__tests__/hgwiredata/batch.txt
	@@ -0,0 +1,16 @@
	+batch
	+* 0
	+cmds 19
	+heads ;known nodes=
	+~~~~~~~~~~
	+[
	+ {
	+ "command" : "batch",
	+ "arguments" : {
	+ "*" : {
	+ },
	+ "cmds" : "heads ;known nodes="
	+ },
	+ "raw" : "batch\n* 0\ncmds 19\nheads ;known nodes="
	+ }
	+]
	Index: src/applications/diffusion/ssh/__tests__/hgwiredata/capabilities.txt
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/__tests__/hgwiredata/capabilities.txt
	@@ -0,0 +1,10 @@
	+capabilities
	+
	+~~~~~~~~~~
	+[
	+ {
	+ "command" : "capabilities",
	+ "arguments" : [],
	+ "raw" : "capabilities\n"
	+ }
	+]
	Index: src/applications/diffusion/ssh/__tests__/hgwiredata/capabilities2.txt
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/__tests__/hgwiredata/capabilities2.txt
	@@ -0,0 +1,16 @@
	+capabilities
	+capabilities
	+
	+~~~~~~~~~~
	+[
	+ {
	+ "command" : "capabilities",
	+ "arguments" : [],
	+ "raw" : "capabilities\n"
	+ },
	+ {
	+ "command" : "capabilities",
	+ "arguments" : [],
	+ "raw" : "capabilities\n"
	+ }
	+]
	Index: src/applications/diffusion/ssh/__tests__/hgwiredata/getbundle.txt
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/__tests__/hgwiredata/getbundle.txt
	@@ -0,0 +1,18 @@
	+getbundle
	+* 2
	+common 40
	+0000000000000000000000000000000000000000heads 122
	+7cb27ad591d60500c020283b81c6467540218eda 1036b72db89a0451fa82fcd5462d903f591f0a3c 0b9d8290c4e067a0b91b43062ee9de392e8fae88
	+~~~~~~~~~~
	+[
	+ {
	+ "command" : "getbundle",
	+ "arguments" : {
	+ "*" : {
	+ "common" : "0000000000000000000000000000000000000000",
	+ "heads" : "7cb27ad591d60500c020283b81c6467540218eda 1036b72db89a0451fa82fcd5462d903f591f0a3c 0b9d8290c4e067a0b91b43062ee9de392e8fae88"
	+ }
	+ },
	+ "raw" : "getbundle\n* 2\ncommon 40\n0000000000000000000000000000000000000000heads 122\n7cb27ad591d60500c020283b81c6467540218eda 1036b72db89a0451fa82fcd5462d903f591f0a3c 0b9d8290c4e067a0b91b43062ee9de392e8fae88"
	+ }
	+]
	Index: src/applications/diffusion/ssh/__tests__/hgwiredata/unbundle.txt
	===================================================================
	--- /dev/null
	+++ src/applications/diffusion/ssh/__tests__/hgwiredata/unbundle.txt
	@@ -0,0 +1,28 @@
	+unbundle
	+heads 53
	+686173686564 8022e00be6886fcf1be8f57f96c78aa924967f8320
	+aaaaaaaaaaaaaaaaaaaa20
	+bbbbbbbbbbbbbbbbbbbb0
	+
	+~~~~~~~~~~
	+[
	+ {
	+ "command" : "unbundle",
	+ "arguments" : {
	+ "heads" : "686173686564 8022e00be6886fcf1be8f57f96c78aa924967f83"
	+ },
	+ "raw" : "unbundle\nheads 53\n686173686564 8022e00be6886fcf1be8f57f96c78aa924967f83"
	+ },
	+ {
	+ "command" : "<raw-data>",
	+ "raw" : "20\naaaaaaaaaaaaaaaaaaaa"
	+ },
	+ {
	+ "command" : "<raw-data>",
	+ "raw" : "20\nbbbbbbbbbbbbbbbbbbbb"
	+ },
	+ {
	+ "command" : "<raw-data>",
	+ "raw" : "0\n"
	+ }
	+]
	Index: src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php
	===================================================================
	--- src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php
	+++ src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php
	@@ -76,6 +76,13 @@

	public function writeErrorIOCallback(PhutilChannel $channel, $data) {
	$this->errorChannel->write($data);
	+
	+ // TODO: Because of the way `waitForAny()` works, we degrade to a busy
	+ // wait if we hand it a writable, write-only channel. We should handle this
	+ // case better in `waitForAny()`. For now, just flush the error channel
	+ // explicity after writing data over it.
	+
	+ $this->errorChannel->flush();
	}

	public function execute() {
	@@ -98,7 +105,9 @@
	$channels = array($command_channel, $io_channel, $error_channel);

	while (true) {
	- PhutilChannel::waitForAny($channels);
	+ // TODO: See note in writeErrorIOCallback!
	+ $wait = array($command_channel, $io_channel);
	+ PhutilChannel::waitForAny($wait);

	$io_channel->update();
	$command_channel->update();
	@@ -107,21 +116,24 @@
	$done = !$command_channel->isOpen();

	$in_message = $io_channel->read();
	- $in_message = $this->willWriteData($in_message);
	if ($in_message !== null) {
	- $command_channel->write($in_message);
	+ $in_message = $this->willWriteData($in_message);
	+ if ($in_message !== null) {
	+ $command_channel->write($in_message);
	+ }
	}

	$out_message = $command_channel->read();
	- $out_message = $this->willReadData($out_message);
	if ($out_message !== null) {
	- $io_channel->write($out_message);
	+ $out_message = $this->willReadData($out_message);
	+ if ($out_message !== null) {
	+ $io_channel->write($out_message);
	+ }
	}

	// If we have nothing left on stdin, close stdin on the subprocess.
	if (!$io_channel->isOpenForReading()) {
	- // TODO: This should probably be part of PhutilExecChannel?
	- $this->execFuture->write('');
	+ $command_channel->closeWriteChannel();
	}

	if ($done) {

File Metadata

Mime Type: text/plain
Expires: Sun, Dec 29, 2:53 PM (9 h, 23 m)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 6942593
Default Alt Text: D7553.diff (23 KB)

D7553.diffNo OneTemporaryActions

D7553.diffView Options

File Metadata

Event Timeline

D7553.diff
No OneTemporary
Actions

D7553.diff
View Options