Merge pull request #10493 from cakephp/3next-response-cookies

3.next - Integrate CookieCollection into Response

src/Http/Client/Response.php

@@ -442,7 +442,7 @@ class Response extends Message implements ResponseInterface
             return null;
         }
 
-        return $this->cookies->get($name)->toArrayCompat();
+        return $this->cookies->get($name)->toArrayClient();
     }
 
     /**
@@ -469,7 +469,7 @@ class Response extends Message implements ResponseInterface
 
         $cookies = [];
         foreach ($this->cookies as $cookie) {
-            $cookies[$cookie->getName()] = $cookie->toArrayCompat();
+            $cookies[$cookie->getName()] = $cookie->toArrayClient();
         }
 
         return $cookies;

src/Http/Cookie/Cookie.php

@@ -15,7 +15,9 @@ namespace Cake\Http\Cookie;
 
 use Cake\Chronos\Chronos;
 use Cake\Utility\Hash;
+use DateTimeImmutable;
 use DateTimeInterface;
+use DateTimezone;
 use InvalidArgumentException;
 use RuntimeException;
 
@@ -79,9 +81,9 @@ class Cookie implements CookieInterface
     /**
      * Expiration time
      *
-     * @var int
+     * @var DateTimeInterface
      */
-    protected $expiresAt = 0;
+    protected $expiresAt;
 
     /**
      * Path
@@ -152,10 +154,10 @@ class Cookie implements CookieInterface
 
         $this->validateBool($secure);
         $this->secure = $secure;
-
-        if ($expiresAt !== null) {
-            $this->expiresAt = (int)$expiresAt->format('U');
+        if ($expiresAt) {
+            $expiresAt = $expiresAt->setTimezone(new DateTimezone('GMT'));
         }
+        $this->expiresAt = $expiresAt;
     }
 
     /**
@@ -163,12 +165,30 @@ class Cookie implements CookieInterface
      *
      * @return string
      */
