Merge pull request #150 from clue-labs/err

Fix faulty write buffer behavior when sending large data chunks over TLS (Mac OS X only)
reactphp · May 2, 2020 · 610aa60 · 610aa60
2 parents 1a849e7 + 26d5640
commit 610aa60
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 38 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -18,28 +18,17 @@ matrix:
     - php: hhvm-3.18
       install:
         - composer require phpunit/phpunit:^5 --dev --no-interaction # requires legacy phpunit
-    - os: osx
+    - name: Mac OS X
+      os: osx
       language: generic
-      php: 7.0 # just to look right on travis
-      env:
-        - PACKAGE: php70
+      before_install:
+        - curl -s http://getcomposer.org/installer | php
+        - mv composer.phar /usr/local/bin/composer
   allow_failures:
     - php: hhvm-3.18
     - os: osx
 
 install:
-  # OSX install inspired by https://github.com/kiler129/TravisCI-OSX-PHP
-  - |
-    if [[ "${TRAVIS_OS_NAME}" == "osx" ]]; then 
-      brew tap homebrew/homebrew-php 
-      echo "Installing PHP ..."
-      brew install "${PACKAGE}"
-      brew install "${PACKAGE}"-xdebug
-      brew link "${PACKAGE}"
-      echo "Installing composer ..."
-      curl -s http://getcomposer.org/installer | php
-      mv composer.phar /usr/local/bin/composer
-    fi
   - composer install --no-interaction
 
 script:

diff --git a/src/WritableResourceStream.php b/src/WritableResourceStream.php
@@ -113,13 +113,8 @@ public function close()
     public function handleWrite()
     {
         $error = null;
-        \set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$error) {
-            $error = array(
-                'message' => $errstr,
-                'number' => $errno,
-                'file' => $errfile,
-                'line' => $errline
-            );
+        \set_error_handler(function ($_, $errstr) use (&$error) {
+            $error = $errstr;
         });
 
         if ($this->writeChunkSize === -1) {
@@ -130,25 +125,16 @@ public function handleWrite()
 
         \restore_error_handler();
 
-        // Only report errors if *nothing* could be sent.
+        // Only report errors if *nothing* could be sent and an error has been raised.
+        // Ignore non-fatal warnings if *some* data could be sent.
         // Any hard (permanent) error will fail to send any data at all.
         // Sending excessive amounts of data will only flush *some* data and then
         // report a temporary error (EAGAIN) which we do not raise here in order
         // to keep the stream open for further tries to write.
         // Should this turn out to be a permanent error later, it will eventually
         // send *nothing* and we can detect this.
-        if ($sent === 0 || $sent === false) {
-            if ($error !== null) {
-                $error = new \ErrorException(
-                    $error['message'],
-                    0,
-                    $error['number'],
-                    $error['file'],
-                    $error['line']
-                );
-            }
-
-            $this->emit('error', array(new \RuntimeException('Unable to write to stream: ' . ($error !== null ? $error->getMessage() : 'Unknown error'), 0, $error)));
+        if (($sent === 0 || $sent === false) && $error !== null) {
+            $this->emit('error', array(new \RuntimeException('Unable to write to stream: ' . $error)));
             $this->close();
 
             return;

diff --git a/tests/FunctionalInternetTest.php b/tests/FunctionalInternetTest.php
@@ -78,11 +78,20 @@ public function testUploadKilobyteSecure()
         $this->assertNotEquals('', $buffer);
     }
 
-    public function testUploadBiggerBlockSecureRequiresSmallerChunkSize()
+    public function testUploadBiggerBlockSecure()
     {
-        $size = 50 * 1000;
+        // A few dozen kilobytes should be enough to verify this works.
+        // Underlying buffer sizes are platform-specific, so let's increase this
+        // a bit to trigger different behavior on Linux vs Mac OS X.
+        $size = 136 * 1000;
+
         $stream = stream_socket_client('tls://httpbin.org:443');
 
+        // PHP < 7.1.4 (and PHP < 7.0.18) suffers from a bug when writing big
+        // chunks of data over TLS streams at once.
+        // We work around this by limiting the write chunk size to 8192 bytes
+        // here to also support older PHP versions.
+        // See https://github.com/reactphp/socket/issues/105
         $loop = Factory::create();
         $stream = new DuplexResourceStream(
             $stream,