Add Connection: close default header to allow toggling keep-alive

reactphp · Jan 21, 2023 · ebaf6f1 · ebaf6f1
1 parent 28943f4
commit ebaf6f1
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 64 deletions.
diff --git a/src/Browser.php b/src/Browser.php
@@ -23,6 +23,7 @@ class Browser
     private $baseUrl;
     private $protocolVersion = '1.1';
     private $defaultHeaders = array(
+        'Connection' => 'close',
         'User-Agent' => 'ReactPHP/1'
     );
 

diff --git a/src/Io/Sender.php b/src/Io/Sender.php
@@ -98,13 +98,6 @@ public function send(RequestInterface $request)
             $size = 0;
         }
 
-        // automatically add `Connection: close` request header for HTTP/1.1 requests to avoid connection reuse
-        if ($request->getProtocolVersion() === '1.1') {
-            $request = $request->withHeader('Connection', 'close');
-        } else {
-            $request = $request->withoutHeader('Connection');
-        }
-
         // automatically add `Authorization: Basic …` request header if URL includes `user:pass@host`
         if ($request->getUri()->getUserInfo() !== '' && !$request->hasHeader('Authorization')) {
             $request = $request->withHeader('Authorization', 'Basic ' . \base64_encode($request->getUri()->getUserInfo()));

diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php
@@ -302,7 +302,7 @@ private function makeRedirectRequest(RequestInterface $request, UriInterface $lo
                 ->withMethod($request->getMethod() === 'HEAD' ? 'HEAD' : 'GET')
                 ->withoutHeader('Content-Type')
                 ->withoutHeader('Content-Length')
-                ->withBody(new EmptyBodyStream());
+                ->withBody(new BufferedBody(''));
         }
 
         return $request;

diff --git a/tests/BrowserTest.php b/tests/BrowserTest.php
@@ -556,6 +556,8 @@ public function testWithMultipleHeadersShouldBeMergedCorrectlyWithMultipleDefaul
                 'user-Agent' => array('ABC'),
                 'another-header' => array('value'),
                 'custom-header' => array('data'),
+
+                'Connection' => array('close')
             );
 
             $that->assertEquals($expectedHeaders, $request->getHeaders());
@@ -584,6 +586,32 @@ public function testWithoutHeaderShouldRemoveExistingHeader()
         $this->browser->get('http://example.com/');
     }
 
+    public function testWithoutHeaderConnectionShouldRemoveDefaultConnectionHeader()
+    {
+        $this->browser = $this->browser->withoutHeader('Connection');
+
+        $that = $this;
+        $this->sender->expects($this->once())->method('send')->with($this->callback(function (RequestInterface $request) use ($that) {
+            $that->assertEquals(array(), $request->getHeader('Connection'));
+            return true;
+        }))->willReturn(new Promise(function () { }));
+
+        $this->browser->get('http://example.com/');
+    }
+
+    public function testWithHeaderConnectionShouldOverwriteDefaultConnectionHeader()
+    {
+        $this->browser = $this->browser->withHeader('Connection', 'keep-alive');
+
+        $that = $this;
+        $this->sender->expects($this->once())->method('send')->with($this->callback(function (RequestInterface $request) use ($that) {
+            $that->assertEquals(array('keep-alive'), $request->getHeader('Connection'));
+            return true;
+        }))->willReturn(new Promise(function () { }));
+
+        $this->browser->get('http://example.com/');
+    }
+
     public function testBrowserShouldSendDefaultUserAgentHeader()
     {
         $that = $this;

diff --git a/tests/FunctionalBrowserTest.php b/tests/FunctionalBrowserTest.php
@@ -553,6 +553,60 @@ public function testReceiveStreamAndExplicitlyCloseConnectionEvenWhenServerKeeps
         $socket->close();
     }
 
