Merge pull request #6475 from JavaDeveloperKiev/cachingBugFix

Fix broken caching system when array of allowed parameters used
codeigniter4 · Sep 4, 2022 · a8072a5 · a8072a5
2 parents 51e67a7 + 2b5dae8
commit a8072a5
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 2 deletions.
diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php
@@ -730,9 +730,12 @@ protected function generateCacheName(Cache $config): string
         }
 
         $uri = $this->request->getUri();
-
         if ($config->cacheQueryString) {
-            $name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery());
+            if (is_array($config->cacheQueryString)) {
+                $name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(['only' => $config->cacheQueryString]));
+            } else {
+                $name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery());
+            }
         } else {
             $name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath());
         }

diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php
@@ -18,6 +18,7 @@
 use CodeIgniter\Test\Filters\CITestStreamFilter;
 use CodeIgniter\Test\Mock\MockCodeIgniter;
 use Config\App;
+use Config\Cache;
 use Config\Filters;
 use Config\Modules;
 use Tests\Support\Filters\Customfilter;
@@ -622,4 +623,81 @@ public function testPageCacheSendSecureHeaders()
         stream_filter_remove($outputStreamFilter);
         stream_filter_remove($errorStreamFilter);
     }
+
+    /**
+     * @param array|bool $cacheQueryStringValue
+     * @dataProvider cacheQueryStringProvider
+     *
+     * @see https://github.com/codeigniter4/CodeIgniter4/pull/6410
+     */
+    public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $expectedPagesInCache, array $testingUrls)
+    {
+        // Suppress command() output
+        CITestStreamFilter::$buffer = '';
+        $outputStreamFilter         = stream_filter_append(STDOUT, 'CITestStreamFilter');
+        $errorStreamFilter          = stream_filter_append(STDERR, 'CITestStreamFilter');
+
+        // Create cache config with cacheQueryString value from the dataProvider
+        $cacheConfig                   = new Cache();
+        $cacheConfig->cacheQueryString = $cacheQueryStringValue;
+
+        // Clear cache before starting the test
+        command('cache:clear');
+
+        // Calculate amount of items in the cache before the test
+        $cache             = \Config\Services::cache();
+        $cacheStartCounter = count($cache->getCacheInfo());
+
+        // Generate request to each URL from the testing array
+        foreach ($testingUrls as $testingUrl) {
+            $_SERVER['REQUEST_URI'] = '/' . $testingUrl;
+            $routes                 = Services::routes(true);
+            $routes->add($testingUrl, static function () {
+                CodeIgniter::cache(0); // Dont cache the page in the run() function because CodeIgniter class will create default $cacheConfig and overwrite settings from the dataProvider
+                $response = Services::response();
+                $string = 'This is a test page, to check cache configuration';
+
+                return $response->setBody($string);
+            });
+
+            // Inject router
+            $router = Services::router($routes, Services::request(null, false));
+            Services::injectMock('router', $router);
+
+            // Cache the page output using default caching function and $cacheConfig with value from the data provider
+            $this->codeigniter->useSafeOutput(true)->run();
+            $this->codeigniter->cachePage($cacheConfig); // Cache the page using our own $cacheConfig confugration
+        }
+
+        // Calculate how much cached items exist in the cache after the test requests
+        $cacheEndCounter = count($cache->getCacheInfo());
+        $newPagesCached  = $cacheEndCounter - $cacheStartCounter;
+
+        // Clear cache after the test
+        command('cache:clear');
+
+        // Check that amount of new items created in the cache matching expected value from the data provider
+        $this->assertSame($expectedPagesInCache, $newPagesCached);
+
+        // Remove stream filters
+        stream_filter_remove($outputStreamFilter);
+        stream_filter_remove($errorStreamFilter);
+    }
+
+    public function cacheQueryStringProvider(): array
+    {
+        $testingUrls = [
+            'test', // URL #1
+            'test?important_parameter=1', // URL #2
+            'test?important_parameter=2',  // URL #3
+            'test?important_parameter=1&not_important_parameter=2', // URL #4
+            'test?important_parameter=1&not_important_parameter=2&another_not_important_parameter=3', // URL #5
+        ];
+
+        return [
+            '$cacheQueryString=false' => [false, 1, $testingUrls], // We expect only 1 page in the cache, because when cacheQueryString is set to false, all GET parameter should be ignored, and page URI will be absolutely same "/test" string for all 5 requests
+            '$cacheQueryString=true'  => [true, 5, $testingUrls], // We expect all 5 pages in the cache, because when cacheQueryString is set to true, all GET parameter should be processed as unique requests
+            '$cacheQueryString=array' => [['important_parameter'], 3, $testingUrls], // We expect only 3 pages in the cache, because when cacheQueryString is set to array with important parameters, we should ignore all parameters thats not in the array. Only URL #1, URL #2 and URL #3 should be cached. URL #4 and URL #5 is duplication of URL #2 (with value ?important_parameter=1), so they should not be processed as new unique requests and application should return already cached page for URL #2
+        ];
+    }
 }