From 6b3700db7508ff7e41a1ccdce331a5b53ddad61e Mon Sep 17 00:00:00 2001 From: "m.chwedziak" Date: Mon, 13 May 2013 14:57:53 +0200 Subject: [PATCH 1/4] BrowserKit should not redirect if HTTP Code is not 30x --- UPGRADE-2.3.md | 16 +++++++++++++ src/Symfony/Component/BrowserKit/CHANGELOG.md | 4 ++++ src/Symfony/Component/BrowserKit/Client.php | 16 +++++++++---- .../Component/BrowserKit/Tests/ClientTest.php | 24 +++++++++++++++++++ 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/UPGRADE-2.3.md b/UPGRADE-2.3.md index 9bb123e32afd7..8d3ab0505b750 100644 --- a/UPGRADE-2.3.md +++ b/UPGRADE-2.3.md @@ -263,3 +263,19 @@ Console ``` if (OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) { ... } ``` + +BrowserKit +---------- + + * If you are receiving responses with non-30x Status Code and Location header + please be aware that you won't be able to use auto-redirects on these kind + of responses. To force redirection even if the response is not HTTP 30x, + call `Client::followRedirect(true)` to force-redirect if `Location` header + is present. + + If you are correctly passing 30x Status Code with Location header, you + don't have to worry about the change. + + If you were using responses with Location header and non-30x Status Code, + you have to update your code to forcely follow the redirect with + `Client::followRedirect(true)`. \ No newline at end of file diff --git a/src/Symfony/Component/BrowserKit/CHANGELOG.md b/src/Symfony/Component/BrowserKit/CHANGELOG.md index f67099bdbe981..f1d36e2b59760 100644 --- 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-30x 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 index 825cd42a052ba..d105649d2f64b 100644 --- a/src/Symfony/Component/BrowserKit/Client.php +++ b/src/Symfony/Component/BrowserKit/Client.php @@ -328,7 +328,10 @@ 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(); + $location = $this->internalResponse->getHeader('Location'); + + $this->redirect = $status >= 300 && $status < 400 ? $location : null; if ($this->followRedirects && $this->redirect) { return $this->crawler = $this->followRedirect(); @@ -470,13 +473,15 @@ public function reload() * * @return Crawler * + * @param boolean $force Do not check Status Code and follow if Location header present (default: false) + * * @throws \LogicException If request was not a redirect * * @api */ - public function followRedirect() + public function followRedirect($force = false) { - if (empty($this->redirect)) { + if (!$force && empty($this->redirect)) { throw new \LogicException('The request was not redirected.'); } @@ -488,7 +493,10 @@ public function followRedirect() $this->isMainRequest = false; - $response = $this->request('get', $this->redirect); + $response = $this->request('get', $force + ? $this->internalResponse->getHeader('Location') + : $this->redirect + ); $this->isMainRequest = true; diff --git a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php index 3f96091c12903..6fb4db2d96149 100644 --- a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php @@ -348,6 +348,30 @@ 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'); + } + + $client->setNextResponse(new Response('', 201, array('Location' => 'http://www.example.com/redirected'))); + $client->request('GET', 'http://www.example.com/foo/foobar'); + $client->followRedirect(true); + + $this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect(true) follows a redirect even if HTTP Code differs from 30x'); } public function testFollowRedirectWithMaxRedirects() From 6a7e3bd19253682a6f4587056538503b6c6852cd Mon Sep 17 00:00:00 2001 From: "m.chwedziak" Date: Mon, 13 May 2013 17:08:03 +0200 Subject: [PATCH 2/4] removed new argument from followRedirect --- UPGRADE-2.3.md | 8 +++----- src/Symfony/Component/BrowserKit/Client.php | 11 +++-------- src/Symfony/Component/BrowserKit/Tests/ClientTest.php | 6 ------ 3 files changed, 6 insertions(+), 19 deletions(-) diff --git a/UPGRADE-2.3.md b/UPGRADE-2.3.md index 8d3ab0505b750..9c0795111d442 100644 --- a/UPGRADE-2.3.md +++ b/UPGRADE-2.3.md @@ -269,13 +269,11 @@ BrowserKit * If you are receiving responses with non-30x Status Code and Location header please be aware that you won't be able to use auto-redirects on these kind - of responses. To force redirection even if the response is not HTTP 30x, - call `Client::followRedirect(true)` to force-redirect if `Location` header - is present. + of responses. If you are correctly passing 30x Status Code with Location header, you don't have to worry about the change. If you were using responses with Location header and non-30x Status Code, - you have to update your code to forcely follow the redirect with - `Client::followRedirect(true)`. \ No newline at end of file + you have to update your code to manually create another request to URL + grabbed from the Location header. \ No newline at end of file diff --git a/src/Symfony/Component/BrowserKit/Client.php b/src/Symfony/Component/BrowserKit/Client.php index d105649d2f64b..1e5913522f155 100644 --- a/src/Symfony/Component/BrowserKit/Client.php +++ b/src/Symfony/Component/BrowserKit/Client.php @@ -473,15 +473,13 @@ public function reload() * * @return Crawler * - * @param boolean $force Do not check Status Code and follow if Location header present (default: false) - * * @throws \LogicException If request was not a redirect * * @api */ - public function followRedirect($force = false) + public function followRedirect() { - if (!$force && empty($this->redirect)) { + if (empty($this->redirect)) { throw new \LogicException('The request was not redirected.'); } @@ -493,10 +491,7 @@ public function followRedirect($force = false) $this->isMainRequest = false; - $response = $this->request('get', $force - ? $this->internalResponse->getHeader('Location') - : $this->redirect - ); + $response = $this->request('get', $this->redirect); $this->isMainRequest = true; diff --git a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php index 6fb4db2d96149..d0609a64a33d0 100644 --- a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php @@ -366,12 +366,6 @@ public function testFollowRedirect() } catch (\Exception $e) { $this->assertInstanceof('LogicException', $e, '->followRedirect() throws a \LogicException if the request did not respond with 30x HTTP Code'); } - - $client->setNextResponse(new Response('', 201, array('Location' => 'http://www.example.com/redirected'))); - $client->request('GET', 'http://www.example.com/foo/foobar'); - $client->followRedirect(true); - - $this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect(true) follows a redirect even if HTTP Code differs from 30x'); } public function testFollowRedirectWithMaxRedirects() From 00c53148ca4a960774fe431e375bfb7fd0a3daeb Mon Sep 17 00:00:00 2001 From: Marcin Chwedziak Date: Mon, 13 May 2013 19:43:00 +0200 Subject: [PATCH 3/4] no call to getHeader if not needed --- src/Symfony/Component/BrowserKit/Client.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/BrowserKit/Client.php b/src/Symfony/Component/BrowserKit/Client.php index 1e5913522f155..dcffdd6e132d6 100644 --- a/src/Symfony/Component/BrowserKit/Client.php +++ b/src/Symfony/Component/BrowserKit/Client.php @@ -329,9 +329,12 @@ public function request($method, $uri, array $parameters = array(), array $files $this->cookieJar->updateFromResponse($this->internalResponse, $uri); $status = $this->internalResponse->getStatus(); - $location = $this->internalResponse->getHeader('Location'); - - $this->redirect = $status >= 300 && $status < 400 ? $location : null; + + 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(); From c6fc508249bc25b6b7735a94735caa736e27a1f6 Mon Sep 17 00:00:00 2001 From: Marcin Chwedziak Date: Mon, 13 May 2013 22:56:53 +0200 Subject: [PATCH 4/4] updated upgrade note: 30x -> 3xx --- UPGRADE-2.3.md | 6 +++--- src/Symfony/Component/BrowserKit/CHANGELOG.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/UPGRADE-2.3.md b/UPGRADE-2.3.md index 9c0795111d442..02b36e66f2415 100644 --- a/UPGRADE-2.3.md +++ b/UPGRADE-2.3.md @@ -267,13 +267,13 @@ Console BrowserKit ---------- - * If you are receiving responses with non-30x Status Code and Location header + * 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 30x Status Code with Location header, you + 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-30x Status Code, + 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. \ No newline at end of file diff --git a/src/Symfony/Component/BrowserKit/CHANGELOG.md b/src/Symfony/Component/BrowserKit/CHANGELOG.md index f1d36e2b59760..d2b1074029fd1 100644 --- a/src/Symfony/Component/BrowserKit/CHANGELOG.md +++ b/src/Symfony/Component/BrowserKit/CHANGELOG.md @@ -5,7 +5,7 @@ CHANGELOG ----- * [BC BREAK] `Client::followRedirect()` won't redirect responses with - a non-30x Status Code and `Location` header anymore, as per + 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