Merge pull request #91 from seregazhuk/master

Pass custom request headers when following redirects
clue · Feb 9, 2018 · f56a136 · f56a136
2 parents 7971b71 + 1f3466f
commit f56a136
Show file tree

Hide file tree

Showing 2 changed files with 153 additions and 8 deletions.
diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php
@@ -8,6 +8,7 @@
 use Clue\React\Buzz\Message\MessageFactory;
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\UriInterface;
 use React\Promise;
 use React\Promise\Stream;
 use React\Stream\ReadableStreamInterface;
@@ -131,10 +132,7 @@ private function onResponseRedirect(ResponseInterface $response, RequestInterfac
         // resolve location relative to last request URI
         $location = $this->messageFactory->uriRelative($request->getUri(), $response->getHeaderLine('Location'));
 
-        // naïve approach..
-        $method = ($request->getMethod() === 'HEAD') ? 'HEAD' : 'GET';
-        $request = $this->messageFactory->request($method, $location);
-
+        $request = $this->makeRedirectRequest($request, $location);
         $this->progress('redirect', array($request));
 
         if ($this->numRequests >= $this->maxRedirects) {
@@ -144,6 +142,30 @@ private function onResponseRedirect(ResponseInterface $response, RequestInterfac
         return $this->next($request);
     }
 
+    /**
+     * @param RequestInterface $request
+     * @param UriInterface $location
+     * @return \Clue\React\Buzz\Message\RequestInterface
+     */
+    private function makeRedirectRequest(RequestInterface $request, UriInterface $location)
+    {
+        $originalHost = $request->getUri()->getHost();
+        $request = $request
+            ->withoutHeader('Host')
+            ->withoutHeader('Content-Type')
+            ->withoutHeader('Content-Length');
+
+        // Remove authorization if changing hostnames (but not if just changing ports or protocols).
+        if ($location->getHost() !== $originalHost) {
+            $request = $request->withoutHeader('Authentication');
+        }
+
+        // naïve approach..
+        $method = ($request->getMethod() === 'HEAD') ? 'HEAD' : 'GET';
+
+        return $this->messageFactory->request($method, $location, $request->getHeaders());
+    }
+
     private function progress($name, array $args = array())
     {
         return;

diff --git a/tests/Io/TransactionTest.php b/tests/Io/TransactionTest.php
@@ -2,6 +2,8 @@
 
 use Clue\React\Buzz\Io\Transaction;
 use Clue\React\Buzz\Message\ResponseException;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Http\Message\RequestInterface;
 use RingCentral\Psr7\Response;
 use Clue\React\Buzz\Message\MessageFactory;
 use React\Promise;
@@ -17,7 +19,7 @@ public function testReceivingErrorResponseWillRejectWithResponseException()
         $response = new Response(404);
 
         // mock sender to resolve promise with the given $response in response to the given $request
-        $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock();
+        $sender = $this->makeSenderMock();
         $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response));
 
         $transaction = new Transaction($request, $sender, array(), new MessageFactory());
@@ -47,7 +49,7 @@ public function testReceivingStreamingBodyWillResolveWithBufferedResponseByDefau
         $response = $messageFactory->response(1.0, 200, 'OK', array(), $stream);
 
         // mock sender to resolve promise with the given $response in response to the given $request
-        $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock();
+        $sender = $this->makeSenderMock();
         $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response));
 
         $transaction = new Transaction($request, $sender, array(), $messageFactory);
@@ -75,7 +77,7 @@ public function testCancelBufferingResponseWillCloseStreamAndReject()
         $response = $messageFactory->response(1.0, 200, 'OK', array(), $stream);
 
         // mock sender to resolve promise with the given $response in response to the given $request
-        $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock();
+        $sender = $this->makeSenderMock();
         $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response));
 
         $transaction = new Transaction($request, $sender, array(), $messageFactory);
@@ -93,7 +95,7 @@ public function testReceivingStreamingBodyWillResolveWithStreamingResponseIfStre
         $response = $messageFactory->response(1.0, 200, 'OK', array(), $this->getMockBuilder('React\Stream\ReadableStreamInterface')->getMock());
 
         // mock sender to resolve promise with the given $response in response to the given $request
-        $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock();
+        $sender = $this->makeSenderMock();
         $sender->expects($this->once())->method('send')->with($this->equalTo($request))->willReturn(Promise\resolve($response));
 
         $transaction = new Transaction($request, $sender, array('streaming' => true), $messageFactory);
@@ -104,4 +106,125 @@ public function testReceivingStreamingBodyWillResolveWithStreamingResponseIfStre
         $this->assertEquals(200, $response->getStatusCode());
         $this->assertEquals('', (string)$response->getBody());
     }