-    protected function _buildExpirationValue()
+    protected function getFormattedExpires()
+    {
+        if (!$this->expiresAt) {
+            return '';
+        }
+
+        return $this->expiresAt->format(static::EXPIRES_FORMAT);
+    }
+
+    /**
+     * Get the timestamp from the expiration time
+     *
+     * Timestamps are strings as large timestamps can overflow MAX_INT
+     * in 32bit systems.
+     *
+     * @return string|null The expiry time as a string timestamp.
+     */
+    public function getExpiresTimestamp()
     {
-        return sprintf(
-            'expires=%s',
-            gmdate(static::EXPIRES_FORMAT, $this->expiresAt)
-        );
+        if (!$this->expiresAt) {
+            return null;
+        }
+
+        return $this->expiresAt->format('U');
     }
 
     /**
@@ -184,8 +204,8 @@ class Cookie implements CookieInterface
         }
         $headerValue[] = sprintf('%s=%s', $this->name, urlencode($value));
 
-        if ($this->expiresAt !== 0) {
-            $headerValue[] = $this->_buildExpirationValue();
+        if ($this->expiresAt) {
+            $headerValue[] = sprintf('expires=%s', $this->getFormattedExpires());
         }
         if ($this->path !== '') {
             $headerValue[] = sprintf('path=%s', $this->path);
@@ -248,10 +268,11 @@ class Cookie implements CookieInterface
      * @param string $name Name of the cookie
      * @return void
      * @throws \InvalidArgumentException
+     * @link https://tools.ietf.org/html/rfc2616#section-2.2 Rules for naming cookies.
      */
     protected function validateName($name)
     {
-        if (preg_match("/[=,; \t\r\n\013\014]/", $name)) {
+        if (preg_match("/[=,;\t\r\n\013\014]/", $name)) {
             throw new InvalidArgumentException(
                 sprintf('The cookie name `%s` contains invalid characters.', $name)
             );
@@ -441,7 +462,7 @@ class Cookie implements CookieInterface
     public function withExpiry(DateTimeInterface $dateTime)
     {
         $new = clone $this;
-        $new->expiresAt = (int)$dateTime->format('U');
+        $new->expiresAt = $dateTime->setTimezone(new DateTimezone('GMT'));
 
         return $new;
     }
@@ -449,7 +470,7 @@ class Cookie implements CookieInterface
     /**
      * Get the current expiry time
      *
-     * @return int|null Timestamp of expiry or null
+     * @return DateTimeInterface|null Timestamp of expiry or null
      */
     public function getExpiry()
     {
@@ -464,7 +485,7 @@ class Cookie implements CookieInterface
     public function withNeverExpire()
     {
         $new = clone $this;
-        $new->expiresAt = Chronos::createFromDate(2038, 1, 1)->format('U');
+        $new->expiresAt = Chronos::createFromDate(2038, 1, 1);
 
         return $new;
     }
@@ -479,7 +500,7 @@ class Cookie implements CookieInterface
     public function withExpired()
     {
         $new = clone $this;
-        $new->expiresAt = Chronos::parse('-1 year')->format('U');
+        $new->expiresAt = Chronos::parse('-1 year');
 
         return $new;
     }
@@ -619,7 +640,7 @@ class Cookie implements CookieInterface
             'domain' => $this->getDomain(),
             'secure' => $this->isSecure(),
             'httponly' => $this->isHttpOnly(),
-            'expires' => $this->getExpiry()
+            'expires' => $this->getExpiresTimestamp()
         ];
     }
 
@@ -631,7 +652,7 @@ class Cookie implements CookieInterface
      *
      * @return array
      */
-    public function toArrayCompat()
+    public function toArrayClient()
     {
         return [
             'name' => $this->getName(),
@@ -640,7 +661,28 @@ class Cookie implements CookieInterface
             'domain' => $this->getDomain(),
             'secure' => $this->isSecure(),
             'httponly' => $this->isHttpOnly(),
-            'expires' => gmdate(static::EXPIRES_FORMAT, $this->expiresAt)
+            'expires' => $this->getFormattedExpires()
+        ];
+    }
+
+    /**
+     * Convert the cookie into an array of its properties.
+     *
+     * This method is compatible with the historical behavior of Cake\Http\Response,
+     * where `httponly` is `httpOnly` and `expires` is `expire`
+     *
+     * @return array
+     */
+    public function toArrayResponse()
+    {
+        return [
+            'name' => $this->getName(),
+            'value' => $this->getValue(),
+            'path' => $this->getPath(),
+            'domain' => $this->getDomain(),
+            'secure' => $this->isSecure(),
+            'httpOnly' => $this->isHttpOnly(),
+            'expire' => $this->getExpiresTimestamp()
         ];
     }
 

src/Http/Cookie/CookieCollection.php

@@ -253,7 +253,7 @@ class CookieCollection implements IteratorAggregate, Countable
                 $domain = ltrim($domain, '.');
             }
 
-            $expires = $cookie->getExpiry();
+            $expires = $cookie->getExpiresTimestamp();
             if ($expires && time() > $expires) {
                 continue;
             }
@@ -389,7 +389,7 @@ class CookieCollection implements IteratorAggregate, Countable
         $hostPattern = '/' . preg_quote($host, '/') . '$/';
 
         foreach ($this->cookies as $i => $cookie) {
-            $expires = $cookie->getExpiry();
+            $expires = $cookie->getExpiresTimestamp();
             $expired = ($expires > 0 && $expires < $time);
 
             $pathMatches = strpos($path, $cookie->getPath()) === 0;

src/Http/Response.php

@@ -16,6 +16,8 @@ namespace Cake\Http;
 
 use Cake\Core\Configure;
 use Cake\Filesystem\File;
+use Cake\Http\Cookie\Cookie;
+use Cake\Http\Cookie\CookieCollection;
 use Cake\Log\Log;
 use Cake\Network\CorsBuilder;
 use Cake\Network\Exception\NotFoundException;
@@ -28,10 +30,7 @@ use Zend\Diactoros\MessageTrait;
 use Zend\Diactoros\Stream;
 
 /**
- * Cake Response is responsible for managing the response text, status and headers of a HTTP response.
- *
- * By default controllers will use this class to render their response. If you are going to use
- * a custom response class it should subclass this object in order to ensure compatibility.
+ * Responses contain the response text, status and headers of a HTTP response.
  */
 class Response implements ResponseInterface
 {
@@ -392,11 +391,11 @@ class Response implements ResponseInterface
     protected $_cacheDirectives = [];
 
     /**
-     * Holds cookies to be sent to the client
+     * Collection of cookies to send to the client
      *
-     * @var array
+     * @var \Cake\Http\Cookie\CookieCollection
      */
-    protected $_cookies = [];
+    protected $_cookies = null;
 
     /**
      * Reason Phrase
@@ -462,6 +461,7 @@ class Response implements ResponseInterface
             $this->_contentType = $this->resolveType($options['type']);
         }
         $this->_setContentType();
+        $this->_cookies = new CookieCollection();
     }
 
     /**
@@ -1931,18 +1931,18 @@ class Response implements ResponseInterface
     public function cookie($options = null)
     {
         if ($options === null) {
-            return $this->_cookies;
+            return $this->getCookies();
         }
 
         if (is_string($options)) {
-            if (!isset($this->_cookies[$options])) {
+            if (!$this->_cookies->has($options)) {
                 return null;
             }
 
-            return $this->_cookies[$options];
+            return $this->_cookies->get($options)->toArrayResponse();
         }
 
-        $defaults = [
+        $options += [
             'name' => 'CakeCookie[default]',
             'value' => '',
             'expire' => 0,
@@ -1951,9 +1951,17 @@ class Response implements ResponseInterface
             'secure' => false,
             'httpOnly' => false
         ];
-        $options += $defaults;
-
-        $this->_cookies[$options['name']] = $options;
+        $expires = $options['expire'] ? new DateTime('@' . $options['expire']) : null;
+        $cookie = new Cookie(
+            $options['name'],
+            $options['value'],
+            $expires,
+            $options['path'],
+            $options['domain'],
+            $options['secure'],
+            $options['httpOnly']
+        );
+        $this->_cookies = $this->_cookies->add($cookie);
     }
 
     /**
@@ -1977,30 +1985,45 @@ class Response implements ResponseInterface
      *
      * // customize cookie attributes
      * $response = $response->withCookie('remember_me', ['path' => '/login']);
+     *
+     * // add a cookie object
+     * $response = $response->withCookie(new Cookie('remember_me', 1));
      * ```
      *
-     * @param string $name The name of the cookie to set.
+     * @param string|\Cake\Http\Cookie\Cookie $name The name of the cookie to set, or a cookie object
      * @param array|string $data Either a string value, or an array of cookie options.
      * @return static
      */
     public function withCookie($name, $data = '')
     {
-        if (!is_array($data)) {
-            $data = ['value' => $data];
+        if ($name instanceof Cookie) {
+            $cookie = $name;
+        } else {
+            if (!is_array($data)) {
+                $data = ['value' => $data];
+            }
+            $data += [
+                'value' => '',
+                'expire' => 0,
+                'path' => '/',
+                'domain' => '',
+                'secure' => false,
+                'httpOnly' => false
+            ];
+            $expires = $data['expire'] ? new DateTime('@' . $data['expire']) : null;
+            $cookie = new Cookie(
+                $name,
+                $data['value'],
+                $expires,
+                $data['path'],
+                $data['domain'],
+                $data['secure'],
+                $data['httpOnly']
+            );
         }
-        $defaults = [
-            'value' => '',
-            'expire' => 0,
-            'path' => '/',
-            'domain' => '',
-            'secure' => false,
-            'httpOnly' => false
-        ];
-        $data += $defaults;
-        $data['name'] = $name;
 
         $new = clone $this;
-        $new->_cookies[$name] = $data;
+        $new->_cookies = $new->_cookies->add($cookie);
 
         return $new;
     }
@@ -2016,11 +2039,11 @@ class Response implements ResponseInterface
      */
     public function getCookie($name)
     {
-        if (isset($this->_cookies[$name])) {
-            return $this->_cookies[$name];
+        if (!$this->_cookies->has($name)) {
+            return null;
         }
 
-        return null;
+        return $this->_cookies->get($name)->toArrayResponse();
     }
 
     /**
@@ -2032,6 +2055,21 @@ class Response implements ResponseInterface
      */
     public function getCookies()
     {
+        $out = [];
+        foreach ($this->_cookies as $cookie) {
+            $out[$cookie->getName()] = $cookie->toArrayResponse();
+        }
+
+        return $out;
+    }
+
+    /**
+     * Get the CookieCollection from the response
+     *
+     * @return \Cake\Http\Cookie\CookieCollection
+     */
+    public function getCookieCollection()
+    {
         return $this->_cookies;
     }
 

tests/TestCase/Http/Cookie/CookieCollectionTest.php

@@ -222,14 +222,14 @@ class CookieCollectionTest extends TestCase
         $this->assertSame('/app', $new->get('test')->getPath(), 'cookies should inherit request path');
         $this->assertSame('/', $new->get('expiring')->getPath(), 'path attribute should be used.');
 
-        $this->assertSame(0, $new->get('test')->getExpiry(), 'No expiry');
+        $this->assertNull($new->get('test')->getExpiry(), 'No expiry');
         $this->assertSame(
             '2021-06-09 10:18:14',
-            date('Y-m-d H:i:s', $new->get('expiring')->getExpiry()),
+            $new->get('expiring')->getExpiry()->format('Y-m-d H:i:s'),
             'Has expiry'
         );
         $session = $new->get('session');
-        $this->assertSame(0, $session->getExpiry(), 'No expiry');
+        $this->assertNull($session->getExpiry(), 'No expiry');
         $this->assertSame('www.example.com', $session->getDomain(), 'Has domain');
     }
 

tests/TestCase/Http/Cookie/CookieTest.php

@@ -30,14 +30,32 @@ class CookieTest extends TestCase
     protected $encryptionKey = 'someverysecretkeythatisatleast32charslong';
 
     /**
+     * Generate invalid cookie names.
+     *
+     * @return array
+     */
+    public function invalidNameProvider()
+    {
+        return [
+            ['no='],
+            ["no\rnewline"],
+            ["no\nnewline"],
+            ["no\ttab"],
+            ["no,comma"],
+            ["no;semi"],
+        ];
+    }
+
+    /**
      * Test invalid cookie name
      *
+     * @dataProvider invalidNameProvider
      * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage The cookie name `no, this wont, work` contains invalid characters.
+     * @expectedExceptionMessage contains invalid characters.
      */
-    public function testValidateNameInvalidChars()
+    public function testValidateNameInvalidChars($name)
     {
-        new Cookie('no, this wont, work', '');
+        new Cookie($name, 'value');
     }
 
     /**
@@ -338,6 +356,26 @@ class CookieTest extends TestCase
     }
 
     /**
+     * Test the withExpiry method changes timezone
+     *
+     * @return void
+     */
+    public function testWithExpiryChangesTimezone()
+    {
+        $cookie = new Cookie('cakephp', 'cakephp-rocks');
+        $date = Chronos::createFromDate(2022, 6, 15);
+        $date = $date->setTimezone('America/New_York');
+
+        $new = $cookie->withExpiry($date);
+        $this->assertNotSame($new, $cookie, 'Should clone');
+        $this->assertNotContains('expires', $cookie->toHeaderValue());
+
+        $this->assertContains('expires=Wed, 15-Jun-2022', $new->toHeaderValue());
+        $this->assertContains('GMT', $new->toHeaderValue());
+        $this->assertSame($date->format('U'), $new->getExpiresTimestamp());
+    }
+
+    /**
      * Test the withName method
      *
      * @return void
@@ -541,7 +579,7 @@ class CookieTest extends TestCase
             'value' => 'cakephp-rocks',
             'path' => '/api',
             'domain' => 'cakephp.org',
-            'expires' => (int)$date->format('U'),
+            'expires' => $date->format('U'),
             'secure' => true,
             'httponly' => true
         ];
@@ -549,11 +587,11 @@ class CookieTest extends TestCase
     }
 
     /**
-     * Test toArrayCompat
+     * Test toArrayClient
      *
      * @return void
      */
-    public function testToArrayCompat()
+    public function testToArrayClient()
     {
         $date = Chronos::parse('2017-03-31 12:34:56');
         $cookie = new Cookie('cakephp', 'cakephp-rocks');
@@ -571,6 +609,32 @@ class CookieTest extends TestCase
             'secure' => true,
             'httponly' => true
         ];
-        $this->assertEquals($expected, $cookie->toArrayCompat());
+        $this->assertEquals($expected, $cookie->toArrayClient());
+    }
+
+    /**
+     * Test toArrayResponse
+     *
+     * @return void
+     */
+    public function testToArrayResponse()
+    {
+        $date = Chronos::parse('2017-03-31 12:34:56');
+        $cookie = new Cookie('cakephp', 'cakephp-rocks');
+        $cookie = $cookie->withDomain('cakephp.org')
+            ->withPath('/api')
+            ->withExpiry($date)
+            ->withHttpOnly(true)
+            ->withSecure(true);
+        $expected = [
+            'name' => 'cakephp',
+            'value' => 'cakephp-rocks',
+            'path' => '/api',
+            'domain' => 'cakephp.org',
+            'expire' => $date->format('U'),
+            'secure' => true,
+            'httpOnly' => true
+        ];
+        $this->assertEquals($expected, $cookie->toArrayResponse());
     }
 }

tests/TestCase/Http/ResponseTest.php

@@ -16,6 +16,8 @@ namespace Cake\Test\TestCase\Http;
 
 include_once CORE_TEST_CASES . DS . 'Http' . DS . 'server_mocks.php';
 
+use Cake\Http\Cookie\Cookie;
+use Cake\Http\Cookie\CookieCollection;
 use Cake\Http\Response;
 use Cake\Http\ServerRequest;
 use Cake\Network\Exception\NotFoundException;
@@ -1332,7 +1334,8 @@ class ResponseTest extends TestCase
             'path' => '/',
             'domain' => '',
             'secure' => false,
-            'httpOnly' => false];
+            'httpOnly' => false
+        ];
         $result = $response->cookie('CakeTestCookie[Testing]');
         $this->assertEquals($expected, $result);
 
