From 73902f0730454f499d1a8bb49382e5021943656e Mon Sep 17 00:00:00 2001 From: rhertogh Date: Tue, 15 Aug 2023 18:38:10 +0200 Subject: [PATCH 1/5] Added support for string and DateTimeInterface as Cookie::$expire (#19920) * Added support for string as Cookie::$expire * Updated changelog for #19920 (Broadened the accepted type of `Cookie::$expire` from `int` to `int|string|null`) * Fixed `\yiiunit\framework\web\ResponseTest::parseHeaderCookies()` to overwrite existing cookie. * Added support for `\DateTimeInterface` in `\yii\web\Cookie::$expire` * Fixed `\yiiunit\framework\web\ResponseTest::cookiesTestProvider()` for PHP 5.4 and commited missing code for \DateTimeInterface processing in `\yii\web\Response::sendCookies()` --- framework/CHANGELOG.md | 1 + framework/web/Cookie.php | 4 +- framework/web/CookieCollection.php | 17 +++- framework/web/Response.php | 15 +++- tests/framework/web/ResponseTest.php | 111 +++++++++++++++++++++++++-- 5 files changed, 132 insertions(+), 16 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 9833be22ac..0b47c48c78 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -15,6 +15,7 @@ Yii Framework 2 Change Log - Enh #19884: Added support Enums in Query Builder (sk1t0n) - Bug #19908: Fix associative array cell content rendering in Table widget (rhertogh) - Bug #19906: Fixed multiline strings in the `\yii\console\widgets\Table` widget (rhertogh) +- Enh #19920: Broadened the accepted type of `Cookie::$expire` from `int` to `int|string|\DateTimeInterface|null` (rhertogh) 2.0.48.1 May 24, 2023 diff --git a/framework/web/Cookie.php b/framework/web/Cookie.php index ac9a723371..d249afa6cb 100644 --- a/framework/web/Cookie.php +++ b/framework/web/Cookie.php @@ -57,8 +57,8 @@ class Cookie extends \yii\base\BaseObject */ public $domain = ''; /** - * @var int the timestamp at which the cookie expires. This is the server timestamp. - * Defaults to 0, meaning "until the browser is closed". + * @var int|string|\DateTimeInterface|null the timestamp or date at which the cookie expires. This is the server timestamp. + * Defaults to 0, meaning "until the browser is closed" (the same applies to `null`). */ public $expire = 0; /** diff --git a/framework/web/CookieCollection.php b/framework/web/CookieCollection.php index 5c56e46f70..597fc2f5fd 100644 --- a/framework/web/CookieCollection.php +++ b/framework/web/CookieCollection.php @@ -51,7 +51,7 @@ class CookieCollection extends BaseObject implements \IteratorAggregate, \ArrayA * Returns an iterator for traversing the cookies in the collection. * This method is required by the SPL interface [[\IteratorAggregate]]. * It will be implicitly called when you use `foreach` to traverse the collection. - * @return ArrayIterator an iterator for traversing the cookies in the collection. + * @return ArrayIterator an iterator for traversing the cookies in the collection. */ #[\ReturnTypeWillChange] public function getIterator() @@ -113,7 +113,18 @@ class CookieCollection extends BaseObject implements \IteratorAggregate, \ArrayA public function has($name) { return isset($this->_cookies[$name]) && $this->_cookies[$name]->value !== '' - && ($this->_cookies[$name]->expire === null || $this->_cookies[$name]->expire === 0 || $this->_cookies[$name]->expire >= time()); + && ($this->_cookies[$name]->expire === null + || $this->_cookies[$name]->expire === 0 + || ( + (is_string($this->_cookies[$name]->expire) && strtotime($this->_cookies[$name]->expire) >= time()) + || ( + interface_exists('\\DateTimeInterface') + && $this->_cookies[$name]->expire instanceof \DateTimeInterface + && $this->_cookies[$name]->expire->getTimestamp() >= time() + ) + || $this->_cookies[$name]->expire >= time() + ) + ); } /** @@ -174,7 +185,7 @@ class CookieCollection extends BaseObject implements \IteratorAggregate, \ArrayA /** * Returns the collection as a PHP array. - * @return array the array representation of the collection. + * @return Cookie[] the array representation of the collection. * The array keys are cookie names, and the array values are the corresponding cookie objects. */ public function toArray() diff --git a/framework/web/Response.php b/framework/web/Response.php index 57349b599e..1724438803 100644 --- a/framework/web/Response.php +++ b/framework/web/Response.php @@ -401,12 +401,21 @@ class Response extends \yii\base\Response } foreach ($this->getCookies() as $cookie) { $value = $cookie->value; - if ($cookie->expire != 1 && isset($validationKey)) { + $expire = $cookie->expire; + if (is_string($expire)) { + $expire = strtotime($expire); + } elseif (interface_exists('\\DateTimeInterface') && $expire instanceof \DateTimeInterface) { + $expire = $expire->getTimestamp(); + } + if ($expire === null || $expire === false) { + $expire = 0; + } + if ($expire != 1 && isset($validationKey)) { $value = Yii::$app->getSecurity()->hashData(serialize([$cookie->name, $value]), $validationKey); } if (PHP_VERSION_ID >= 70300) { setcookie($cookie->name, $value, [ - 'expires' => $cookie->expire, + 'expires' => $expire, 'path' => $cookie->path, 'domain' => $cookie->domain, 'secure' => $cookie->secure, @@ -420,7 +429,7 @@ class Response extends \yii\base\Response if (!is_null($cookie->sameSite)) { $cookiePath .= '; samesite=' . $cookie->sameSite; } - setcookie($cookie->name, $value, $cookie->expire, $cookiePath, $cookie->domain, $cookie->secure, $cookie->httpOnly); + setcookie($cookie->name, $value, $expire, $cookiePath, $cookie->domain, $cookie->secure, $cookie->httpOnly); } } } diff --git a/tests/framework/web/ResponseTest.php b/tests/framework/web/ResponseTest.php index 07e55b7dfa..2c49964fbc 100644 --- a/tests/framework/web/ResponseTest.php +++ b/tests/framework/web/ResponseTest.php @@ -380,21 +380,116 @@ class ResponseTest extends \yiiunit\TestCase ); } - public function testSameSiteCookie() + /** + * @dataProvider cookiesTestProvider + */ + public function testCookies($cookieConfig, $expected) { $response = new Response(); - $response->cookies->add(new Cookie([ - 'name' => 'test', - 'value' => 'testValue', - 'sameSite' => Cookie::SAME_SITE_STRICT, - ])); + $response->cookies->add(new Cookie(array_merge( + [ + 'name' => 'test', + 'value' => 'testValue', + ], + $cookieConfig + ))); ob_start(); $response->send(); $content = ob_get_clean(); - // Only way to test is that it doesn't create any errors - $this->assertEquals('', $content); + $cookies = $this->parseHeaderCookies(); + if ($cookies === false) { + // Unable to resolve cookies, only way to test is that it doesn't create any errors + $this->assertEquals('', $content); + } else { + $testCookie = $cookies['test']; + $actual = array_intersect_key($testCookie, $expected); + ksort($actual); + ksort($expected); + $this->assertEquals($expected, $actual); + } + } + + public function cookiesTestProvider() + { + $expireInt = time() + 3600; + $expireString = date('D, d-M-Y H:i:s', $expireInt) . ' GMT'; + + $testCases = [ + 'same-site' => [ + ['sameSite' => Cookie::SAME_SITE_STRICT], + ['samesite' => Cookie::SAME_SITE_STRICT], + ], + 'expire-as-int' => [ + ['expire' => $expireInt], + ['expires' => $expireString], + ], + 'expire-as-string' => [ + ['expire' => $expireString], + ['expires' => $expireString], + ], + ]; + + if (version_compare(PHP_VERSION, '5.5.0', '>=')) { + $testCases = array_merge($testCases, [ + 'expire-as-date_time' => [ + ['expire' => new \DateTime('@' . $expireInt)], + ['expires' => $expireString], + ], + 'expire-as-date_time_immutable' => [ + ['expire' => new \DateTimeImmutable('@' . $expireInt)], + ['expires' => $expireString], + ], + ]); + } + + return $testCases; + } + + /** + * Tries to parse cookies set in the response headers. + * When running PHP on the CLI headers are not available (the `headers_list()` function always returns an + * empty array). If possible use xDebug: http://xdebug.org/docs/all_functions#xdebug_get_headers + * @param $name + * @return array|false + */ + protected function parseHeaderCookies() { + + if (!function_exists('xdebug_get_headers')) { + return false; + } + + $cookies = []; + foreach(xdebug_get_headers() as $header) { + if (strpos($header, 'Set-Cookie: ') !== 0) { + continue; + } + + $name = null; + $params = []; + $pairs = explode(';', substr($header, 12)); + foreach ($pairs as $index => $pair) { + $pair = trim($pair); + if (strpos($pair, '=') === false) { + $params[strtolower($pair)] = true; + } else { + list($paramName, $paramValue) = explode('=', $pair, 2); + if ($index === 0) { + $name = $paramName; + $params['value'] = urldecode($paramValue); + } else { + $params[strtolower($paramName)] = urldecode($paramValue); + } + } + } + if ($name === null) { + throw new \Exception('Could not determine cookie name for header "' . $header . '".'); + } + $cookies[$name] = $params; + } + + return $cookies; } /** From e40fb70fa85650c1c2a892f17edfb90d45d0459e Mon Sep 17 00:00:00 2001 From: Mark Huot Date: Fri, 18 Aug 2023 07:15:39 -0400 Subject: [PATCH 2/5] Remove unnecessary type (#19932) Because Web and Console application extend Base application there is no need to specify it. Actually, PHPStan narrows this to _just_ BaseApplication because it is the only consistent type of the three so you lose Web and Console Application type hints. See, https://phpstan.org/r/d21fb99f-c436-480b-99c2-32df35ec07fa --- framework/BaseYii.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/BaseYii.php b/framework/BaseYii.php index 8f76ea5123..dfa5419cb0 100644 --- a/framework/BaseYii.php +++ b/framework/BaseYii.php @@ -68,7 +68,7 @@ class BaseYii */ public static $classMap = []; /** - * @var \yii\console\Application|\yii\web\Application|\yii\base\Application the application instance + * @var \yii\console\Application|\yii\web\Application the application instance */ public static $app; /** From 73f57fdcb01b1651ef0f2c8a7588bd6c8508bdf6 Mon Sep 17 00:00:00 2001 From: Gusakov Egor <85644473+eegusakov@users.noreply.github.com> Date: Fri, 18 Aug 2023 14:19:24 +0300 Subject: [PATCH 3/5] Fix #19872: Fixed the definition of dirty attributes in AR properties for a non-associative array in case of changing the order of elements --- framework/CHANGELOG.md | 1 + framework/db/BaseActiveRecord.php | 2 +- tests/framework/db/BaseActiveRecordTest.php | 42 +++++++++++++++++ .../db/mysql/BaseActiveRecordTest.php | 37 +++++++++++++++ .../db/pgsql/BaseActiveRecordTest.php | 46 +++++++++++++++++++ 5 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 tests/framework/db/BaseActiveRecordTest.php create mode 100644 tests/framework/db/mysql/BaseActiveRecordTest.php create mode 100644 tests/framework/db/pgsql/BaseActiveRecordTest.php diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 0b47c48c78..eaa8bc6154 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -4,6 +4,7 @@ Yii Framework 2 Change Log 2.0.49 under development ------------------------ +- Bug #19872: Fixed the definition of dirty attributes in AR properties for a non-associative array in case of changing the order of elements (eegusakov) - Bug #19899: Fixed `GridView` in some cases calling `Model::generateAttributeLabel()` to generate label values that are never used (PowerGamer1) - Bug #9899: Fix caching a MSSQL query with BLOB data type (terabytesoftw) - Bug #16208: Fix `yii\log\FileTarget` to not export empty messages (terabytesoftw) diff --git a/framework/db/BaseActiveRecord.php b/framework/db/BaseActiveRecord.php index 88fb2f11d7..2648a48dae 100644 --- a/framework/db/BaseActiveRecord.php +++ b/framework/db/BaseActiveRecord.php @@ -1774,7 +1774,7 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface */ private function isValueDifferent($newValue, $oldValue) { - if (is_array($newValue) && is_array($oldValue) && !ArrayHelper::isAssociative($oldValue)) { + if (is_array($newValue) && is_array($oldValue) && ArrayHelper::isAssociative($oldValue)) { $newValue = ArrayHelper::recursiveSort($newValue); $oldValue = ArrayHelper::recursiveSort($oldValue); } diff --git a/tests/framework/db/BaseActiveRecordTest.php b/tests/framework/db/BaseActiveRecordTest.php new file mode 100644 index 0000000000..0427421cd8 --- /dev/null +++ b/tests/framework/db/BaseActiveRecordTest.php @@ -0,0 +1,42 @@ +getConnection(); + } + + public function provideArrayValueWithChange() + { + return [ + 'not an associative array with data change' => [ + [1, 2, 3], + [1, 3, 2], + ], + + 'associative array with data change case 1' => [ + ['pineapple' => 2, 'apple' => 5, 'banana' => 1], + ['apple' => 5, 'pineapple' => 1, 'banana' => 3], + ], + 'associative array with data change case 2' => [ + ['pineapple' => 2, 'apple' => 5, 'banana' => 1], + ['pineapple' => 2, 'apple' => 3, 'banana' => 1], + ], + + 'filling an empty array' => [ + [], + ['pineapple' => 3, 'apple' => 1, 'banana' => 1], + ], + 'zeroing the array' => [ + ['pineapple' => 3, 'apple' => 1, 'banana' => 17], + [], + ], + ]; + } +} diff --git a/tests/framework/db/mysql/BaseActiveRecordTest.php b/tests/framework/db/mysql/BaseActiveRecordTest.php new file mode 100644 index 0000000000..394922e87f --- /dev/null +++ b/tests/framework/db/mysql/BaseActiveRecordTest.php @@ -0,0 +1,37 @@ +getConnection()->getSchema()->getServerVersion(), '5.7', '<')) { + $this->markTestSkipped('JSON columns are not supported in MySQL < 5.7'); + } + if (version_compare(PHP_VERSION, '5.6', '<')) { + $this->markTestSkipped('JSON columns are not supported in PDO for PHP < 5.6'); + } + + $createdStorage = new Storage(['data' => $actual]); + + $createdStorage->save(); + + $foundStorage = Storage::find()->limit(1)->one(); + + $this->assertNotNull($foundStorage); + + $foundStorage->data = $modified; + + $this->assertSame(['data' => $modified], $foundStorage->getDirtyAttributes()); + } +} diff --git a/tests/framework/db/pgsql/BaseActiveRecordTest.php b/tests/framework/db/pgsql/BaseActiveRecordTest.php new file mode 100644 index 0000000000..a499368524 --- /dev/null +++ b/tests/framework/db/pgsql/BaseActiveRecordTest.php @@ -0,0 +1,46 @@ + new JsonExpression($actual), + ]); + + $createdStorage->save(); + + $foundStorage = ArrayAndJsonType::find()->limit(1)->one(); + + $this->assertNotNull($foundStorage); + + $foundStorage->json_col = $modified; + + $this->assertSame(['json_col' => $modified], $foundStorage->getDirtyAttributes()); + } +} + +/** + * {@inheritdoc} + * @property array id + * @property array json_col + */ +class ArrayAndJsonType extends ActiveRecord +{ + public static function tableName() + { + return '{{%array_and_json_types}}'; + } +} From 786a75ca639b1532eac98de9cbe269d52a69d1d6 Mon Sep 17 00:00:00 2001 From: Safuan Date: Fri, 18 Aug 2023 14:20:46 +0300 Subject: [PATCH 4/5] Fix incorrect translation in db-query-builder.md (#19916) --- docs/guide-ru/db-query-builder.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide-ru/db-query-builder.md b/docs/guide-ru/db-query-builder.md index 41b164124e..50bf94771e 100644 --- a/docs/guide-ru/db-query-builder.md +++ b/docs/guide-ru/db-query-builder.md @@ -373,7 +373,7 @@ $query->orderBy([ ``` В данном коде, ключи массива - это имена столбцов, а значения массива - это соответствующее направление сортировки. -PHP константа `SORT_ASC` определяет сортировку по возрастанию и `SORT_DESC` сортировка по умолчанию. +PHP константа `SORT_ASC` определяет сортировку по возрастанию и `SORT_DESC` сортировку по убыванию. Если `ORDER BY` содержит только простые имена столбцов, вы можете определить их с помощью столбцов, также как и при написании обычного SQL. Например, From e916e9d564ab0e24326f35f53eb62bb131a62edb Mon Sep 17 00:00:00 2001 From: rhertogh Date: Fri, 18 Aug 2023 13:31:09 +0200 Subject: [PATCH 5/5] Fix #19914: Fixed `ArrayHelper::keyExists()` and `::remove()` functions when the key is a float and the value is `null` --- framework/CHANGELOG.md | 1 + framework/db/BaseActiveRecord.php | 4 +- framework/helpers/BaseArrayHelper.php | 18 +++++-- tests/framework/helpers/ArrayHelperTest.php | 60 +++++++++++++++++++-- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index eaa8bc6154..4ed527b0a7 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -16,6 +16,7 @@ Yii Framework 2 Change Log - Enh #19884: Added support Enums in Query Builder (sk1t0n) - Bug #19908: Fix associative array cell content rendering in Table widget (rhertogh) - Bug #19906: Fixed multiline strings in the `\yii\console\widgets\Table` widget (rhertogh) +- Bug #19914: Fixed `ArrayHelper::keyExists()` and `::remove()` functions when the key is a float and the value is `null` (rhertogh) - Enh #19920: Broadened the accepted type of `Cookie::$expire` from `int` to `int|string|\DateTimeInterface|null` (rhertogh) diff --git a/framework/db/BaseActiveRecord.php b/framework/db/BaseActiveRecord.php index 2648a48dae..60f621eb44 100644 --- a/framework/db/BaseActiveRecord.php +++ b/framework/db/BaseActiveRecord.php @@ -282,7 +282,7 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface */ public function __get($name) { - if (isset($this->_attributes[$name]) || array_key_exists($name, $this->_attributes)) { + if (array_key_exists($name, $this->_attributes)) { return $this->_attributes[$name]; } @@ -290,7 +290,7 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface return null; } - if (isset($this->_related[$name]) || array_key_exists($name, $this->_related)) { + if (array_key_exists($name, $this->_related)) { return $this->_related[$name]; } $value = parent::__get($name); diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index 0e615a579d..56411163e1 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -327,7 +327,12 @@ class BaseArrayHelper */ public static function remove(&$array, $key, $default = null) { - if (is_array($array) && (isset($array[$key]) || array_key_exists($key, $array))) { + // ToDo: This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) + if (is_float($key)) { + $key = (int)$key; + } + + if (is_array($array) && array_key_exists($key, $array)) { $value = $array[$key]; unset($array[$key]); @@ -608,17 +613,20 @@ class BaseArrayHelper * Checks if the given array contains the specified key. * This method enhances the `array_key_exists()` function by supporting case-insensitive * key comparison. - * @param string $key the key to check + * @param string|int $key the key to check * @param array|ArrayAccess $array the array with keys to check * @param bool $caseSensitive whether the key comparison should be case-sensitive * @return bool whether the array contains the specified key */ public static function keyExists($key, $array, $caseSensitive = true) { + // ToDo: This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) + if (is_float($key)) { + $key = (int)$key; + } + if ($caseSensitive) { - // Function `isset` checks key faster but skips `null`, `array_key_exists` handles this case - // https://www.php.net/manual/en/function.array-key-exists.php#107786 - if (is_array($array) && (isset($array[$key]) || array_key_exists($key, $array))) { + if (is_array($array) && array_key_exists($key, $array)) { return true; } // Cannot use `array_has_key` on Objects for PHP 7.4+, therefore we need to check using [[ArrayAccess::offsetExists()]] diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 063eeafcaf..0d706bc552 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -135,6 +135,29 @@ class ArrayHelperTest extends TestCase $this->assertEquals('defaultValue', $default); } + /** + * @return void + */ + public function testRemoveWithFloat() + { + if (version_compare(PHP_VERSION, '8.1.0', '>=')) { + $this->markTestSkipped('Using floats as array key is deprecated.'); + } + + $array = ['name' => 'b', 'age' => 3, 1.1 => null]; + + $name = ArrayHelper::remove($array, 'name'); + $this->assertEquals($name, 'b'); + $this->assertEquals($array, ['age' => 3, 1.1 => null]); + + $floatVal = ArrayHelper::remove($array, 1.1); + $this->assertNull($floatVal); + $this->assertEquals($array, ['age' => 3]); + + $default = ArrayHelper::remove($array, 'nonExisting', 'defaultValue'); + $this->assertEquals('defaultValue', $default); + } + public function testRemoveValueMultiple() { $array = [ @@ -506,14 +529,21 @@ class ArrayHelperTest extends TestCase /** * @see https://github.com/yiisoft/yii2/pull/11549 */ - public function test() + public function testGetValueWithFloatKeys() { + if (version_compare(PHP_VERSION, '8.1.0', '>=')) { + $this->markTestSkipped('Using floats as array key is deprecated.'); + } + $array = []; - $array[1.0] = 'some value'; - - $result = ArrayHelper::getValue($array, 1.0); + $array[1.1] = 'some value'; + $array[2.1] = null; + $result = ArrayHelper::getValue($array, 1.2); $this->assertEquals('some value', $result); + + $result = ArrayHelper::getValue($array, 2.2); + $this->assertNull($result); } public function testIndex() @@ -712,6 +742,7 @@ class ArrayHelperTest extends TestCase 'a' => 1, 'B' => 2, ]; + $this->assertTrue(ArrayHelper::keyExists('a', $array)); $this->assertFalse(ArrayHelper::keyExists('b', $array)); $this->assertTrue(ArrayHelper::keyExists('B', $array)); @@ -723,6 +754,27 @@ class ArrayHelperTest extends TestCase $this->assertFalse(ArrayHelper::keyExists('c', $array, false)); } + public function testKeyExistsWithFloat() + { + if (version_compare(PHP_VERSION, '8.1.0', '>=')) { + $this->markTestSkipped('Using floats as array key is deprecated.'); + } + + $array = [ + 1 => 3, + 2.2 => 4, // Note: Floats are cast to ints, which means that the fractional part will be truncated. + 3.3 => null, + ]; + + $this->assertTrue(ArrayHelper::keyExists(1, $array)); + $this->assertTrue(ArrayHelper::keyExists(1.1, $array)); + $this->assertTrue(ArrayHelper::keyExists(2, $array)); + $this->assertTrue(ArrayHelper::keyExists('2', $array)); + $this->assertTrue(ArrayHelper::keyExists(2.2, $array)); + $this->assertTrue(ArrayHelper::keyExists(3, $array)); + $this->assertTrue(ArrayHelper::keyExists(3.3, $array)); + } + public function testKeyExistsArrayAccess() { $array = new TraversableArrayAccessibleObject([