Merge pull request #10451 from cakephp/3next-client-cookie-compat

3.next - Add client compatibilty methods to CookieCollection

src/Http/Client/CookieCollection.php

@@ -13,23 +13,20 @@
  */
 namespace Cake\Http\Client;
 
+use Cake\Http\Cookie\CookieCollection as BaseCollection;
+
 /**
  * Container class for cookies used in Http\Client.
  *
  * Provides cookie jar like features for storing cookies between
  * requests, as well as appending cookies to new requests.
+ *
+ * @deprecated 3.5.0 Use Cake\Http\Cookie\CookieCollection instead.
  */
-class CookieCollection
+class CookieCollection extends BaseCollection
 {
 
     /**
-     * The cookies stored in this jar.
-     *
-     * @var array
-     */
-    protected $_cookies = [];
-
-    /**
      * Store the cookies from a response.
      *
      * Store the cookies that haven't expired. If a cookie has been expired
@@ -45,27 +42,13 @@ class CookieCollection
         $path = parse_url($url, PHP_URL_PATH);
         $path = $path ?: '/';
 
-        $cookies = $response->cookies();
-        foreach ($cookies as $name => $cookie) {
-            if (empty($cookie['domain'])) {
-                $cookie['domain'] = $host;
-            }
-            if (empty($cookie['path'])) {
-                $cookie['path'] = $path;
-            }
-            $key = implode(';', [$cookie['name'], $cookie['domain'], $cookie['path']]);
-
-            $expires = isset($cookie['expires']) ? $cookie['expires'] : false;
-            $expiresTime = false;
-            if ($expires) {
-                $expiresTime = strtotime($expires);
-            }
-            if ($expiresTime && $expiresTime <= time()) {
-                unset($this->_cookies[$key]);
-                continue;
-            }
-            $this->_cookies[$key] = $cookie;
+        $header = $response->getHeader('Set-Cookie');
+        $cookies = $this->parseSetCookieHeader($header);
+        $cookies = $this->setRequestDefaults($cookies, $host, $path);
+        foreach ($cookies as $cookie) {
+            $this->cookies[$cookie->getId()] = $cookie;
         }
+        $this->removeExpiredCookies($host, $path);
     }
 
     /**
@@ -83,28 +66,7 @@ class CookieCollection
         $host = parse_url($url, PHP_URL_HOST);
         $scheme = parse_url($url, PHP_URL_SCHEME);
 
-        $out = [];
-        foreach ($this->_cookies as $cookie) {
-            if ($scheme === 'http' && !empty($cookie['secure'])) {
-                continue;
-            }
-            if (strpos($path, $cookie['path']) !== 0) {
-                continue;
-            }
-            $leadingDot = $cookie['domain'][0] === '.';
-            if ($leadingDot) {
-                $cookie['domain'] = ltrim($cookie['domain'], '.');
-            }
-
-            $pattern = '/' . preg_quote(substr($cookie['domain'], 1), '/') . '$/';
-            if (!preg_match($pattern, $host)) {
-                continue;
-            }
-
-            $out[$cookie['name']] = $cookie['value'];
-        }
-
-        return $out;
+        return $this->findMatchingCookies($scheme, $host, $path);
     }
 
     /**
@@ -114,7 +76,20 @@ class CookieCollection
      */
     public function getAll()
     {
-        return array_values($this->_cookies);
+        $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;
     }
 }
 

src/Http/Cookie/Cookie.php

