Merge pull request #130 from clue-labs/redirect-location

Only try to follow redirects if Location response header is present
clue · Apr 3, 2019 · 02eee38 · 02eee38
2 parents 06f2be4 + f3e39eb
commit 02eee38
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 1 deletion.
diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php
@@ -165,7 +165,9 @@ public function onResponse(ResponseInterface $response, RequestInterface $reques
     {
         $this->progress('response', array($response, $request));
 
-        if ($this->followRedirects && ($response->getStatusCode() >= 300 && $response->getStatusCode() < 400)) {
+        // follow 3xx (Redirection) response status codes if Location header is present and not explicitly disabled
+        // @link https://tools.ietf.org/html/rfc7231#section-6.4
+        if ($this->followRedirects && ($response->getStatusCode() >= 300 && $response->getStatusCode() < 400) && $response->hasHeader('Location')) {
             return $this->onResponseRedirect($response, $request, $deferred);
         }
 

diff --git a/tests/FunctionalBrowserTest.php b/tests/FunctionalBrowserTest.php
@@ -175,6 +175,15 @@ public function testRejectingRedirectsRejects()
         Block\await($browser->get($this->base . 'redirect/3'), $this->loop);
     }
 
+    /**
+     * @group online
+     * @doesNotPerformAssertions
+     */
+    public function testResponseStatus300WithoutLocationShouldResolveWithoutFollowingRedirect()
+    {
+        Block\await($this->browser->get($this->base . 'status/300'), $this->loop);
+    }
+
     /**
      * @group online
      * @doesNotPerformAssertions

diff --git a/tests/Io/TransactionTest.php b/tests/Io/TransactionTest.php
@@ -157,6 +157,42 @@ public function testReceivingStreamingBodyWillResolveWithStreamingResponseIfStre
         $this->assertEquals('', (string)$response->getBody());
     }
 
+    public function testResponseCode304WithoutLocationWillResolveWithResponseAsIs()
+    {
+        $messageFactory = new MessageFactory();
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+
+        // conditional GET request will respond with 304 (Not Modified
+        $request = $messageFactory->request('GET', 'http://example.com', array('If-None-Match' => '"abc"'));
+        $response = $messageFactory->response(1.0, 304, null, array('ETag' => '"abc"'));
+        $sender = $this->makeSenderMock();
+        $sender->expects($this->once())->method('send')->with($request)->willReturn(Promise\resolve($response));
+
+        $transaction = new Transaction($sender, $messageFactory, $loop);
+        $promise = $transaction->send($request);
+
+        $promise->then($this->expectCallableOnceWith($response));
+    }
+
+    public function testCustomRedirectResponseCode333WillFollowLocationHeaderAndSendRedirectedRequest()
+    {
+        $messageFactory = new MessageFactory();
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+
+        // original GET request will respond with custom 333 redirect status code and follow location header
+        $requestOriginal = $messageFactory->request('GET', 'http://example.com');
+        $response = $messageFactory->response(1.0, 333, null, array('Location' => 'foo'));
+        $requestRedirected = $messageFactory->request('GET', 'http://example.com/foo');
+        $sender = $this->makeSenderMock();
+        $sender->expects($this->exactly(2))->method('send')->withConsecutive($requestOriginal, $requestRedirected)->willReturnOnConsecutiveCalls(
+            Promise\resolve($response),
+            new \React\Promise\Promise(function () { })
+        );
+
+        $transaction = new Transaction($sender, $messageFactory, $loop);
+        $transaction->send($requestOriginal);
+    }
+
     public function testFollowingRedirectWithSpecifiedHeaders()
     {
         $messageFactory = new MessageFactory();
@@ -544,6 +580,17 @@ protected function expectCallableOnce()
         return $mock;
     }
 
+    protected function expectCallableOnceWith($value)
+    {
+        $mock = $this->createCallableMock();
+        $mock
+            ->expects($this->once())
+            ->method('__invoke')
+            ->with($value);
+
+        return $mock;
+    }
+
     protected function createCallableMock()
     {
         return $this->getMockBuilder('stdClass')->setMethods(array('__invoke'))->getMock();