From d49428c97060719f84f447cf0d741a98d7838f80 Mon Sep 17 00:00:00 2001 From: Bizley Date: Mon, 24 Jan 2022 19:10:50 +0100 Subject: [PATCH 1/4] Case-sensitive header names kept in addition to the normalized ones --- framework/CHANGELOG.md | 1 + framework/UPGRADE.md | 6 ++ framework/filters/PageCache.php | 47 +++++++---- framework/web/HeaderCollection.php | 60 ++++++++----- tests/framework/filters/PageCacheTest.php | 18 ++++ tests/framework/web/HeaderCollectionTest.php | 88 ++++++++++++++++++++ 6 files changed, 185 insertions(+), 35 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index dc7f5f50684..e9416f4df37 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -9,6 +9,7 @@ Yii Framework 2 Change Log - Bug #19148: Fix undefined array key errors in `yii\db\ActiveRelationTrait` (stevekr) - Bug #19041: Fix PHP 8.1 issues (longthanhtran, samdark, pamparam83, sartor, githubjeka) - Enh #19171: Added `$pagination` and `$sort` to `\yii\rest\IndexAction` for easy configuration (rhertogh) +- Bug #19187: Fix `yii\filters\PageCache` to store original headers names instead of normalized ones (bizley) 2.0.44 December 30, 2021 diff --git a/framework/UPGRADE.md b/framework/UPGRADE.md index 8a9a20a4737..76052af2bdd 100644 --- a/framework/UPGRADE.md +++ b/framework/UPGRADE.md @@ -51,6 +51,12 @@ if you want to upgrade from version A to version C and there is version B between A and C, you need to follow the instructions for both A and B. +Upgrade from Yii 2.0.44 +----------------------- + +* `yii\filters\PageCache::$cacheHeaders` now takes a case-sensitive list of header names since PageCache is no longer + storing the normalized versions of them. + Upgrade from Yii 2.0.43 ----------------------- diff --git a/framework/filters/PageCache.php b/framework/filters/PageCache.php index 1930aa0873f..f02bf2f3826 100644 --- a/framework/filters/PageCache.php +++ b/framework/filters/PageCache.php @@ -129,7 +129,7 @@ class PageCache extends ActionFilter implements DynamicContentAwareInterface public $cacheCookies = false; /** * @var bool|array a boolean value indicating whether to cache all HTTP headers, or an array of - * HTTP header names (case-insensitive) indicating which HTTP headers can be cached. + * HTTP header names (case-sensitive) indicating which HTTP headers can be cached. * Note if your HTTP headers contain sensitive information, you should white-list which headers can be cached. * @since 2.0.4 */ @@ -253,40 +253,59 @@ public function cacheResponse() foreach (['format', 'version', 'statusCode', 'statusText'] as $name) { $data[$name] = $response->{$name}; } - $this->insertResponseCollectionIntoData($response, 'headers', $data); - $this->insertResponseCollectionIntoData($response, 'cookies', $data); + $this->insertResponseHeaderCollectionIntoData($response, $data); + $this->insertResponseCookieCollectionIntoData($response, $data); $this->cache->set($this->calculateCacheKey(), $data, $this->duration, $this->dependency); $data['content'] = $this->updateDynamicContent($data['content'], $this->getDynamicPlaceholders()); echo $data['content']; } /** - * Inserts (or filters/ignores according to config) response headers/cookies into a cache data array. + * Inserts (or filters/ignores according to config) response cookies into a cache data array. * @param Response $response the response. - * @param string $collectionName currently it's `headers` or `cookies`. * @param array $data the cache data. */ - private function insertResponseCollectionIntoData(Response $response, $collectionName, array &$data) + private function insertResponseCookieCollectionIntoData(Response $response, array &$data) { - $property = 'cache' . ucfirst($collectionName); - if ($this->{$property} === false) { + if ($this->cacheCookies === false) { return; } - $all = $response->{$collectionName}->toArray(); - if (is_array($this->{$property})) { + $all = $response->cookies->toArray(); + if (is_array($this->cacheCookies)) { $filtered = []; - foreach ($this->{$property} as $name) { - if ($collectionName === 'headers') { - $name = strtolower($name); + foreach ($this->cacheCookies as $name) { + if (isset($all[$name])) { + $filtered[$name] = $all[$name]; } + } + $all = $filtered; + } + $data['cookies'] = $all; + } + + /** + * Inserts (or filters/ignores according to config) response headers into a cache data array. + * @param Response $response the response. + * @param array $data the cache data. + */ + private function insertResponseHeaderCollectionIntoData(Response $response, array &$data) + { + if ($this->cacheHeaders === false) { + return; + } + + $all = $response->headers->toArray(true); + if (is_array($this->cacheHeaders)) { + $filtered = []; + foreach ($this->cacheHeaders as $name) { if (isset($all[$name])) { $filtered[$name] = $all[$name]; } } $all = $filtered; } - $data[$collectionName] = $all; + $data['headers'] = $all; } /** diff --git a/framework/web/HeaderCollection.php b/framework/web/HeaderCollection.php index 737e2b5c1ce..68ac2e9fbf5 100644 --- a/framework/web/HeaderCollection.php +++ b/framework/web/HeaderCollection.php @@ -22,9 +22,13 @@ class HeaderCollection extends BaseObject implements \IteratorAggregate, \ArrayAccess, \Countable { /** - * @var array the headers in this collection (indexed by the header names) + * @var array the headers in this collection (indexed by the normalized header names) */ private $_headers = []; + /** + * @var array the original names of the headers (indexed by the normalized header names) + */ + private $_originalHeaderNames = []; /** @@ -72,9 +76,9 @@ public function getCount() */ public function get($name, $default = null, $first = true) { - $name = strtolower($name); - if (isset($this->_headers[$name])) { - return $first ? reset($this->_headers[$name]) : $this->_headers[$name]; + $normalizedName = strtolower($name); + if (isset($this->_headers[$normalizedName])) { + return $first ? reset($this->_headers[$normalizedName]) : $this->_headers[$normalizedName]; } return $default; @@ -89,8 +93,9 @@ public function get($name, $default = null, $first = true) */ public function set($name, $value = '') { - $name = strtolower($name); - $this->_headers[$name] = (array) $value; + $normalizedName = strtolower($name); + $this->_headers[$normalizedName] = (array) $value; + $this->_originalHeaderNames[$normalizedName] = $name; return $this; } @@ -105,8 +110,11 @@ public function set($name, $value = '') */ public function add($name, $value) { - $name = strtolower($name); - $this->_headers[$name][] = $value; + $normalizedName = strtolower($name); + $this->_headers[$normalizedName][] = $value; + if (!\array_key_exists($normalizedName, $this->_originalHeaderNames)) { + $this->_originalHeaderNames[$normalizedName] = $name; + } return $this; } @@ -120,9 +128,10 @@ public function add($name, $value) */ public function setDefault($name, $value) { - $name = strtolower($name); - if (empty($this->_headers[$name])) { - $this->_headers[$name][] = $value; + $normalizedName = strtolower($name); + if (empty($this->_headers[$normalizedName])) { + $this->_headers[$normalizedName][] = $value; + $this->_originalHeaderNames[$normalizedName] = $name; } return $this; @@ -135,9 +144,7 @@ public function setDefault($name, $value) */ public function has($name) { - $name = strtolower($name); - - return isset($this->_headers[$name]); + return isset($this->_headers[strtolower($name)]); } /** @@ -147,10 +154,10 @@ public function has($name) */ public function remove($name) { - $name = strtolower($name); - if (isset($this->_headers[$name])) { - $value = $this->_headers[$name]; - unset($this->_headers[$name]); + $normalizedName = strtolower($name); + if (isset($this->_headers[$normalizedName])) { + $value = $this->_headers[$normalizedName]; + unset($this->_headers[$normalizedName], $this->_originalHeaderNames[$normalizedName]); return $value; } @@ -163,16 +170,25 @@ public function remove($name) public function removeAll() { $this->_headers = []; + $this->_originalHeaderNames = []; } /** * Returns the collection as a PHP array. * @return array the array representation of the collection. * The array keys are header names, and the array values are the corresponding header values. + * Since 2.0.45 you can pass true here to get the headers list of original header names (case-sensitive) instead of + * default normalized (case-insensitive) ones. */ - public function toArray() + public function toArray($originalNames = false) { - return $this->_headers; + if ($originalNames === false) { + return $this->_headers; + } + + return \array_map(function ($normalizedName) { + return $this->_headers[$normalizedName]; + }, \array_flip($this->_originalHeaderNames)); } /** @@ -182,7 +198,9 @@ public function toArray() */ public function fromArray(array $array) { - $this->_headers = array_change_key_case($array, CASE_LOWER); + foreach ($array as $name => $value) { + $this->set($name, $value); + } } /** diff --git a/tests/framework/filters/PageCacheTest.php b/tests/framework/filters/PageCacheTest.php index f13ce4890c0..09d2e11fb7b 100644 --- a/tests/framework/filters/PageCacheTest.php +++ b/tests/framework/filters/PageCacheTest.php @@ -116,6 +116,16 @@ public function cacheTestCaseProvider() 'test-header-2' => false, ], ]], + [[ + 'name' => 'originalNameHeaders', + 'properties' => [ + 'cacheHeaders' => ['Test-Header-1'], + ], + 'headers' => [ + 'Test-Header-1' => true, + 'Test-Header-2' => false, + ], + ]], // All together [[ @@ -233,12 +243,20 @@ public function testCache($testCase) } // Headers if (isset($testCase['headers'])) { + $headersExpected = false; foreach ($testCase['headers'] as $name => $expected) { $this->assertSame($expected, Yii::$app->response->headers->has($name), $testCase['name']); if ($expected) { + $headersExpected = true; $this->assertSame($headers[$name], Yii::$app->response->headers->get($name), $testCase['name']); } } + if ($headersExpected) { + $this->assertSame( + \array_keys($testCase['headers']), + \array_keys(Yii::$app->response->headers->toArray(true)) + ); + } } } diff --git a/tests/framework/web/HeaderCollectionTest.php b/tests/framework/web/HeaderCollectionTest.php index 7fe9c889f54..9569ba3f3ab 100644 --- a/tests/framework/web/HeaderCollectionTest.php +++ b/tests/framework/web/HeaderCollectionTest.php @@ -24,4 +24,92 @@ public function testFromArray() ]); $this->assertEquals($location, $headerCollection->get('Location')); } + + public function testSetter() + { + $headerCollection = new HeaderCollection(); + + $this->assertSame('default', $headerCollection->get('X-Header', 'default')); + $this->assertFalse($headerCollection->has('X-Header')); + + $headerCollection->set('X-Header', '1'); + $this->assertTrue($headerCollection->has('X-Header')); + $this->assertTrue($headerCollection->offsetExists('X-Header')); + $this->assertSame('1', $headerCollection->get('X-Header')); + $this->assertSame(['1'], $headerCollection->get('X-Header', null, false)); + $this->assertTrue($headerCollection->has('x-header')); + $this->assertSame('1', $headerCollection->get('x-header')); + $this->assertSame(['1'], $headerCollection->get('x-header', null, false)); + $this->assertSame('1', $headerCollection->get('x-hEadER')); + $this->assertSame(['1'], $headerCollection->get('x-hEadER', null, false)); + $this->assertSame(['x-header' => ['1']], $headerCollection->toArray()); + $this->assertSame(['X-Header' => ['1']], $headerCollection->toArray(true)); + + $headerCollection->set('X-HEADER', '2'); + $this->assertSame('2', $headerCollection->get('X-Header')); + $this->assertSame('2', $headerCollection->get('x-header')); + $this->assertSame('2', $headerCollection->get('x-hEadER')); + $this->assertSame(['x-header' => ['2']], $headerCollection->toArray()); + $this->assertSame(['X-HEADER' => ['2']], $headerCollection->toArray(true)); + + $headerCollection->offsetSet('X-HEADER', '3'); + $this->assertSame('3', $headerCollection->get('X-Header')); + } + + public function testSetterDefault() + { + $headerCollection = new HeaderCollection(); + $headerCollection->setDefault('X-Header', '1'); + $this->assertSame(['1'], $headerCollection->get('X-Header', null, false)); + + $headerCollection->setDefault('X-Header', '2'); + $this->assertSame(['1'], $headerCollection->get('X-Header', null, false)); + } + + public function testAdder() + { + $headerCollection = new HeaderCollection(); + $headerCollection->add('X-Header', '1'); + $this->assertSame('1', $headerCollection->get('X-Header')); + $this->assertSame(['1'], $headerCollection->get('X-Header', null, false)); + $this->assertSame('1', $headerCollection->get('x-header')); + $this->assertSame(['1'], $headerCollection->get('x-header', null, false)); + $this->assertSame('1', $headerCollection->get('x-hEadER')); + $this->assertSame(['1'], $headerCollection->get('x-hEadER', null, false)); + $this->assertSame(['x-header' => ['1']], $headerCollection->toArray()); + $this->assertSame(['X-Header' => ['1']], $headerCollection->toArray(true)); + + $headerCollection->add('X-HEADER', '2'); + $this->assertSame('1', $headerCollection->get('X-Header')); + $this->assertSame('1', $headerCollection->get('x-header')); + $this->assertSame('1', $headerCollection->get('x-hEadER')); + $this->assertSame(['1', '2'], $headerCollection->get('x-header', null, false)); + $this->assertSame(['x-header' => ['1', '2']], $headerCollection->toArray()); + $this->assertSame(['X-Header' => ['1', '2']], $headerCollection->toArray(true)); + } + + public function testRemover() + { + $headerCollection = new HeaderCollection(); + $headerCollection->add('X-Header', '1'); + $this->assertSame(1, $headerCollection->count()); + $this->assertSame(1, $headerCollection->getCount()); + $headerCollection->remove('X-Header'); + $this->assertSame(0, $headerCollection->count()); + + $headerCollection->add('X-Header', '1'); + $this->assertSame(1, $headerCollection->count()); + $headerCollection->remove('x-header'); + $this->assertSame(0, $headerCollection->count()); + + $headerCollection->add('X-Header', '1'); + $headerCollection->offsetUnset('X-HEADER'); + $this->assertSame(0, $headerCollection->count()); + + $headerCollection->add('X-Header-1', '1'); + $headerCollection->add('X-Header-2', '1'); + $this->assertSame(2, $headerCollection->count()); + $headerCollection->removeAll(); + $this->assertSame(0, $headerCollection->count()); + } } From 57b2a196557ae52e7a517584e2c63d7f6892c2de Mon Sep 17 00:00:00 2001 From: Bizley Date: Mon, 24 Jan 2022 19:20:45 +0100 Subject: [PATCH 2/4] PageCache headers name test --- tests/framework/filters/PageCacheTest.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/framework/filters/PageCacheTest.php b/tests/framework/filters/PageCacheTest.php index 09d2e11fb7b..1f0896f65ba 100644 --- a/tests/framework/filters/PageCacheTest.php +++ b/tests/framework/filters/PageCacheTest.php @@ -243,20 +243,14 @@ public function testCache($testCase) } // Headers if (isset($testCase['headers'])) { - $headersExpected = false; + $headersExpected = Yii::$app->response->headers->toArray(true); foreach ($testCase['headers'] as $name => $expected) { $this->assertSame($expected, Yii::$app->response->headers->has($name), $testCase['name']); if ($expected) { - $headersExpected = true; $this->assertSame($headers[$name], Yii::$app->response->headers->get($name), $testCase['name']); + $this->assertArrayHasKey($name, $headersExpected); } } - if ($headersExpected) { - $this->assertSame( - \array_keys($testCase['headers']), - \array_keys(Yii::$app->response->headers->toArray(true)) - ); - } } } From a82cc2c468b41a4338dd63e064b7b0dee10c4afe Mon Sep 17 00:00:00 2001 From: Bizley Date: Tue, 25 Jan 2022 08:59:20 +0100 Subject: [PATCH 3/4] Update UPGRADE.md --- framework/UPGRADE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/UPGRADE.md b/framework/UPGRADE.md index 76052af2bdd..fd6d35a1f87 100644 --- a/framework/UPGRADE.md +++ b/framework/UPGRADE.md @@ -55,7 +55,7 @@ Upgrade from Yii 2.0.44 ----------------------- * `yii\filters\PageCache::$cacheHeaders` now takes a case-sensitive list of header names since PageCache is no longer - storing the normalized versions of them. + storing the normalized (lowercase) versions of them so make sure this list is properly updated. Upgrade from Yii 2.0.43 ----------------------- From 349ce12e5771cb2b237cfc8693535a75dd1ac607 Mon Sep 17 00:00:00 2001 From: Bizley Date: Tue, 25 Jan 2022 09:21:47 +0100 Subject: [PATCH 4/4] Update UPGRADE.md --- framework/UPGRADE.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/framework/UPGRADE.md b/framework/UPGRADE.md index fd6d35a1f87..81a09a6ad0c 100644 --- a/framework/UPGRADE.md +++ b/framework/UPGRADE.md @@ -55,12 +55,14 @@ Upgrade from Yii 2.0.44 ----------------------- * `yii\filters\PageCache::$cacheHeaders` now takes a case-sensitive list of header names since PageCache is no longer - storing the normalized (lowercase) versions of them so make sure this list is properly updated. + storing the normalized (lowercase) versions of them so make sure this list is properly updated and your page cache + is recreated. Upgrade from Yii 2.0.43 ----------------------- -* `Json::encode()` can now handle zero-indexed objects in same way as `json_encode()` and keep them as objects. In order to avoid breaking backwards compatibility this behavior could be enabled by a new option flag but is disabled by default. +* `Json::encode()` can now handle zero-indexed objects in same way as `json_encode()` and keep them as objects. In order + to avoid breaking backwards compatibility this behavior could be enabled by a new option flag but is disabled by default. * Set `yii/helpers/Json::$keepObjectType = true` anywhere in your application code * Or configure json response formatter to enable it for all JSON responses: ```php @@ -74,7 +76,8 @@ Upgrade from Yii 2.0.43 ], ], ``` -* `yii\caching\Cache::multiSet()` now uses the default cache duration (`yii\caching\Cache::$defaultDuration`) when no duration is provided. A duration of 0 should be explicitly passed if items should not expire. +* `yii\caching\Cache::multiSet()` now uses the default cache duration (`yii\caching\Cache::$defaultDuration`) when no + duration is provided. A duration of 0 should be explicitly passed if items should not expire. Upgrade from Yii 2.0.42 -----------------------