From f08eeb443345237ac4629d1aabfd755fcc2ef218 Mon Sep 17 00:00:00 2001 From: Francis Besset Date: Mon, 4 Jul 2011 20:46:58 +0200 Subject: [PATCH 1/4] Moved managing cookies of HeaderBag in ResponseHeaderBag By example, a cookie can't be set in a request --- .../Component/HttpFoundation/HeaderBag.php | 63 ------------------- .../HttpFoundation/ResponseHeaderBag.php | 62 ++++++++++++++++++ 2 files changed, 62 insertions(+), 63 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/HeaderBag.php b/src/Symfony/Component/HttpFoundation/HeaderBag.php index fb3c3aaf36..42097fe787 100644 --- a/src/Symfony/Component/HttpFoundation/HeaderBag.php +++ b/src/Symfony/Component/HttpFoundation/HeaderBag.php @@ -19,7 +19,6 @@ namespace Symfony\Component\HttpFoundation; class HeaderBag { protected $headers; - protected $cookies; protected $cacheControl; /** @@ -30,7 +29,6 @@ class HeaderBag public function __construct(array $headers = array()) { $this->cacheControl = array(); - $this->cookies = array(); $this->headers = array(); foreach ($headers as $key => $values) { $this->set($key, $values); @@ -200,67 +198,6 @@ class HeaderBag } } - /** - * Sets a cookie. - * - * @param Cookie $cookie - * @return void - */ - public function setCookie(Cookie $cookie) - { - $this->cookies[$cookie->getName()] = $cookie; - } - - /** - * Removes a cookie from the array, but does not unset it in the browser - * - * @param string $name - * @return void - */ - public function removeCookie($name) - { - unset($this->cookies[$name]); - } - - /** - * Whether the array contains any cookie with this name - * - * @param string $name - * @return Boolean - */ - public function hasCookie($name) - { - return isset($this->cookies[$name]); - } - - /** - * Returns a cookie - * - * @param string $name - * - * @throws \InvalidArgumentException When the cookie does not exist - * - * @return Cookie - */ - public function getCookie($name) - { - if (!$this->hasCookie($name)) { - throw new \InvalidArgumentException(sprintf('There is no cookie with name "%s".', $name)); - } - - return $this->cookies[$name]; - } - - /** - * Returns an array with all cookies - * - * @return array - */ - public function getCookies() - { - return $this->cookies; - } - /** * Returns the HTTP header value converted to a date. * diff --git a/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php b/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php index 5f6d408646..7e7de11eef 100644 --- a/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php +++ b/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php @@ -19,6 +19,7 @@ namespace Symfony\Component\HttpFoundation; class ResponseHeaderBag extends HeaderBag { protected $computedCacheControl = array(); + protected $cookies = array(); /** * Constructor. @@ -102,6 +103,67 @@ class ResponseHeaderBag extends HeaderBag return array_key_exists($key, $this->computedCacheControl) ? $this->computedCacheControl[$key] : null; } + /** + * Sets a cookie. + * + * @param Cookie $cookie + * @return void + */ + public function setCookie(Cookie $cookie) + { + $this->cookies[$cookie->getName()] = $cookie; + } + + /** + * Removes a cookie from the array, but does not unset it in the browser + * + * @param string $name + * @return void + */ + public function removeCookie($name) + { + unset($this->cookies[$name]); + } + + /** + * Whether the array contains any cookie with this name + * + * @param string $name + * @return Boolean + */ + public function hasCookie($name) + { + return isset($this->cookies[$name]); + } + + /** + * Returns a cookie + * + * @param string $name + * + * @throws \InvalidArgumentException When the cookie does not exist + * + * @return Cookie + */ + public function getCookie($name) + { + if (!$this->hasCookie($name)) { + throw new \InvalidArgumentException(sprintf('There is no cookie with name "%s".', $name)); + } + + return $this->cookies[$name]; + } + + /** + * Returns an array with all cookies + * + * @return array + */ + public function getCookies() + { + return $this->cookies; + } + /** * Clears a cookie in the browser * From f91f4dda135f8144a6991c0d2dfdfafd4adf63a8 Mon Sep 17 00:00:00 2001 From: Francis Besset Date: Mon, 11 Jul 2011 22:54:08 +0200 Subject: [PATCH 2/4] Added the possibility to set cookies with the same name for different domains and paths for Symfony\Component\HttpFoundation\ResponseHeaderBag ResponseHeaderBag::hasCookie() and ResponseHeaderBag::getCookie() were removed --- .../HttpFoundation/ResponseHeaderBag.php | 69 ++++++++++--------- .../HttpFoundation/ResponseHeaderBagTest.php | 43 ++++++++++++ .../CookieClearingLogoutHandlerTest.php | 10 +-- ...istentTokenBasedRememberMeServicesTest.php | 7 +- .../TokenBasedRememberMeServicesTest.php | 13 ++-- 5 files changed, 100 insertions(+), 42 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php b/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php index 7e7de11eef..69f932cf5b 100644 --- a/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php +++ b/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php @@ -18,6 +18,9 @@ namespace Symfony\Component\HttpFoundation; */ class ResponseHeaderBag extends HeaderBag { + const COOKIES_FLAT = 'flat'; + const COOKIES_ARRAY = 'array'; + protected $computedCacheControl = array(); protected $cookies = array(); @@ -41,7 +44,7 @@ class ResponseHeaderBag extends HeaderBag public function __toString() { $cookies = ''; - foreach ($this->cookies as $cookie) { + foreach ($this->getCookies() as $cookie) { $cookies .= 'Set-Cookie: '.$cookie."\r\n"; } @@ -111,57 +114,59 @@ class ResponseHeaderBag extends HeaderBag */ public function setCookie(Cookie $cookie) { - $this->cookies[$cookie->getName()] = $cookie; + $this->cookies[$cookie->getDomain()][$cookie->getPath()][$cookie->getName()] = $cookie; } /** * Removes a cookie from the array, but does not unset it in the browser * * @param string $name + * @param string $path + * @param string $domain * @return void */ - public function removeCookie($name) + public function removeCookie($name, $path = null, $domain = null) { - unset($this->cookies[$name]); - } + unset($this->cookies[$domain][$path][$name]); - /** - * Whether the array contains any cookie with this name - * - * @param string $name - * @return Boolean - */ - public function hasCookie($name) - { - return isset($this->cookies[$name]); - } + if (empty($this->cookies[$domain][$path])) { + unset($this->cookies[$domain][$path]); - /** - * Returns a cookie - * - * @param string $name - * - * @throws \InvalidArgumentException When the cookie does not exist - * - * @return Cookie - */ - public function getCookie($name) - { - if (!$this->hasCookie($name)) { - throw new \InvalidArgumentException(sprintf('There is no cookie with name "%s".', $name)); + if (empty($this->cookies[$domain])) { + unset($this->cookies[$domain]); + } } - - return $this->cookies[$name]; } /** * Returns an array with all cookies * + * @param string $format + * + * @throws \InvalidArgumentException When the $format is invalid + * * @return array */ - public function getCookies() + public function getCookies($format = 'flat') { - return $this->cookies; + if (!in_array($format, array(static::COOKIES_FLAT, static::COOKIES_ARRAY))) { + throw new \InvalidArgumentException(sprintf('Format "%s" invalid (%s).', $format, implode(', ', array(static::COOKIES_FLAT, static::COOKIES_ARRAY)))); + } + + if (static::COOKIES_ARRAY === $format) { + return $this->cookies; + } + + $return = array(); + foreach ($this->cookies as $path) { + foreach ($path as $cookies) { + foreach ($cookies as $cookie) { + $return[] = $cookie; + } + } + } + + return $return; } /** diff --git a/tests/Symfony/Tests/Component/HttpFoundation/ResponseHeaderBagTest.php b/tests/Symfony/Tests/Component/HttpFoundation/ResponseHeaderBagTest.php index 9670c166d5..a69c966a9a 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/ResponseHeaderBagTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/ResponseHeaderBagTest.php @@ -75,4 +75,47 @@ class ResponseHeaderBagTest extends \PHPUnit_Framework_TestCase $this->assertContains("Set-Cookie: foo=deleted; expires=".gmdate("D, d-M-Y H:i:s T", time() - 31536001)."; httponly", explode("\r\n", $bag->__toString())); } + + public function testCookiesWithSameNames() + { + $bag = new ResponseHeaderBag(); + $bag->setCookie(new Cookie('foo', 'bar', 0, '/path/foo', 'foo.bar')); + $bag->setCookie(new Cookie('foo', 'bar', 0, '/path/bar', 'foo.bar')); + $bag->setCookie(new Cookie('foo', 'bar', 0, '/path/bar', 'bar.foo')); + $bag->setCookie(new Cookie('foo', 'bar')); + + $this->assertEquals(4, count($bag->getCookies())); + + $headers = explode("\r\n", $bag->__toString()); + $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; path=/; httponly", $headers); + + $cookies = $bag->getCookies(ResponseHeaderBag::COOKIES_ARRAY); + $this->assertTrue(isset($cookies['foo.bar']['/path/foo']['foo'])); + $this->assertTrue(isset($cookies['foo.bar']['/path/bar']['foo'])); + $this->assertTrue(isset($cookies['bar.foo']['/path/bar']['foo'])); + $this->assertTrue(isset($cookies['']['/']['foo'])); + } + + public function testRemoveCookie() + { + $bag = new ResponseHeaderBag(); + $bag->setCookie(new Cookie('foo', 'bar', 0, '/path/foo', 'foo.bar')); + $bag->setCookie(new Cookie('bar', 'foo', 0, '/path/bar', 'foo.bar')); + + $cookies = $bag->getCookies(ResponseHeaderBag::COOKIES_ARRAY); + $this->assertTrue(isset($cookies['foo.bar']['/path/foo'])); + + $bag->removeCookie('foo', '/path/foo', 'foo.bar'); + + $cookies = $bag->getCookies(ResponseHeaderBag::COOKIES_ARRAY); + $this->assertFalse(isset($cookies['foo.bar']['/path/foo'])); + + $bag->removeCookie('bar', '/path/bar', 'foo.bar'); + + $cookies = $bag->getCookies(ResponseHeaderBag::COOKIES_ARRAY); + $this->assertFalse(isset($cookies['foo.bar'])); + } } diff --git a/tests/Symfony/Tests/Component/Security/Http/Logout/CookieClearingLogoutHandlerTest.php b/tests/Symfony/Tests/Component/Security/Http/Logout/CookieClearingLogoutHandlerTest.php index 41e548c8e0..9d77f82990 100644 --- a/tests/Symfony/Tests/Component/Security/Http/Logout/CookieClearingLogoutHandlerTest.php +++ b/tests/Symfony/Tests/Component/Security/Http/Logout/CookieClearingLogoutHandlerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Tests\Component\Security\Http\Logout; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\ResponseHeaderBag; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Security\Http\Logout\CookieClearingLogoutHandler; @@ -25,20 +26,21 @@ class CookieClearingLogoutHandlerTest extends \PHPUnit_Framework_TestCase $handler = new CookieClearingLogoutHandler(array('foo' => array('path' => '/foo', 'domain' => 'foo.foo'), 'foo2' => array('path' => null, 'domain' => null))); - $this->assertFalse($response->headers->hasCookie('foo')); + $cookies = $response->headers->getCookies(); + $this->assertEquals(0, count($cookies)); $handler->logout($request, $response, $token); - $cookies = $response->headers->getCookies(); + $cookies = $response->headers->getCookies(ResponseHeaderBag::COOKIES_ARRAY); $this->assertEquals(2, count($cookies)); - $cookie = $cookies['foo']; + $cookie = $cookies['foo.foo']['/foo']['foo']; $this->assertEquals('foo', $cookie->getName()); $this->assertEquals('/foo', $cookie->getPath()); $this->assertEquals('foo.foo', $cookie->getDomain()); $this->assertTrue($cookie->isCleared()); - $cookie = $cookies['foo2']; + $cookie = $cookies['']['']['foo2']; $this->assertStringStartsWith('foo2', $cookie->getName()); $this->assertNull($cookie->getPath()); $this->assertNull($cookie->getDomain()); diff --git a/tests/Symfony/Tests/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php b/tests/Symfony/Tests/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php index d8e647b82b..daa3263717 100644 --- a/tests/Symfony/Tests/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php +++ b/tests/Symfony/Tests/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServicesTest.php @@ -19,6 +19,7 @@ use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentToken; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\ResponseHeaderBag; use Symfony\Component\Security\Http\RememberMe\PersistentTokenBasedRememberMeServices; use Symfony\Component\Security\Core\Exception\TokenNotFoundException; use Symfony\Component\Security\Core\Exception\CookieTheftException; @@ -281,11 +282,13 @@ class PersistentTokenBasedRememberMeServicesTest extends \PHPUnit_Framework_Test ; $service->setTokenProvider($tokenProvider); - $this->assertFalse($response->headers->hasCookie('foo')); + $cookies = $response->headers->getCookies(); + $this->assertEquals(0, count($cookies)); $service->loginSuccess($request, $response, $token); - $cookie = $response->headers->getCookie('foo'); + $cookies = $response->headers->getCookies(ResponseHeaderBag::COOKIES_ARRAY); + $cookie = $cookies['myfoodomain.foo']['/foo/path']['foo']; $this->assertFalse($cookie->isCleared()); $this->assertTrue($cookie->isSecure()); $this->assertTrue($cookie->isHttpOnly()); diff --git a/tests/Symfony/Tests/Component/Security/Http/RememberMe/TokenBasedRememberMeServicesTest.php b/tests/Symfony/Tests/Component/Security/Http/RememberMe/TokenBasedRememberMeServicesTest.php index 220ca80323..4a587dd7fb 100644 --- a/tests/Symfony/Tests/Component/Security/Http/RememberMe/TokenBasedRememberMeServicesTest.php +++ b/tests/Symfony/Tests/Component/Security/Http/RememberMe/TokenBasedRememberMeServicesTest.php @@ -19,6 +19,7 @@ use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentToken; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\ResponseHeaderBag; use Symfony\Component\Security\Http\RememberMe\TokenBasedRememberMeServices; use Symfony\Component\Security\Core\Exception\TokenNotFoundException; use Symfony\Component\Security\Core\Exception\CookieTheftException; @@ -184,11 +185,13 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('foo')) ; - $this->assertFalse($response->headers->hasCookie('foo')); + $cookies = $response->headers->getCookies(); + $this->assertEquals(0, count($cookies)); $service->loginSuccess($request, $response, $token); - $this->assertFalse($response->headers->hasCookie('foo')); + $cookies = $response->headers->getCookies(); + $this->assertEquals(0, count($cookies)); } public function testLoginSuccess() @@ -215,11 +218,13 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue($user)) ; - $this->assertFalse($response->headers->hasCookie('foo')); + $cookies = $response->headers->getCookies(); + $this->assertEquals(0, count($cookies)); $service->loginSuccess($request, $response, $token); - $cookie = $response->headers->getCookie('foo'); + $cookies = $response->headers->getCookies(ResponseHeaderBag::COOKIES_ARRAY); + $cookie = $cookies['myfoodomain.foo']['/foo/path']['foo']; $this->assertFalse($cookie->isCleared()); $this->assertTrue($cookie->isSecure()); $this->assertTrue($cookie->isHttpOnly()); From 7cf891a4483081aaff0a01cf0cf474d9621713e3 Mon Sep 17 00:00:00 2001 From: Francis Besset Date: Wed, 13 Jul 2011 14:10:50 +0200 Subject: [PATCH 3/4] Renamed variable returned and used self in place of static for constants --- .../Component/HttpFoundation/ResponseHeaderBag.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php b/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php index 69f932cf5b..a6fb6a0c14 100644 --- a/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php +++ b/src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php @@ -147,26 +147,26 @@ class ResponseHeaderBag extends HeaderBag * * @return array */ - public function getCookies($format = 'flat') + public function getCookies($format = self::COOKIES_FLAT) { - if (!in_array($format, array(static::COOKIES_FLAT, static::COOKIES_ARRAY))) { - throw new \InvalidArgumentException(sprintf('Format "%s" invalid (%s).', $format, implode(', ', array(static::COOKIES_FLAT, static::COOKIES_ARRAY)))); + if (!in_array($format, array(self::COOKIES_FLAT, self::COOKIES_ARRAY))) { + throw new \InvalidArgumentException(sprintf('Format "%s" invalid (%s).', $format, implode(', ', array(self::COOKIES_FLAT, self::COOKIES_ARRAY)))); } - if (static::COOKIES_ARRAY === $format) { + if (self::COOKIES_ARRAY === $format) { return $this->cookies; } - $return = array(); + $flattenedCookies = array(); foreach ($this->cookies as $path) { foreach ($path as $cookies) { foreach ($cookies as $cookie) { - $return[] = $cookie; + $flattenedCookies[] = $cookie; } } } - return $return; + return $flattenedCookies; } /** From 64e92633aaef18719a84cc09bf98de29e37d03f9 Mon Sep 17 00:00:00 2001 From: Francis Besset Date: Wed, 13 Jul 2011 14:31:09 +0200 Subject: [PATCH 4/4] Updated UPDATE.md --- UPDATE.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/UPDATE.md b/UPDATE.md index 4e63372d75..620a7cbfd0 100644 --- a/UPDATE.md +++ b/UPDATE.md @@ -9,6 +9,19 @@ timeline closely anyway. RC4 to RC5 ---------- +* Moved cookies management of HeaderBag in ResponseHeaderBag of the HttpFoundation. + To access a cookie in Request instance, please use Request::$cookies public attribute. + +* It is not possible to know if a cookie is present or get a cookie in ResponseHeaderBag + because getCookie() and hasCookie() methods were removed. + +* The method `ResponseHeaderBag::getCookie()` accepts an argument for the return format, + the possible values are `ResponseHeaderBag::COOKIES_FLAT` (default value) or `ResponseHeaderBag::COOKIES_ARRAY` + * ResponseHeaderBag::COOKIES_FLAT return a simple array + * array(0 => `a cookie instance`, 1 => `another cookie instance`) + * ResponseHeaderBag::COOKIES_ARRAY return a multidimensional array + * array(`the domain` => array(`the path` => array(`the cookie name` => `a cookie instance`))) + * Removed the guesser for the Choice constraint as the constraint only knows about the valid keys, and not their values.