@@ -212,6 +212,20 @@ class Cookie implements CookieInterface
     }
 
     /**
+     * Get the id for a cookie
+     *
+     * Cookies are unique across name, domain, path tuples.
+     *
+     * @return string
+     */
+    public function getId()
+    {
+        $name = mb_strtolower($this->name);
+
+        return "{$name};{$this->domain};{$this->path}";
+    }
+
+    /**
      * Gets the cookie name
      *
      * @return string

src/Http/Cookie/CookieCollection.php

@@ -14,6 +14,7 @@
 namespace Cake\Http\Cookie;
 
 use ArrayIterator;
+use Cake\Http\Client\Response as ClientResponse;
 use Countable;
 use DateTime;
 use InvalidArgumentException;
@@ -46,9 +47,7 @@ class CookieCollection implements IteratorAggregate, Countable
     {
         $this->checkCookies($cookies);
         foreach ($cookies as $cookie) {
-            $name = $cookie->getName();
-            $key = mb_strtolower($name);
-            $this->cookies[$key] = $cookie;
+            $this->cookies[$cookie->getId()] = $cookie;
         }
     }
 
@@ -65,8 +64,9 @@ class CookieCollection implements IteratorAggregate, Countable
     /**
      * 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
@@ -74,7 +74,7 @@ class CookieCollection implements IteratorAggregate, Countable
     public function add(CookieInterface $cookie)
     {
         $new = clone $this;
-        $new->cookies[] = $cookie;
+        $new->cookies[$cookie->getId()] = $cookie;
 
         return $new;
     }
@@ -82,13 +82,8 @@ class CookieCollection implements IteratorAggregate, Countable
     /**
      * Get the first cookie by name.
      *
-     * If the provided name matches a URL (matches `#^https?://#`) this method
-     * will assume you want a list of cookies that match that URL. This is
-     * backwards compatible behavior that will be removed in 4.0.0
-     *
-     * @param string $name The name of the cookie. If the name looks like a URL,
-     *  backwards compatible behavior will be used.
-     * @return \Cake\Http\Cookie\CookieInterface|null|array
+     * @param string $name The name of the cookie.
+     * @return \Cake\Http\Cookie\CookieInterface|null
      */
     public function get($name)
     {
@@ -190,7 +185,22 @@ class CookieCollection implements IteratorAggregate, Countable
         $path = $uri->getPath();
         $host = $uri->getHost();
         $scheme = $uri->getScheme();
+        $cookies = $this->findMatchingCookies($scheme, $host, $path);
+        $cookies = array_merge($request->getCookieParams(), $cookies);
 
+        return $request->withCookieParams($cookies);
+    }
+
+    /**
+     * Find cookies matching the scheme, host, and path
+     *
+     * @param string $scheme The http scheme to match
+     * @param string $host The host to match.
+     * @param string $path The path to match
+     * @return array An array of cookie name/value pairs
+     */
+    protected function findMatchingCookies($scheme, $host, $path)
+    {
         $out = [];
         foreach ($this->cookies as $cookie) {
             if ($scheme === 'http' && $cookie->isSecure()) {
@@ -205,7 +215,8 @@ class CookieCollection implements IteratorAggregate, Countable
                 $domain = ltrim($domain, '.');
             }
 
-            if ($cookie->getExpiry() && time() > $cookie->getExpiry()) {
+            $expires = $cookie->getExpiry();
+            if ($expires && time() > $expires) {
                 continue;
             }
 
@@ -216,9 +227,8 @@ class CookieCollection implements IteratorAggregate, Countable
 
             $out[$cookie->getName()] = $cookie->getValue();
         }
-        $cookies = array_merge($request->getCookieParams(), $out);
 
-        return $request->withCookieParams($cookies);
+        return $out;
     }
 
     /**
@@ -236,26 +246,38 @@ class CookieCollection implements IteratorAggregate, Countable
 
         $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;
     }
 
     /**
@@ -299,7 +321,8 @@ class CookieCollection implements IteratorAggregate, Countable
             }
             $expires = null;
             if ($cookie['expires']) {
-                $expires = new DateTime($cookie['expires']);
+                $expires = new DateTime();
+                $expires->setTimestamp(strtotime($cookie['expires']));
             }
 
             $cookies[] = new Cookie(
@@ -315,4 +338,28 @@ class CookieCollection implements IteratorAggregate, Countable
 
         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
+     */
+    protected 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]);
+            }
+        }
+    }
 }

