diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch fabpot/cookie (PR #7738) 

This PR was merged into the 2.1 branch.

Discussion
----------

[BrowserKit] fixed detection of secure cookies received over https

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7666, #7738
| License       | MIT
| Doc PR        | N/A

As reported in symfony/symfony#7666, BrowserKit's cookie handling only recognises a secure cookie if the cookie option is set and the cookie was set over an https request. The client was not passing the url into the cookiejar update code, causing Cookie::isSecure() to always return false for every cookie.

I have corrected this behaviour and added an additional unit test to prove the bug and fix.

Commits
-------

36d057b [HttpFoundation][BrowserKit] fixed path when converting a cookie to a string
c884151 [BrowserKit] removed dead code
495d0e3 [HttpFoundation] fixed empty domain= in Cookie::__toString()
c2bc707 fixed detection of secure cookies received over https
This commit is contained in:
Fabien Potencier 2013-04-20 20:17:49 +02:00
parent 1454af7235 36d057b69e
commit 499c9e6fdc
7 changed files with 34 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Symfony/Component/BrowserKit/Client.php
View File

@ -266,7 +266,7 @@ abstract class Client
$response = $this->filterResponse($this->response);
$this->cookieJar->updateFromResponse($response);
$this->cookieJar->updateFromResponse($response, $uri);
$this->redirect = $response->getHeader('Location');

7
src/Symfony/Component/BrowserKit/Cookie.php
View File

@ -95,7 +95,7 @@ class Cookie
$cookie .= '; domain='.$this->domain;
}
if ('/' !== $this->path) {
if ($this->path) {
$cookie .= '; path='.$this->path;
}
@ -145,10 +145,9 @@ class Cookie
if ((false === $urlParts = parse_url($url)) || !isset($urlParts['host']) || !isset($urlParts['path'])) {
throw new \InvalidArgumentException(sprintf('The URL "%s" is not valid.', $url));
}
$parts = array_merge($urlParts, $parts);
$values['domain'] = $parts['host'];
$values['path'] = substr($parts['path'], 0, strrpos($parts['path'], '/'));
$values['domain'] = $urlParts['host'];
$values['path'] = substr($urlParts['path'], 0, strrpos($urlParts['path'], '/'));
}
foreach ($parts as $part) {

9
src/Symfony/Component/BrowserKit/Tests/ClientTest.php
View File

@ -205,6 +205,15 @@ class ClientTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(array('foo' => 'bar'), $client->getCookieJar()->allValues('http://www.example.com/foo/foobar'), '->request() updates the CookieJar');
}
public function testRequestSecureCookies()
{
$client = new TestClient();
$client->setNextResponse(new Response('<html><a href="/foo">foo</a></html>', 200, array('Set-Cookie' => 'foo=bar; path=/; secure')));
$client->request('GET', 'https://www.example.com/foo/foobar');
$this->assertTrue($client->getCookieJar()->get('foo', '/', 'www.example.com')->isSecure());
}
public function testClick()
{
if (!class_exists('Symfony\Component\DomCrawler\Crawler')) {

24
src/Symfony/Component/BrowserKit/Tests/CookieTest.php
View File

@ -26,14 +26,14 @@ class CookieTest extends \PHPUnit_Framework_TestCase
public function getTestsForToFromString()
{
return array(
array('foo=bar'),
array('foo=bar; path=/'),
array('foo=bar; path=/foo'),
array('foo=bar; domain=google.com'),
array('foo=bar; domain=example.com; secure', 'https://example.com/'),
array('foo=bar; httponly'),
array('foo=bar; domain=google.com; path=/'),
array('foo=bar; domain=example.com; path=/; secure', 'https://example.com/'),
array('foo=bar; path=/; httponly'),
array('foo=bar; domain=google.com; path=/foo; secure; httponly', 'https://google.com/'),
array('foo=bar=baz'),
array('foo=bar%3Dbaz'),
array('foo=bar=baz; path=/'),
array('foo=bar%3Dbaz; path=/'),
);
}
@ -67,17 +67,17 @@ class CookieTest extends \PHPUnit_Framework_TestCase
public function testFromStringWithCapitalization()
{
$this->assertEquals('Foo=Bar', (string) Cookie::fromString('Foo=Bar'));
$this->assertEquals('foo=bar; expires=Fri, 31 Dec 2010 23:59:59 GMT', (string) Cookie::fromString('foo=bar; Expires=Fri, 31 Dec 2010 23:59:59 GMT'));
$this->assertEquals('foo=bar; domain=www.example.org; httponly', (string) Cookie::fromString('foo=bar; DOMAIN=www.example.org; HttpOnly'));
$this->assertEquals('Foo=Bar; path=/', (string) Cookie::fromString('Foo=Bar'));
$this->assertEquals('foo=bar; expires=Fri, 31 Dec 2010 23:59:59 GMT; path=/', (string) Cookie::fromString('foo=bar; Expires=Fri, 31 Dec 2010 23:59:59 GMT'));
$this->assertEquals('foo=bar; domain=www.example.org; path=/; httponly', (string) Cookie::fromString('foo=bar; DOMAIN=www.example.org; HttpOnly'));
}
public function testFromStringWithUrl()
{
$this->assertEquals('foo=bar; domain=www.example.com', (string) Cookie::FromString('foo=bar', 'http://www.example.com/'));
$this->assertEquals('foo=bar; domain=www.example.com; path=/', (string) Cookie::FromString('foo=bar', 'http://www.example.com/'));
$this->assertEquals('foo=bar; domain=www.example.com; path=/foo', (string) Cookie::FromString('foo=bar', 'http://www.example.com/foo/bar'));
$this->assertEquals('foo=bar; domain=www.example.com', (string) Cookie::FromString('foo=bar; path=/', 'http://www.example.com/foo/bar'));
$this->assertEquals('foo=bar; domain=www.myotherexample.com', (string) Cookie::FromString('foo=bar; domain=www.myotherexample.com', 'http://www.example.com/'));
$this->assertEquals('foo=bar; domain=www.example.com; path=/', (string) Cookie::FromString('foo=bar; path=/', 'http://www.example.com/foo/bar'));
$this->assertEquals('foo=bar; domain=www.myotherexample.com; path=/', (string) Cookie::FromString('foo=bar; domain=www.myotherexample.com', 'http://www.example.com/'));
}
public function testFromStringThrowsAnExceptionIfCookieIsNotValid()

4
src/Symfony/Component/HttpFoundation/Cookie.php
View File

@ -91,11 +91,11 @@ class Cookie
}
}
if ('/' !== $this->path) {
if ($this->path) {
$str .= '; path='.$this->path;
}
if (null !== $this->getDomain()) {
if ($this->getDomain()) {
$str .= '; domain='.$this->getDomain();
}

7
src/Symfony/Component/HttpFoundation/Tests/CookieTest.php
View File

@ -117,11 +117,12 @@ class CookieTest extends \PHPUnit_Framework_TestCase
public function testToString()
{
$cookie = new Cookie('foo', 'bar', strtotime('Fri, 20-May-2011 15:25:52 GMT'), '/', '.myfoodomain.com', true);
$this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; domain=.myfoodomain.com; secure; httponly', $cookie->__toString(), '->__toString() returns string representation of the cookie');
$this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; path=/; domain=.myfoodomain.com; secure; httponly', $cookie->__toString(), '->__toString() returns string representation of the cookie');
$cookie = new Cookie('foo', null, 1, '/admin/', '.myfoodomain.com');
$this->assertEquals('foo=deleted; expires=' . gmdate("D, d-M-Y H:i:s T", time()-31536001) . '; path=/admin/; domain=.myfoodomain.com; httponly', $cookie->__toString(), '->__toString() returns string representation of a cleared cookie if value is NULL');
$cookie = new Cookie('foo', 'bar', 0, '/', '');
$this->assertEquals('foo=bar; path=/; httponly', $cookie->__toString());
}
}

6
src/Symfony/Component/HttpFoundation/Tests/ResponseHeaderBagTest.php
View File

@ -69,11 +69,11 @@ class ResponseHeaderBagTest extends \PHPUnit_Framework_TestCase
$bag = new ResponseHeaderBag(array());
$bag->setCookie(new Cookie('foo', 'bar'));
$this->assertContains("Set-Cookie: foo=bar; httponly", explode("\r\n", $bag->__toString()));
$this->assertContains("Set-Cookie: foo=bar; path=/; httponly", explode("\r\n", $bag->__toString()));
$bag->clearCookie('foo');
$this->assertContains("Set-Cookie: foo=deleted; expires=".gmdate("D, d-M-Y H:i:s T", time() - 31536001)."; httponly", explode("\r\n", $bag->__toString()));
$this->assertContains("Set-Cookie: foo=deleted; expires=".gmdate("D, d-M-Y H:i:s T", time() - 31536001)."; path=/; httponly", explode("\r\n", $bag->__toString()));
}
public function testReplace()
@ -113,7 +113,7 @@ class ResponseHeaderBagTest extends \PHPUnit_Framework_TestCase
$this->assertContains("Set-Cookie: foo=bar; path=/path/foo; domain=foo.bar; httponly", $headers);
$this->assertContains("Set-Cookie: foo=bar; path=/path/foo; domain=foo.bar; httponly", $headers);
$this->assertContains("Set-Cookie: foo=bar; path=/path/bar; domain=bar.foo; httponly", $headers);
$this->assertContains("Set-Cookie: foo=bar; httponly", $headers);
$this->assertContains("Set-Cookie: foo=bar; path=/; httponly", $headers);
$cookies = $bag->getCookies(ResponseHeaderBag::COOKIES_ARRAY);
$this->assertTrue(isset($cookies['foo.bar']['/path/foo']['foo']));