Merge pull request #8917 from chinpei215/http-client-hang

Fix http request not being timed out correctly

src/Network/Http/Adapter/Stream.php

@@ -257,18 +257,36 @@ class Stream
      */
     protected function _send(Request $request)
     {
+        $deadline = false;
+        if (isset($this->_contextOptions['timeout']) && $this->_contextOptions['timeout'] > 0) {
+            $deadline = time() + $this->_contextOptions['timeout'];
+        }
+
         $url = $request->url();
         $this->_open($url);
         $content = '';
+        $timedOut = false;
+
         while (!feof($this->_stream)) {
+            if ($deadline !== false) {
+                stream_set_timeout($this->_stream, max($deadline - time(), 1));
+            }
+
             $content .= fread($this->_stream, 8192);
+
+            $meta = stream_get_meta_data($this->_stream);
+            if ($meta['timed_out'] || ($deadline !== false && time() > $deadline)) {
+                $timedOut = true;
+                break;
+            }
         }
         $meta = stream_get_meta_data($this->_stream);
         fclose($this->_stream);
 
-        if ($meta['timed_out']) {
+        if ($timedOut) {
             throw new Exception('Connection timed out ' . $url);
         }
+
         $headers = $meta['wrapper_data'];
         if (isset($headers['headers']) && is_array($headers['headers'])) {
             $headers = $headers['headers'];

tests/TestCase/Network/Http/Adapter/StreamTest.php

@@ -18,6 +18,80 @@ use Cake\Network\Http\Request;
 use Cake\TestSuite\TestCase;
 
 /**
+ * CakeStreamWrapper class
+ */
+class CakeStreamWrapper implements \ArrayAccess
+{
+
+    private $_stream;
+
+    private $_query = [];
+
+    private $_data = [
+        'headers' => [
+            'HTTP/1.1 200 OK',
+        ],
+    ];
+
+    public function stream_open($path, $mode, $options, &$openedPath)
+    {
+        $query = parse_url($path, PHP_URL_QUERY);
+        if ($query) {
+            parse_str($query, $this->_query);
+        }
+
+        $this->_stream = fopen('php://memory', 'rb+');
+        fwrite($this->_stream, str_repeat('x', 20000));
+        rewind($this->_stream);
+
+        return true;
+    }
+
+    public function stream_close()
+    {
+        return fclose($this->_stream);
+    }
+
+    public function stream_read($count)
+    {
+        if (isset($this->_query['sleep'])) {
+            sleep(1);
+        }
+        return fread($this->_stream, $count);
+    }
+
+    public function stream_eof()
+    {
+        return feof($this->_stream);
+    }
+
+    public function stream_set_option($option, $arg1, $arg2)
+    {
+        return false;
+    }
+
+    public function offsetExists($offset)
+    {
+        return isset($this->_data[$offset]);
+    }
+
+    public function offsetGet($offset)
+    {
+        return $this->_data[$offset];
+    }
+
+    public function offsetSet($offset, $value)
+    {
+        $this->_data[$offset] = $value;
+    }
+
+    public function offsetUnset($offset)
+    {
+        unset($this->_data[$offset]);
+    }
+}
+
+/**
  * HTTP stream adapter test.
  */
 class StreamTest extends TestCase
@@ -30,6 +104,13 @@ class StreamTest extends TestCase
             'Cake\Network\Http\Adapter\Stream',
             ['_send']
         );
+        stream_wrapper_register('cakephp', __NAMESPACE__ . '\CakeStreamWrapper');
+    }
+
+    public function tearDown()
+    {
+        parent::tearDown();
+        stream_wrapper_unregister('cakephp');
     }
 
     /**
@@ -54,6 +135,23 @@ class StreamTest extends TestCase
     }
 
     /**
+     * Test the send method by using cakephp:// protocol.
+     *
+     * @return void
+     */
+    public function testSendByUsingCakephpProtocol()
+    {
+        $stream = new Stream();
+        $request = new Request();
+        $request->url('cakephp://dummy/');
+
+        $responses = $stream->send($request, []);
+        $this->assertInstanceOf('Cake\Network\Http\Response', $responses[0]);
+
+        $this->assertEquals(20000, strlen($responses[0]->body()));
+    }
+
+    /**
      * Test building the context headers
      *
      * @return void
@@ -301,4 +399,42 @@ class StreamTest extends TestCase
         $this->assertEquals(null, $responses[2]->cookie('second'));
         $this->assertEquals('works', $responses[2]->cookie('third'));
     }
+
+    /**
+     * Test that no exception is radied when not timed out.
+     *
+     * @return void
+     */
+    public function testKeepDeadline()
+    {
+        $request = new Request();
+        $request->url('cakephp://dummy/?sleep');
+        $options = [
+            'timeout' => 5,
+        ];
+
+        $t = microtime(true);
+        $stream = new Stream();
+        $stream->send($request, $options);
+        $this->assertLessThan(5, microtime(true) - $t);
+    }
+
+    /**
+     * Test that an exception is raised when timed out.
+     *
+     * @expectedException \Cake\Core\Exception\Exception
+     * @expectedExceptionMessage Connection timed out cakephp://dummy/?sleep
+     * @return void
+     */
+    public function testMissDeadline()
+    {
+        $request = new Request();
+        $request->url('cakephp://dummy/?sleep');
+        $options = [
+            'timeout' => 2,
+        ];
+
+        $stream = new Stream();
+        $stream->send($request, $options);
+    }
 }