@@ -1471,6 +1474,22 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * Test withCookie() and a cookie instance
+     *
+     * @return void
+     */
+    public function testWithCookieObject()
+    {
+        $response = new Response();
+        $cookie = new Cookie('yay', 'a value');
+        $new = $response->withCookie($cookie);
+        $this->assertNull($response->getCookie('yay'), 'withCookie does not mutate');
+
+        $this->assertNotEmpty($new->getCookie('yay'));
+        $this->assertSame($cookie, $new->getCookieCollection()->get('yay'));
+    }
+
+    /**
      * Test getCookies() and array data.
      *
      * @return void
@@ -1478,13 +1497,6 @@ class ResponseTest extends TestCase
     public function testGetCookies()
     {
         $response = new Response();
-        $cookie = [
-            'name' => 'ignored key',
-            'value' => '[a,b,c]',
-            'expire' => 1000,
-            'path' => '/test',
-            'secure' => true
-        ];
         $new = $response->withCookie('testing', 'a')
             ->withCookie('test2', ['value' => 'b', 'path' => '/test', 'secure' => true]);
         $expected = [
@@ -1511,6 +1523,28 @@ class ResponseTest extends TestCase
     }
 
     /**
+     * Test getCookieCollection() as array data
+     *
+     * @return void
+     */
+    public function testGetCookieCollection()
+    {
+        $response = new Response();
+        $new = $response->withCookie('testing', 'a')
+            ->withCookie('test2', ['value' => 'b', 'path' => '/test', 'secure' => true]);
+        $cookies = $response->getCookieCollection();
+        $this->assertInstanceOf(CookieCollection::class, $cookies);
+        $this->assertCount(0, $cookies, 'Original response not mutated');
+
+        $cookies = $new->getCookieCollection();
+        $this->assertInstanceOf(CookieCollection::class, $cookies);
+        $this->assertCount(2, $cookies);
+
+        $this->assertTrue($cookies->has('testing'));
+        $this->assertTrue($cookies->has('test2'));
+    }
+
+    /**
      * Test CORS
      *
      * @dataProvider corsData
@@ -3010,7 +3044,7 @@ class ResponseTest extends TestCase
             ],
             'file' => null,
             'fileRange' => [],
-            'cookies' => [],
+            'cookies' => new CookieCollection(),
             'cacheDirectives' => [],
             'body' => ''
         ];

tests/TestCase/Http/ResponseTransformerTest.php

@@ -156,13 +156,13 @@ class ResponseTransformerTest extends TestCase
     public function testToCakeCookies()
     {
         $cookies = [
-            'remember me=1";"1',
+            'remember_me=1";"1',
             'forever=yes; Expires=Wed, 13 Jan 2021 12:30:40 GMT; Path=/some/path; Domain=example.com; HttpOnly; Secure'
         ];
         $psr = new PsrResponse('php://memory', 200, ['Set-Cookie' => $cookies]);
         $result = ResponseTransformer::toCake($psr);
         $expected = [
-            'name' => 'remember me',
+            'name' => 'remember_me',
             'value' => '1";"1',
             'path' => '/',
             'domain' => '',
@@ -170,7 +170,7 @@ class ResponseTransformerTest extends TestCase
             'secure' => false,
             'httpOnly' => false,
         ];
-        $this->assertEquals($expected, $result->cookie('remember me'));
+        $this->assertEquals($expected, $result->cookie('remember_me'));
 
         $expected = [
             'name' => 'forever',
@@ -242,7 +242,7 @@ class ResponseTransformerTest extends TestCase
     {
         $cake = new CakeResponse(['status' => 200]);
         $cake->cookie([
-            'name' => 'remember me',
+            'name' => 'remember_me',
             'value' => '1 1',
             'path' => '/some/path',
             'domain' => 'example.com',
@@ -252,7 +252,7 @@ class ResponseTransformerTest extends TestCase
         ]);
         $result = ResponseTransformer::toPsr($cake);
         $this->assertEquals(
-            'remember+me=1+1; Expires=Wed, 13 Jan 2021 12:30:40 GMT; Path=/some/path; Domain=example.com; HttpOnly; Secure',
+            'remember_me=1+1; Expires=Wed, 13 Jan 2021 12:30:40 GMT; Path=/some/path; Domain=example.com; HttpOnly; Secure',
             $result->getHeader('Set-Cookie')[0],
             'Cookie attributes should exist, and name/value should be encoded'
         );