src/Http/Cookie/RequestCookies.php

@@ -37,33 +37,4 @@ class RequestCookies extends CookieCollection
 
         return new static($cookies);
     }
-
-    /**
-     * Checks if the collection has a cookie with the given name
-     *
-     * @param string $name Name of the cookie
-     * @return bool
-     */
-    public function has($name)
-    {
-        $key = mb_strtolower($name);
-
-        return isset($this->cookies[$key]);
-    }
-
-    /**
-     * Get a cookie from the collection by name.
-     *
-     * @param string $name Name of the cookie to get
-     * @throws \InvalidArgumentException
-     * @return Cookie
-     */
-    public function get($name)
-    {
-        if (!$this->has($name)) {
-            throw new InvalidArgumentException(sprintf('Cookie `%s` does not exist', $name));
-        }
-
-        return $this->cookies[mb_strtolower($name)];
-    }
 }

tests/TestCase/Http/Client/CookieCollectionTest.php

@@ -58,13 +58,19 @@ class CookieCollectionTest extends TestCase
                 'name' => 'first',
                 'value' => '1',
                 'path' => '/some/path',
-                'domain' => 'example.com'
+                'domain' => 'example.com',
+                'secure' => false,
+                'httponly' => false,
+                'expires' => 0,
             ],
             [
                 'name' => 'second',
                 'value' => '2',
                 'path' => '/',
-                'domain' => '.foo.example.com'
+                'domain' => '.foo.example.com',
+                'secure' => false,
+                'httponly' => false,
+                'expires' => 0,
             ],
         ];
         $this->assertEquals($expected, $result);
@@ -93,7 +99,10 @@ class CookieCollectionTest extends TestCase
                 'name' => 'first',
                 'value' => '1',
                 'path' => '/some/path',
-                'domain' => 'example.com'
+                'domain' => 'example.com',
+                'secure' => false,
+                'httponly' => false,
+                'expires' => 0,
             ],
             [
                 'name' => 'second',
@@ -102,6 +111,7 @@ class CookieCollectionTest extends TestCase
                 'domain' => 'example.com',
                 'secure' => true,
                 'httponly' => true,
+                'expires' => 0,
             ],
         ];
         $this->assertEquals($expected, $result);
@@ -159,7 +169,10 @@ class CookieCollectionTest extends TestCase
                 'name' => 'second',
                 'value' => '2',
                 'path' => '/',
-                'domain' => 'example.com'
+                'domain' => 'example.com',
+                'expires' => 0,
+                'secure' => false,
+                'httponly' => false,
             ],
         ];
         $this->assertEquals($expected, $result);

tests/TestCase/Http/Cookie/CookieCollectionTest.php

@@ -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;
@@ -104,11 +105,13 @@ class CookieCollectionTest extends TestCase
     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');
     }
@@ -269,6 +272,52 @@ class CookieCollectionTest extends TestCase
     }
 
     /**
+     * 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.
      *
      * @return void

tests/TestCase/Http/Cookie/CookieTest.php

@@ -504,4 +504,21 @@ class CookieTest extends TestCase
         $expected = '{"username":"florian","profile":{"profession":"developer"}}';
         $this->assertContains(urlencode($expected), $cookie->toHeaderValue());
     }
+
+    /**
+     * Tests getting the id
+     *
+     * @return void
+     */
+    public function testGetId()
+    {
+        $cookie = new Cookie('cakephp', 'cakephp-rocks');
+        $this->assertEquals('cakephp;;', $cookie->getId());
+
+        $cookie = new Cookie('CAKEPHP', 'cakephp-rocks');
+        $this->assertEquals('cakephp;;', $cookie->getId());
+
+        $cookie = new Cookie('test', 'val', null, '/path', 'example.com');
+        $this->assertEquals('test;example.com;/path', $cookie->getId());
+    }
 }