Skip to content

Commit

Permalink
Add store() and fix cookie expiration/overwrite
Browse files Browse the repository at this point in the history 
Add store() method for backwards compat with Client\CookieCollection.
I've also fixed a few issues that resulted in:

1. Cookies not be updated when a new response has a new value.
2. Cookies not being removed when they expire.

Both of these issues have been solved with an additional private method.
  • Loading branch information
@markstory
markstory committed Mar 27, 2017
1 parent 449a020 commit a9e1d08
Show file tree
Hide file tree
Showing 2 changed files with 223 additions and 18 deletions.
121 changes: 105 additions & 16 deletions src/Http/Cookie/CookieCollection.php
View file
Expand Up @@ -14,6 +14,7 @@
namespace Cake\Http\Cookie;

use ArrayIterator;
use Cake\Http\Client\Response as ClientResponse;
use Countable;
use DateTime;
use InvalidArgumentException;
Expand Down Expand Up @@ -46,9 +47,7 @@ public function __construct(array $cookies = [])
{
$this->checkCookies($cookies);
foreach ($cookies as $cookie) {
$name = $cookie->getName();
$key = mb_strtolower($name);
$this->cookies[$key] = $cookie;
$this->cookies[$cookie->getId()] = $cookie;
}
}

Expand All @@ -65,16 +64,17 @@ public function count()
/**
* Add a cookie and get an updated collection.
*
* Cookie names do not have to be unique in a collection, but
* having duplicate cookie names will change how get() behaves.
* Cookies are stored by id. This means that there can be duplicate
* cookies if a cookie collection is used for cookies across multiple
* domains. This can impact how get(), has() and remove() behave.
*
* @param \Cake\Http\Cookie\CookieInterface $cookie Cookie instance to add.
* @return static
*/
public function add(CookieInterface $cookie)
{
$new = clone $this;
$new->cookies[] = $cookie;
$new->cookies[$cookie->getId()] = $cookie;

return $new;
}
Expand Down Expand Up @@ -236,26 +236,38 @@ public function addFromResponse(ResponseInterface $response, RequestInterface $r

$header = $response->getHeader('Set-Cookie');
$cookies = $this->parseSetCookieHeader($header);
$cookies = $this->setRequestDefaults($cookies, $host, $path);
$new = clone $this;
foreach ($cookies as $cookie) {
$new->cookies[$cookie->getId()] = $cookie;
}
$new->removeExpiredCookies($host, $path);

return $new;
}

/**
* Apply path and host to the set of cookies if they are not set.
*
* @param array $cookies An array of cookies to update.
* @param string $host The host to set.
* @param string $path The path to set.
* @return array An array of updated cookies.
*/
protected function setRequestDefaults(array $cookies, $host, $path)
{
$out = [];
foreach ($cookies as $name => $cookie) {
// Apply path/domain from request if the cookie
// didn't have one.
if (!$cookie->getDomain()) {
$cookie = $cookie->withDomain($host);
}
if (!$cookie->getPath()) {
$cookie = $cookie->withPath($path);
}

$expires = $cookie->getExpiry();
// Don't store expired cookies
if ($expires && $expires <= time()) {
continue;
}
$new->cookies[] = $cookie;
$out[] = $cookie;
}

return $new;
return $out;
}

/**
Expand Down Expand Up @@ -315,4 +327,81 @@ protected function parseSetCookieHeader($values)

return $cookies;
}

/**
* Remove expired cookies from the collection.
*
* @param string $host The host to check for expired cookies on.
* @param string $path The path to check for expired cookies on.
* @return void
*/
private function removeExpiredCookies($host, $path)
{
$time = time();
$hostPattern = '/' . preg_quote($host, '/') . '$/';

foreach ($this->cookies as $i => $cookie) {
$expires = $cookie->getExpiry();
$expired = ($expires > 0 && $expires < $time);

$pathMatches = strpos($path, $cookie->getPath()) === 0;
$hostMatches = preg_match($hostPattern, $cookie->getDomain());
if ($pathMatches && $hostMatches && $expired) {
unset($this->cookies[$i]);
}
}
}

/**
* Store the cookies contained in a response
*
* This method operates on the collection in a mutable way for backwards
* compatibility reasons. This method should not be used and is only
* provided for backwards compatibility.
*
* @param \Cake\Http\Client\Response $response The response to read cookies from
* @param string $url The request URL used for default host/path values.
* @return void
* @deprecated 3.5.0 Will be removed in 4.0.0. Use `addFromResponse()` instead.
*/
public function store(ClientResponse $response, $url)
{
$host = parse_url($url, PHP_URL_HOST);
$path = parse_url($url, PHP_URL_PATH);
$path = $path ?: '/';

$header = $response->getHeader('Set-Cookie');
$cookies = $this->parseSetCookieHeader($header);
$cookies = $this->setRequestDefaults($cookies, $host, $path);
foreach ($cookies as $cookie) {
$this->cookies[] = $cookie;
}
$this->removeExpiredCookies($host, $path);
}