+    public function testRequestWillCreateNewConnectionForSecondRequestByDefaultEvenWhenServerKeepsConnectionOpen()
+    {
+        $twice = $this->expectCallableOnce();
+        $socket = new SocketServer('127.0.0.1:0');
+        $socket->on('connection', function (\React\Socket\ConnectionInterface $connection) use ($socket, $twice) {
+            $connection->on('data', function () use ($connection) {
+                $connection->write("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nhello");
+            });
+
+            $socket->on('connection', $twice);
+            $socket->on('connection', function () use ($socket) {
+                $socket->close();
+            });
+        });
+
+        $this->base = str_replace('tcp:', 'http:', $socket->getAddress()) . '/';
+
+        $response = \React\Async\await($this->browser->get($this->base . 'get'));
+        assert($response instanceof ResponseInterface);
+        $this->assertEquals('hello', (string)$response->getBody());
+
+        $response = \React\Async\await($this->browser->get($this->base . 'get'));
+        assert($response instanceof ResponseInterface);
+        $this->assertEquals('hello', (string)$response->getBody());
+    }
+
+    public function testRequestWithoutConnectionHeaderWillReuseExistingConnectionForSecondRequest()
+    {
+        $this->socket->on('connection', $this->expectCallableOnce());
+
+        // remove default `Connection: close` request header to enable keep-alive
+        $this->browser = $this->browser->withoutHeader('Connection');
+
+        $response = \React\Async\await($this->browser->get($this->base . 'get'));
+        assert($response instanceof ResponseInterface);
+        $this->assertEquals('hello', (string)$response->getBody());
+
+        $response = \React\Async\await($this->browser->get($this->base . 'get'));
+        assert($response instanceof ResponseInterface);
+        $this->assertEquals('hello', (string)$response->getBody());
+    }
+
+    public function testRequestWithoutConnectionHeaderWillReuseExistingConnectionForRedirectedRequest()
+    {
+        $this->socket->on('connection', $this->expectCallableOnce());
+
+        // remove default `Connection: close` request header to enable keep-alive
+        $this->browser = $this->browser->withoutHeader('Connection');
+
+        $response = \React\Async\await($this->browser->get($this->base . 'redirect-to?url=get'));
+        assert($response instanceof ResponseInterface);
+        $this->assertEquals('hello', (string)$response->getBody());
+    }
+
     public function testPostStreamChunked()
     {
         $stream = new ThroughStream();

diff --git a/tests/Io/SenderTest.php b/tests/Io/SenderTest.php
@@ -290,62 +290,6 @@ public function testSendCustomMethodWithExplicitContentLengthZeroWillBePassedAsI
         $sender->send($request);
     }
 
-    /** @test */
-    public function getHttp10RequestShouldSendAGetRequestWithoutConnectionHeaderByDefault()
-    {
-        $client = $this->getMockBuilder('React\Http\Client\Client')->disableOriginalConstructor()->getMock();
-        $client->expects($this->once())->method('request')->with($this->callback(function (RequestInterface $request) {
-            return !$request->hasHeader('Connection');
-        }))->willReturn($this->getMockBuilder('React\Http\Io\ClientRequestStream')->disableOriginalConstructor()->getMock());
-
-        $sender = new Sender($client);
-
-        $request = new Request('GET', 'http://www.example.com', array(), '', '1.0');
-        $sender->send($request);
-    }
-
-    /** @test */
-    public function getHttp10RequestShouldSendAGetRequestWithoutConnectionHeaderEvenWhenConnectionKeepAliveHeaderIsSpecified()
-    {
-        $client = $this->getMockBuilder('React\Http\Client\Client')->disableOriginalConstructor()->getMock();
-        $client->expects($this->once())->method('request')->with($this->callback(function (RequestInterface $request) {
-            return !$request->hasHeader('Connection');
-        }))->willReturn($this->getMockBuilder('React\Http\Io\ClientRequestStream')->disableOriginalConstructor()->getMock());
-
-        $sender = new Sender($client);
-
-        $request = new Request('GET', 'http://www.example.com', array('Connection' => 'keep-alive'), '', '1.0');
-        $sender->send($request);
-    }
-
-    /** @test */
-    public function getHttp11RequestShouldSendAGetRequestWithConnectionCloseHeaderByDefault()
-    {
-        $client = $this->getMockBuilder('React\Http\Client\Client')->disableOriginalConstructor()->getMock();
-        $client->expects($this->once())->method('request')->with($this->callback(function (RequestInterface $request) {
-            return $request->getHeaderLine('Connection') === 'close';
-        }))->willReturn($this->getMockBuilder('React\Http\Io\ClientRequestStream')->disableOriginalConstructor()->getMock());
-
-        $sender = new Sender($client);
-
-        $request = new Request('GET', 'http://www.example.com', array(), '', '1.1');
-        $sender->send($request);
-    }
-
-    /** @test */
-    public function getHttp11RequestShouldSendAGetRequestWithConnectionCloseHeaderEvenWhenConnectionKeepAliveHeaderIsSpecified()
-    {
-        $client = $this->getMockBuilder('React\Http\Client\Client')->disableOriginalConstructor()->getMock();
-        $client->expects($this->once())->method('request')->with($this->callback(function (RequestInterface $request) {
-            return $request->getHeaderLine('Connection') === 'close';
-        }))->willReturn($this->getMockBuilder('React\Http\Io\ClientRequestStream')->disableOriginalConstructor()->getMock());
-
-        $sender = new Sender($client);
-
-        $request = new Request('GET', 'http://www.example.com', array('Connection' => 'keep-alive'), '', '1.1');
-        $sender->send($request);
-    }
-
     /** @test */
     public function getRequestWithUserAndPassShouldSendAGetRequestWithBasicAuthorizationHeader()
     {