From c8bc9533e5c839fa5244144de9e2fcd9e070b58e Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Tue, 23 Apr 2013 12:49:58 +0200 Subject: [PATCH 1/2] Add max redirections limit --- src/Symfony/Component/BrowserKit/Client.php | 20 +++++++++++++++++++ .../Component/BrowserKit/Tests/ClientTest.php | 19 ++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/Symfony/Component/BrowserKit/Client.php b/src/Symfony/Component/BrowserKit/Client.php index 39945740b0..824e4e4774 100644 --- a/src/Symfony/Component/BrowserKit/Client.php +++ b/src/Symfony/Component/BrowserKit/Client.php @@ -41,6 +41,8 @@ abstract class Client protected $insulated; protected $redirect; protected $followRedirects; + protected $maxRedirects = -1; + protected $redirectCount = 0; /** * Constructor. @@ -71,6 +73,16 @@ abstract class Client { $this->followRedirects = (Boolean) $followRedirect; } + + /** + * Sets the maximum number of requests that crawler can follow. + * + * @param integer $maxRedirects + */ + public function setMaxRedirects($maxRedirects) + { + $this->maxRedirects = $maxRedirects < 0 ? -1 : $maxRedirects; + } /** * Sets the insulated flag. @@ -455,6 +467,14 @@ abstract class Client if (empty($this->redirect)) { throw new \LogicException('The request was not redirected.'); } + + if (-1 !== $this->maxRedirects) { + if ($this->redirectCount === $this->maxRedirects) { + throw new \LogicException(sprintf('The maximum number (%d) of redirections was reached.', $this->maxRedirects)); + } + + ++$this->redirectCount; + } return $this->request('get', $this->redirect); } diff --git a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php index 82322acc5c..257875d651 100644 --- a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php @@ -349,6 +349,25 @@ class ClientTest extends \PHPUnit_Framework_TestCase $this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect() automatically follows redirects if followRedirects is true'); } + + public function testFollowRedirectWithMaxRedirects() + { + $client = new TestClient(); + $client->setMaxRedirects(1); + $client->setNextResponse(new Response('', 302, array('Location' => 'http://www.example.com/redirected'))); + $client->request('GET', 'http://www.example.com/foo/foobar'); + + $this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect() follows a redirect if any'); + + $client->setNextResponse(new Response('', 302, array('Location' => 'http://www.example.com/redirected2'))); + + try { + $client->followRedirect(); + $this->fail('->followRedirect() throws a \LogicException if the request was redirected and limit of redirections were reached'); + } catch (\Exception $e) { + $this->assertInstanceof('LogicException', $e, '->followRedirect() throws a \LogicException if the request was redirected and limit of redirections were reached'); + } + } public function testFollowRedirectWithCookies() { From b60290a7c8201d178555495b73a3ed6f9cf85052 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 25 Apr 2013 12:35:08 +0200 Subject: [PATCH 2/2] [BrowserKit] isolated the max redirect count to a given main request (refs #7811) --- src/Symfony/Component/BrowserKit/Client.php | 34 ++++++++++++++----- .../Component/BrowserKit/Tests/ClientTest.php | 12 ++++--- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/BrowserKit/Client.php b/src/Symfony/Component/BrowserKit/Client.php index 824e4e4774..825cd42a05 100644 --- a/src/Symfony/Component/BrowserKit/Client.php +++ b/src/Symfony/Component/BrowserKit/Client.php @@ -41,8 +41,10 @@ abstract class Client protected $insulated; protected $redirect; protected $followRedirects; - protected $maxRedirects = -1; - protected $redirectCount = 0; + + private $maxRedirects; + private $redirectCount; + private $isMainRequest; /** * Constructor. @@ -60,6 +62,9 @@ abstract class Client $this->cookieJar = null === $cookieJar ? new CookieJar() : $cookieJar; $this->insulated = false; $this->followRedirects = true; + $this->maxRedirects = -1; + $this->redirectCount = 0; + $this->isMainRequest = true; } /** @@ -76,12 +81,13 @@ abstract class Client /** * Sets the maximum number of requests that crawler can follow. - * - * @param integer $maxRedirects + * + * @param integer $maxRedirects */ public function setMaxRedirects($maxRedirects) { $this->maxRedirects = $maxRedirects < 0 ? -1 : $maxRedirects; + $this->followRedirects = -1 != $this->maxRedirects; } /** @@ -289,6 +295,12 @@ abstract class Client */ public function request($method, $uri, array $parameters = array(), array $files = array(), array $server = array(), $content = null, $changeHistory = true) { + if ($this->isMainRequest) { + $this->redirectCount = 0; + } else { + ++$this->redirectCount; + } + $uri = $this->getAbsoluteUri($uri); $server = array_merge($this->server, $server); @@ -467,16 +479,20 @@ abstract class Client if (empty($this->redirect)) { throw new \LogicException('The request was not redirected.'); } - + if (-1 !== $this->maxRedirects) { - if ($this->redirectCount === $this->maxRedirects) { + if ($this->redirectCount > $this->maxRedirects) { throw new \LogicException(sprintf('The maximum number (%d) of redirections was reached.', $this->maxRedirects)); } - - ++$this->redirectCount; } - return $this->request('get', $this->redirect); + $this->isMainRequest = false; + + $response = $this->request('get', $this->redirect); + + $this->isMainRequest = true; + + return $response; } /** diff --git a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php index 257875d651..3f96091c12 100644 --- a/src/Symfony/Component/BrowserKit/Tests/ClientTest.php +++ b/src/Symfony/Component/BrowserKit/Tests/ClientTest.php @@ -356,17 +356,19 @@ class ClientTest extends \PHPUnit_Framework_TestCase $client->setMaxRedirects(1); $client->setNextResponse(new Response('', 302, array('Location' => 'http://www.example.com/redirected'))); $client->request('GET', 'http://www.example.com/foo/foobar'); - $this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect() follows a redirect if any'); - + $client->setNextResponse(new Response('', 302, array('Location' => 'http://www.example.com/redirected2'))); - try { $client->followRedirect(); - $this->fail('->followRedirect() throws a \LogicException if the request was redirected and limit of redirections were reached'); + $this->fail('->followRedirect() throws a \LogicException if the request was redirected and limit of redirections was reached'); } catch (\Exception $e) { - $this->assertInstanceof('LogicException', $e, '->followRedirect() throws a \LogicException if the request was redirected and limit of redirections were reached'); + $this->assertInstanceof('LogicException', $e, '->followRedirect() throws a \LogicException if the request was redirected and limit of redirections was reached'); } + + $client->setNextResponse(new Response('', 302, array('Location' => 'http://www.example.com/redirected'))); + $client->request('GET', 'http://www.example.com/foo/foobar'); + $this->assertEquals('http://www.example.com/redirected', $client->getRequest()->getUri(), '->followRedirect() follows a redirect if any'); } public function testFollowRedirectWithCookies()