/**
* Get all cookie data as arrays.
*
* This method should not be used and is only provided for backwards compatibility.
*
* @return array
* @deprecated 3.5.0 Will be removed in 4.0.0
*/
public function getAll()
{
$out = [];
foreach ($this->cookies as $cookie) {
$out[] = [
'name' => $cookie->getName(),
'value' => $cookie->getValue(),
'path' => $cookie->getPath(),
'domain' => $cookie->getDomain(),
'secure' => $cookie->isSecure(),
'httponly' => $cookie->isHttpOnly(),
'expires' => $cookie->getExpiry()
];
}

return $out;
}
}
120 changes: 118 additions & 2 deletions tests/TestCase/Http/Cookie/CookieCollectionTest.php
View file
Expand Up @@ -12,6 +12,7 @@
*/
namespace Cake\Test\TestCase\Http\Cookie;

use Cake\Http\Client\Response as ClientResponse;
use Cake\Http\Cookie\Cookie;
use Cake\Http\Cookie\CookieCollection;
use Cake\Http\Response;
Expand Down Expand Up @@ -104,11 +105,13 @@ public function testAdd()
public function testAddDuplicates()
{
$remember = new Cookie('remember_me', 'yes');
$rememberNo = new Cookie('remember_me', 'no');
$rememberNo = new Cookie('remember_me', 'no', null, '/path2');
$this->assertNotEquals($remember->getId(), $rememberNo->getId(), 'Cookies should have different ids');

$collection = new CookieCollection([]);
$new = $collection->add($remember)->add($rememberNo);

$this->assertCount(2, $new);
$this->assertCount(2, $new, 'Cookies with different ids create duplicates.');
$this->assertNotSame($new, $collection);
$this->assertSame($remember, $new->get('remember_me'), 'get() fetches first cookie');
}
Expand Down Expand Up @@ -268,6 +271,52 @@ public function testAddFromResponseIgnoreExpired()
$this->assertFalse($new->has('expired'), 'Should drop expired cookies');
}

/**
* Test adding cookies from a response removes existing cookies if
* the new response marks them as expired.
*
* @return void
*/
public function testAddFromResponseRemoveExpired()
{
$collection = new CookieCollection([
new Cookie('expired', 'not yet', null, '/', 'example.com')
]);
$request = new ServerRequest([
'url' => '/app',
'environment' => [
'HTTP_HOST' => 'example.com'
]
]);
$response = (new Response())
->withAddedHeader('Set-Cookie', 'test=value')
->withAddedHeader('Set-Cookie', 'expired=soon; Expires=Wed, 09-Jun-2012 10:18:14 GMT; Path=/;');
$new = $collection->addFromResponse($response, $request);
$this->assertFalse($new->has('expired'), 'Should drop expired cookies');
}

/**
* Test adding cookies from responses updates cookie values.
*
* @return void
*/
public function testAddFromResponseUpdateExisting()
{
$collection = new CookieCollection([
new Cookie('key', 'old value', null, '/', 'example.com')
]);
$request = new ServerRequest([
'url' => '/',
'environment' => [
'HTTP_HOST' => 'example.com'
]
]);
$response = (new Response())->withAddedHeader('Set-Cookie', 'key=new value');
$new = $collection->addFromResponse($response, $request);
$this->assertTrue($new->has('key'));
$this->assertSame('new value', $new->get('key')->getValue());
}

/**
* Test adding cookies from the collection to request.
*
Expand Down Expand Up @@ -370,4 +419,71 @@ public function testAddToRequestSecureCrumb()
$request = $collection->addToRequest($request);
$this->assertSame(['public' => 'b'], $request->getCookieParams());
}

/**
* Test that store() provides backwards compat behavior.
*
* @return void
*/
public function testStoreCompatibility()
{
$collection = new CookieCollection();
$response = (new ClientResponse())
->withAddedHeader('Set-Cookie', 'test=value')
->withAddedHeader('Set-Cookie', 'expired=soon; Expires=Wed, 09-Jun-2012 10:18:14 GMT; Path=/;');
$result = $collection->store($response, 'http://example.com/blog');

$this->assertNull($result);
$this->assertCount(1, $collection, 'Should store 1 cookie');
$this->assertTrue($collection->has('test'));
$this->assertFalse($collection->has('expired'));
}

/**
* Test that get() provides backwards compat behavior.
*
* When the parameter is a string that looks like a URL
*
* @return void
*/
public function testGetBackwardsCompatibility()
{
$this->markTestIncomplete();
}

/**
* Test that getAll() provides backwards compat behavior.
*
* @return void
*/
public function testGetAllBackwardsCompatibility()
{
$expires = new DateTime('-2 seconds');
$cookies = [
new Cookie('test', 'value', $expires, '/api', 'example.com', true, true),
new Cookie('test_two', 'value_two', null, '/blog', 'blog.example.com', true, true),
];
$collection = new CookieCollection($cookies);
$expected = [
[
'name' => 'test',
'value' => 'value',
'path' => '/api',
'domain' => 'example.com',
'secure' => true,
'httponly' => true,
'expires' => $expires->format('U'),
],
[
'name' => 'test_two',
'value' => 'value_two',
'path' => '/blog',
'domain' => 'blog.example.com',
'secure' => true,
'httponly' => true,
'expires' => 0
],
];
$this->assertEquals($expected, $collection->getAll());
}
}

0 comments on commit a9e1d08

Please sign in to comment.