+
+    public function testFollowingRedirectWithSpecifiedHeaders()
+    {
+        $messageFactory = new MessageFactory();
+
+        $customHeaders = ['User-Agent' => 'Chrome'];
+        $requestWithUserAgent = $messageFactory->request('GET', 'http://example.com', $customHeaders);
+        $sender = $this->makeSenderMock();
+
+        // mock sender to resolve promise with the given $redirectResponse in
+        // response to the given $requestWithUserAgent
+        $redirectResponse = $messageFactory->response(1.0, 301, null, ['Location' => 'http://redirect.com']);
+        $sender->expects($this->at(0))->method('send')->willReturn(Promise\resolve($redirectResponse));
+
+        // mock sender to resolve promise with the given $okResponse in
+        // response to the given $requestWithUserAgent
+        $okResponse = $messageFactory->response(1.0, 200, 'OK');
+        $sender->expects($this->at(1))
+            ->method('send')
+            ->with($this->callback(function (RequestInterface $request) {
+                $this->assertEquals(['Chrome'], $request->getHeader('User-Agent'));
+                return true;
+            }))->willReturn(Promise\resolve($okResponse));
+
+        $transaction = new Transaction($requestWithUserAgent, $sender, [], $messageFactory);
+        $transaction->send();
+    }
+
+    public function testRemovingAuthorizationHeaderWhenChangingHostnamesDuringRedirect()
+    {
+        $messageFactory = new MessageFactory();
+
+        $customHeaders = ['Authentication' => 'secret'];
+        $requestWithAuthentication = $messageFactory->request('GET', 'http://example.com', $customHeaders);
+        $sender = $this->makeSenderMock();
+
+        // mock sender to resolve promise with the given $redirectResponse in
+        // response to the given $requestWithAuthentication
+        $redirectResponse = $messageFactory->response(1.0, 301, null, ['Location' => 'http://redirect.com']);
+        $sender->expects($this->at(0))->method('send')->willReturn(Promise\resolve($redirectResponse));
+
+        // mock sender to resolve promise with the given $okResponse in
+        // response to the given $requestWithAuthentication
+        $okResponse = $messageFactory->response(1.0, 200, 'OK');
+        $sender->expects($this->at(1))
+            ->method('send')
+            ->with($this->callback(function (RequestInterface $request) {
+                $this->assertFalse($request->hasHeader('Authentication'));
+                return true;
+            }))->willReturn(Promise\resolve($okResponse));
+
+        $transaction = new Transaction($requestWithAuthentication, $sender, [], $messageFactory);
+        $transaction->send();
+    }
+
+    public function testAuthorizationHeaderIsForwardedWhenRedirectingToSameDomain()
+    {
+        $messageFactory = new MessageFactory();
+
+        $customHeaders = ['Authentication' => 'secret'];
+        $requestWithAuthentication = $messageFactory->request('GET', 'http://example.com', $customHeaders);
+        $sender = $this->makeSenderMock();
+
+        // mock sender to resolve promise with the given $redirectResponse in
+        // response to the given $requestWithAuthentication
+        $redirectResponse = $messageFactory->response(1.0, 301, null, ['Location' => 'http://example.com/new']);
+        $sender->expects($this->at(0))->method('send')->willReturn(Promise\resolve($redirectResponse));
+
+        // mock sender to resolve promise with the given $okResponse in
+        // response to the given $requestWithAuthentication
+        $okResponse = $messageFactory->response(1.0, 200, 'OK');
+        $sender->expects($this->at(1))
+            ->method('send')
+            ->with($this->callback(function (RequestInterface $request) {
+                $this->assertEquals(['secret'], $request->getHeader('Authentication'));
+                return true;
+            }))->willReturn(Promise\resolve($okResponse));
+
+        $transaction = new Transaction($requestWithAuthentication, $sender, [], $messageFactory);
+        $transaction->send();
+    }
+
+    public function testSomeRequestHeadersShouldBeRemovedWhenRedirecting()
+    {
+        $messageFactory = new MessageFactory();
+
+        $customHeaders = [
+            'Content-Type' => 'text/html; charset=utf-8',
+            'Content-Length' => '111',
+        ];
+
+        $requestWithCustomHeaders = $messageFactory->request('GET', 'http://example.com', $customHeaders);
+        $sender = $this->makeSenderMock();
+
+        // mock sender to resolve promise with the given $redirectResponse in
+        // response to the given $requestWithCustomHeaders
+        $redirectResponse = $messageFactory->response(1.0, 301, null, ['Location' => 'http://example.com/new']);
+        $sender->expects($this->at(0))->method('send')->willReturn(Promise\resolve($redirectResponse));
+
+        // mock sender to resolve promise with the given $okResponse in
+        // response to the given $requestWithCustomHeaders
+        $okResponse = $messageFactory->response(1.0, 200, 'OK');
+        $sender->expects($this->at(1))
+            ->method('send')
+            ->with($this->callback(function (RequestInterface $request) {
+                $this->assertFalse($request->hasHeader('Content-Type'));
+                $this->assertFalse($request->hasHeader('Content-Length'));
+                return true;
+            }))->willReturn(Promise\resolve($okResponse));
+
+        $transaction = new Transaction($requestWithCustomHeaders, $sender, [], $messageFactory);
+        $transaction->send();
+    }
+
+    /**
+     * @return MockObject
+     */
+    private function makeSenderMock()
+    {
+        return $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock();
+    }
 }