Merge pull request #6082 from kenjis/fix-get_cookie-prefix

fix: get_cookie() may not use the cookie prefix

system/Helpers/cookie_helper.php

@@ -56,21 +56,25 @@ function set_cookie(
  /**
  * Fetch an item from the $_COOKIE array
  *
- * @param string $index
+ * @param string $index
+ * @param string|null $prefix Cookie name prefix.
+ * '': the prefix in Config\Cookie
+ * null: no prefix
  *
- * @return mixed
+ * @return array|string|null
  *
  * @see \CodeIgniter\HTTP\IncomingRequest::getCookie()
  */
- function get_cookie($index, bool $xssClean = false)
+ function get_cookie($index, bool $xssClean = false, ?string $prefix = '')
  {
- /** @var Cookie|null $cookie */
- $cookie = config('Cookie');
+ if ($prefix === '') {
+ /** @var Cookie|null $cookie */
+ $cookie = config('Cookie');
 
- // @TODO Remove Config\App fallback when deprecated `App` members are removed.
- $cookiePrefix = $cookie instanceof Cookie ? $cookie->prefix : config(App::class)->cookiePrefix;
+ // @TODO Remove Config\App fallback when deprecated `App` members are removed.
+ $prefix = $cookie instanceof Cookie ? $cookie->prefix : config(App::class)->cookiePrefix;
+ }
 
- $prefix = isset($_COOKIE[$index]) ? '' : $cookiePrefix;
  $request = Services::request();
  $filter = $xssClean ? FILTER_SANITIZE_FULL_SPECIAL_CHARS : FILTER_DEFAULT;
 

tests/system/Helpers/CookieHelperTest.php

@@ -11,6 +11,7 @@
 
 namespace CodeIgniter\Helpers;
 
+use CodeIgniter\Config\Factories;
 use CodeIgniter\Cookie\Exceptions\CookieException;
 use CodeIgniter\HTTP\IncomingRequest;
 use CodeIgniter\HTTP\Response;
@@ -19,6 +20,7 @@
 use CodeIgniter\Test\CIUnitTestCase;
 use CodeIgniter\Test\Mock\MockResponse;
 use Config\App;
+use Config\Cookie as CookieConfig;
 use Config\Services;
 
 /**
@@ -33,6 +35,8 @@ final class CookieHelperTest extends CIUnitTestCase
 
  protected function setUp(): void
  {
+ $_COOKIE = [];
+
  parent::setUp();
 
  $this->name = 'greetings';
@@ -118,11 +122,44 @@ public function testDeleteCookie()
 
  public function testGetCookie()
  {
- $_COOKIE['TEST'] = 5;
+ $_COOKIE['TEST'] = '5';
 
  $this->assertSame('5', get_cookie('TEST'));
  }
 
+ public function testGetCookieDefaultPrefix()
+ {
+ $_COOKIE['prefix_TEST'] = '5';
+
+ $config = new CookieConfig();
+ $config->prefix = 'prefix_';
+ Factories::injectMock('config', CookieConfig::class, $config);
+
+ $this->assertSame('5', get_cookie('TEST', false, ''));
+ }
+
+ public function testGetCookiePrefix()
+ {
+ $_COOKIE['abc_TEST'] = '5';
+
+ $config = new CookieConfig();
+ $config->prefix = 'prefix_';
+ Factories::injectMock('config', CookieConfig::class, $config);
+
+ $this->assertSame('5', get_cookie('TEST', false, 'abc_'));
+ }
+
+ public function testGetCookieNoPrefix()
+ {
+ $_COOKIE['abc_TEST'] = '5';
+
+ $config = new CookieConfig();
+ $config->prefix = 'prefix_';
+ Factories::injectMock('config', CookieConfig::class, $config);
+
+ $this->assertSame('5', get_cookie('abc_TEST', false, null));
+ }
+
  public function testDeleteCookieAfterLastSet()
  {
  delete_cookie($this->name);

user_guide_src/source/changelogs/v4.2.1.rst

@@ -16,3 +16,4 @@ Behavior Changes
 ================
 
 - Guessing the file extension from the MIME type has been changed if the proposed extension is not valid. Previously, the guessing will early terminate and return ``null``. Now, if a proposed extension is given and is invalid, the MIME guessing will continue checking using the mapping of extension to MIME types.
+- If there is a cookie with a prefixed name and a cookie with the same name without a prefix, the previous ``get_cookie()`` had the tricky behavior of returning the cookie without the prefix. Now the behavior has been fixed as a bug, and has been changed. See :ref:`Upgrading <upgrade-421-get_cookie>` for details.

user_guide_src/source/helpers/cookie_helper.rst

@@ -39,21 +39,24 @@ The following functions are available:
  a description of its use, as this function is an alias for
  :php:func:`Response::setCookie() <setCookie>`.
 
-.. php:function:: get_cookie($index[, $xssClean = false])
+.. php:function:: get_cookie($index[, $xssClean = false[, $prefix = '']])
 
  :param string $index: Cookie name
  :param bool $xssClean: Whether to apply XSS filtering to the returned value
+ :param string|null $prefix: Cookie name prefix. If set to ``''``, the default value from **app/Config/Cookie.php** will be used. If set to ``null``, no prefix
  :returns: The cookie value or null if not found
  :rtype: mixed
 
+ .. note:: Since v4.2.1, the third parameter ``$prefix`` has been introduced and the behavior has been changed a bit due to a bug fix. See :ref:`Upgrading <upgrade-421-get_cookie>` for details.
+
  This helper function gives you friendlier syntax to get browser
  cookies. Refer to the :doc:`IncomingRequest Library </incoming/incomingrequest>` for
  detailed description of its use, as this function acts very
  similarly to ``IncomingRequest::getCookie()``, except it will also prepend
  the ``Config\Cookie::$prefix`` that you might've set in your
  **app/Config/Cookie.php** file.
 
-.. warning:: Using XSS filtering is a bad practice. It does not prevent XSS attacks perfectly. Using ``esc()`` with the correct ``$context`` in the views is recommended.
+ .. warning:: Using XSS filtering is a bad practice. It does not prevent XSS attacks perfectly. Using ``esc()`` with the correct ``$context`` in the views is recommended.
 
 .. php:function:: delete_cookie($name[, $domain = ''[, $path = '/'[, $prefix = '']]])
 

user_guide_src/source/installation/upgrade_421.rst

@@ -23,6 +23,41 @@ app/Config/Mimes.php
 Breaking Changes
 ****************
 
+.. _upgrade-421-get_cookie:
+
+get_cookie()
+============
+
+If there is a cookie with a prefixed name and a cookie with the same name without a prefix, the previous ``get_cookie()`` had the tricky behavior of returning the cookie without the prefix.
+
+For example, when ``Config\Cookie::$prefix`` is ``prefix_``, there are two cookies, ``test`` and ``prefix_test``:
+
+.. code-block:: php
+
+ $_COOKIES = [
+ 'test' => 'Non CI Cookie',
+ 'prefix_test' => 'CI Cookie',
+ ];
+
+Previously, ``get_cookie()`` returns the following:
+
+.. code-block:: php
+
+ get_cookie('test'); // returns "Non CI Cookie"
+ get_cookie('prefix_test'); // returns "CI Cookie"
+
+Now the behavior has been fixed as a bug, and has been changed like the following.
+
+.. code-block:: php
+
+ get_cookie('test'); // returns "CI Cookie"
+ get_cookie('prefix_test'); // returns null
+ get_cookie('test', false, null); // returns "Non CI Cookie"
+
+If you depend on the previous behavior, you need to change your code.
+
+.. note:: In the example above, if there is only one cookie ``prefix_test``,
+ the previous ``get_cookie('test')`` also returns ``"CI Cookie"``.
 
 Breaking Enhancements
 *********************