merged branch tiraeth/master-browserkit-redirect (PR #8025)

fabpot · fabpot · commit da6f190a6b63 · 2013-05-15T17:16:47.000+02:00
This PR was squashed before being merged into the master branch (closes #8025). Discussion ---------- [BrowserKit] should not follow redirects if status code is not 30x | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Currently, BrowserKit operates incorrectly. It follows "redirect" when `Location` header is present, but having just the header is not enough to perform redirection. [RFC-2616](http://tools.ietf.org/html/rfc2616#section-14.30) precisely says that the redirection should be performed only with `30x` status codes. This PR fixes the incorrect behaviour of BrowserKit and make it consist with both the RFC document and with other clients, used for example with Behat. I've found the issue while testing my application with Behat. I was returning `Location` header with `HTTP 201/Created` status code and was surprised that BrowserKit follows the redirection. This PR is for 2.3 version (master) of Symfony. Commits ------- 8f54da7 [BrowserKit] should not follow redirects if status code is not 30x
diff --git a/UPGRADE-2.3.md b/UPGRADE-2.3.md
@@ -263,3 +263,17 @@ Console
    ```
    if (OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) { ... }
    ```
+
+BrowserKit
+----------
+
+ * If you are receiving responses with non-3xx Status Code and Location header
+   please be aware that you won't be able to use auto-redirects on these kind
+   of responses.
+
+   If you are correctly passing 3xx Status Code with Location header, you
+   don't have to worry about the change.
+
+   If you were using responses with Location header and non-3xx Status Code,
+   you have to update your code to manually create another request to URL
+   grabbed from the Location header.
diff --git a/src/Symfony/Component/BrowserKit/CHANGELOG.md b/src/Symfony/Component/BrowserKit/CHANGELOG.md
@@ -4,6 +4,10 @@ CHANGELOG
 2.3.0
 -----
 
+ * [BC BREAK] `Client::followRedirect()` won't redirect responses with
+   a non-3xx Status Code and `Location` header anymore, as per 
+   http://tools.ietf.org/html/rfc2616#section-14.30
+
  * added `Client::getInternalRequest()` and `Client::getInternalResponse()` to
    have access to the BrowserKit internal request and response objects
 
diff --git a/src/Symfony/Component/BrowserKit/Client.php b/src/Symfony/Component/BrowserKit/Client.php
@@ -328,7 +328,13 @@ public function request($method, $uri, array $parameters = array(), array $files
 
         $this->cookieJar->updateFromResponse($this->internalResponse, $uri);
 
-        $this->redirect = $this->internalResponse->getHeader('Location');
+        $status = $this->internalResponse->getStatus();
+        
+        if ($status >= 300 && $status < 400) {
+            $this->redirect = $this->internalResponse->getHeader('Location');
+        } else {
+            $this->redirect = null;
+        }
 
         if ($this->followRedirects && $this->redirect) {
             return $this->crawler = $this->followRedirect();
diff --git a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php
@@ -348,6 +348,24 @@ public function testFollowRedirect()
         $client->request('GET', 'http://www.example.com/foo/foobar');
 
         $this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect() automatically follows redirects if followRedirects is true');
+
+        $client = new TestClient();
+        $client->setNextResponse(new Response('', 201, array('Location' => 'http://www.example.com/redirected')));
+        $client->request('GET', 'http://www.example.com/foo/foobar');
+
+        $this->assertEquals('http://www.example.com/foo/foobar', $client->getRequest()->getUri(), '->followRedirect() does not follow redirect if HTTP Code is not 30x');
+
+        $client = new TestClient();
+        $client->setNextResponse(new Response('', 201, array('Location' => 'http://www.example.com/redirected')));
+        $client->followRedirects(false);
+        $client->request('GET', 'http://www.example.com/foo/foobar');
+
+        try {
+            $client->followRedirect();
+            $this->fail('->followRedirect() throws a \LogicException if the request did not respond with 30x HTTP Code');
+        } catch (\Exception $e) {
+            $this->assertInstanceof('LogicException', $e, '->followRedirect() throws a \LogicException if the request did not respond with 30x HTTP Code');
+        }
     }
     
     public function testFollowRedirectWithMaxRedirects()
