Return null when eof reached in socket

amphp · Feb 25, 2018 · eafddca · eafddca
1 parent 1171eb3
commit eafddca
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 19 deletions.
diff --git a/composer.json b/composer.json
@@ -25,9 +25,10 @@
         "psr/log": "^1.0"
     },
     "require-dev": {
-        "phpunit/phpunit": "^6.5",
+        "amphp/phpunit-util": "^1",
+        "friendsofphp/php-cs-fixer": "^2.9",
         "monolog/monolog": "^1.23",
-        "friendsofphp/php-cs-fixer": "^2.9"
+        "phpunit/phpunit": "^6.5"
     },
     "autoload": {
         "psr-4": {

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -1,11 +1,18 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
-<phpunit bootstrap="./vendor/autoload.php"
-         colors="true"
-         convertErrorsToExceptions="true"
-         convertNoticesToExceptions="true"
-         convertWarningsToExceptions="true"
-         syntaxCheck="true">
+<phpunit
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/6.0/phpunit.xsd"
+    backupGlobals="false"
+    backupStaticAttributes="false"
+    bootstrap="vendor/autoload.php"
+    colors="true"
+    convertErrorsToExceptions="true"
+    convertNoticesToExceptions="true"
+    convertWarningsToExceptions="true"
+    processIsolation="false"
+    stopOnFailure="false"
+>
     <php>
         <env name="SHELL_VERBOSITY" value="-1" />
     </php>
@@ -24,4 +31,7 @@
             </exclude>
         </whitelist>
     </filter>
+    <listeners>
+        <listener class="Amp\PHPUnit\LoopReset"/>
+    </listeners>
 </phpunit>
diff --git a/src/Process.php b/src/Process.php
@@ -5,6 +5,7 @@
 use Amp\ByteStream\InputStream;
 use Amp\ByteStream\OutputStream;
 use Amp\Deferred;
+use Amp\Failure;
 use Amp\Promise;
 use Amp\Ssh\Channel\ChannelInputStream;
 use Amp\Ssh\Channel\ChannelOutputStream;
@@ -55,7 +56,7 @@ public function __destruct() {
 
     public function start(): Promise {
         if ($this->resolved !== null || $this->exitCode !== null) {
-            throw new \RuntimeException('Process has already been started.');
+            return new Failure(new \RuntimeException('Process has already been started.'));
         }
 
         $this->resolved = new Deferred();
@@ -79,7 +80,7 @@ public function join(): Promise {
         }
 
         if ($this->resolved === null) {
-            throw new \RuntimeException('Process has not been started.');
+            return new Failure(new \RuntimeException('Process has not been started.'));
         }
 
         return $this->resolved->promise();
@@ -91,7 +92,7 @@ public function kill() {
 
     public function signal(int $signo): Promise {
         if (!$this->isRunning()) {
-            throw new \RuntimeException('Process is not running.');
+            return new Failure(new \RuntimeException('Process is not running.'));
         }
 
         return $this->session->signal($signo);
@@ -124,6 +125,7 @@ protected function handleRequests() {
                 if ($message instanceof ChannelRequestExitStatus) {
                     $resolved = $this->resolved;
                     $this->resolved = null;
+                    $this->exitCode = $message->code;
                     $resolved->resolve($message->code);
 
                     $this->session->close();

diff --git a/src/Transport/LoggerHandler.php b/src/Transport/LoggerHandler.php
@@ -37,6 +37,10 @@ public function read(): Promise {
         return call(function () {
             $packet = yield $this->handler->read();
 
+            if ($packet === null) {
+                return null;
+            }
+
             if ($packet instanceof Message) {
                 $this->logger->debug(\sprintf('Receive %s packet', \get_class($packet)));
             } else {

diff --git a/src/Transport/MessageHandler.php b/src/Transport/MessageHandler.php
@@ -71,6 +71,10 @@ public function read(): Promise {
         return call(function () {
             $packet = yield $this->handler->read();
 
+            if ($packet === null) {
+                return null;
+            }
+
             if ($packet instanceof Message\Message) {
                 return $packet;
             }

diff --git a/src/Transport/PayloadReader.php b/src/Transport/PayloadReader.php
@@ -72,9 +72,19 @@ public function read(): Promise {
              the MAC algorithm MUST be "none".
          */
         return call(function () {
-            $packetLength = \unpack('N', yield $this->doReadDecrypted(4))[1];
+            $packetLengthRead = yield $this->doReadDecrypted(4);
+
+            if ($packetLengthRead === null) {
+                return null;
+            }
+
+            $packetLength = \unpack('N', $packetLengthRead)[1];
             $packet = yield $this->doReadDecrypted($packetLength);
 
+            if ($packet === null) {
+                return null;
+            }
+
             $paddingLength = \unpack('C', $packet)[1];
             $payload = \substr($packet, 1, $packetLength - $paddingLength - 1);
             $padding = \substr($packet, $packetLength - $paddingLength);
@@ -102,6 +112,11 @@ private function doReadDecrypted(int $length): Promise {
         return call(function () use ($length) {
             while (\strlen($this->decryptedBuffer) < $length) {
                 $rawRead = yield $this->doReadRaw($this->decryption->getBlockSize());
+
+                if ($rawRead === null) {
+                    return null;
+                }
+
                 $this->decryptedBuffer .= $this->decryption->decrypt($rawRead);
             }
 
@@ -115,7 +130,13 @@ private function doReadDecrypted(int $length): Promise {
     private function doReadRaw($length): Promise {
         return call(function () use ($length) {
             while (\strlen($this->cryptedBuffer) < $length) {
-                $this->cryptedBuffer .= yield $this->socket->read();
+                $readed = yield $this->socket->read();
+
+                if ($readed === null) {
+                    return null;
+                }
+
+                $this->cryptedBuffer .= $readed;
             }
 
             $read = \substr($this->cryptedBuffer, 0, $length);

diff --git a/src/functions.php b/src/functions.php
@@ -2,6 +2,7 @@
 
 namespace Amp\Ssh;
 
+use Amp\Promise;
 use Amp\Ssh\Authentication\Authentication;
 use Amp\Ssh\Channel\Dispatcher;
 use Amp\Ssh\Transport\LoggerHandler;
@@ -11,7 +12,7 @@
 use Psr\Log\NullLogger;
 use function Amp\call;
 
-function connect($uri, Authentication $authentication, LoggerInterface $logger = null, $identification = 'SSH-2.0-AmpSSH_0.1') {
+function connect($uri, Authentication $authentication, LoggerInterface $logger = null, $identification = 'SSH-2.0-AmpSSH_0.1'): Promise {
     return call(function () use ($uri, $authentication, $identification, $logger) {
         $socket = yield \Amp\Socket\connect($uri);
         $logger = $logger ?? new NullLogger();

diff --git a/tests/ProcessTest.php b/tests/ProcessTest.php
@@ -2,24 +2,35 @@
 
 namespace Amp\Ssh\Tests;
 
+use Amp\Delayed;
+use Amp\Loop;
+use Amp\Ssh\Authentication\UsernamePassword;
+use function Amp\Ssh\connect;
+use Amp\Ssh\Process;
 use PHPUnit\Framework\TestCase;
 
 class ProcessTest extends TestCase
 {
     public function testProcess()
     {
-        \Amp\Loop::run(function () {
-            $authentication = new \Amp\Ssh\Authentication\UsernamePassword('root', 'root');
-            $sshResource = yield \Amp\Ssh\connect('127.0.0.1:2222', $authentication);
+        Loop::run(function () {
+            $authentication = new UsernamePassword('root', 'root');
+            $sshResource = yield connect('127.0.0.1:2222', $authentication);
 
-            $process = new \Amp\Ssh\Process($sshResource, 'echo foo');
+            $process = new Process($sshResource, 'echo foo');
 
             yield $process->start();
+
+            self::assertTrue($process->isRunning());
+
             $output = yield $process->getStdout()->read();
-            yield $process->join();
 
             self::assertEquals("foo\n", $output);
 
+            $exitCode = yield $process->join();
+
+            self::assertEquals(0, $exitCode);
+
             yield $sshResource->close();
         });
     }