Fix #19187: Fix yii\filters\PageCache to store original headers nam…

…es instead of normalized ones
yiisoft · Jan 26, 2022 · dbb157f · dbb157f
1 parent 0eaa71d
commit dbb157f
Show file tree

Hide file tree

Showing 6 changed files with 184 additions and 37 deletions.
diff --git 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)
 - Bug #19191: Change `\Exception` to `\Throwable` in `BadRequestHttpException` and `HttpException` (Dmitrijlin)
 
 

diff --git a/framework/UPGRADE.md b/framework/UPGRADE.md
@@ -51,10 +51,18 @@ 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 (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
@@ -68,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
 -----------------------

diff --git 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
@@ -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
@@ -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,10 +243,12 @@ public function testCache($testCase)
         }
         // Headers
         if (isset($testCase['headers'])) {
+            $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) {
                     $this->assertSame($headers[$name], Yii::$app->response->headers->get($name), $testCase['name']);
+                    $this->assertArrayHasKey($name, $headersExpected);
                 }
             }
         }

diff --git 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());
+    }
 }