[BUGFIX] substituteMarkerArrayCached() must accept special chars 36/45336/4
authorMarkus Klein <markus.klein@typo3.org>
Thu, 17 Dec 2015 14:44:37 +0000 (15:44 +0100)
committerMarkus Klein <markus.klein@typo3.org>
Thu, 17 Dec 2015 15:03:19 +0000 (16:03 +0100)
Add a bunch of unittests and streamline the code as well
by removing a useless preg_match_all() call.
Rename some variables and add comments.

Resolves: #72252
Releases: master, 6.2
Change-Id: I2a31a1c2ab6d83528428693213b922f0e1bc6fe5
Reviewed-on: https://review.typo3.org/45336
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
typo3/sysext/frontend/Tests/Unit/ContentObject/Fixtures/PageRepositoryFixture.php [new file with mode: 0644]

index 41d8ce8..f80f511 100644 (file)
@@ -1844,15 +1844,15 @@ class ContentObjectRenderer {
                // Finding keys and check hash:
                $sPkeys = array_keys($subpartContentArray);
                $wPkeys = array_keys($wrappedSubpartContentArray);
-               $aKeys = array_merge(array_keys($markContentArray), $sPkeys, $wPkeys);
-               if (!count($aKeys)) {
+               $keysToReplace = array_merge(array_keys($markContentArray), $sPkeys, $wPkeys);
+               if (empty($keysToReplace)) {
                        $GLOBALS['TT']->pull();
                        return $content;
                }
-               asort($aKeys);
+               asort($keysToReplace);
                $storeKey = md5('substituteMarkerArrayCached_storeKey:' . serialize(array(
                        $content,
-                       $aKeys
+                       $keysToReplace
                )));
                if ($this->substMarkerCache[$storeKey]) {
                        $storeArr = $this->substMarkerCache[$storeKey];
@@ -1878,31 +1878,25 @@ class ContentObjectRenderer {
                                }
 
                                $storeArr = array();
-                               $result = preg_match_all('/###([\w:-]+)###/', $content, $usedMarkers);
-                               if ($result !== FALSE && !empty($usedMarkers[1])) {
-                                       $tagArray = array_flip($usedMarkers[1]);
-
-                                       $aKeys = array_flip($aKeys);
-                                       $bKeys = array();
+                               $result = preg_match_all('/###([^#](?:[^#]*+|#{1,2}[^#])+)###/', $content, $markersInContent);
+                               if ($result !== FALSE && !empty($markersInContent[1])) {
+                                       $keysToReplaceFlipped = array_flip($keysToReplace);
+                                       $regexKeys = array();
+                                       $wrappedKeys = array();
                                        // Traverse keys and quote them for reg ex.
-                                       foreach ($tagArray as $tV => $tK) {
-                                               $tV = '###' . $tV . '###';
-                                               if (isset($aKeys[$tV])) {
-                                                       $bKeys[$tK] = preg_quote($tV, '/');
+                                       foreach ($markersInContent[1] as $key) {
+                                               if (isset($keysToReplaceFlipped['###' . $key . '###'])) {
+                                                       $regexKeys[] = preg_quote($key, '/');
+                                                       $wrappedKeys[] = '###' . $key . '###';
                                                }
                                        }
-                                       $regex = '/' . implode('|', $bKeys) . '/';
-                                       // Doing regex's
-                                       if (preg_match_all($regex, $content, $keyList) !== FALSE) {
-                                               $storeArr['c'] = preg_split($regex, $content);
-                                               $storeArr['k'] = $keyList[0];
-                                       }
-                                       if (!empty($storeArr)) {
-                                               // Setting cache:
-                                               $this->substMarkerCache[$storeKey] = $storeArr;
-                                               // Storing the cached data:
-                                               $GLOBALS['TSFE']->sys_page->storeHash($storeKey, $storeArr, 'substMarkArrayCached');
-                                       }
+                                       $regex = '/###(?:' . implode('|', $regexKeys) . ')###/';
+                                       $storeArr['c'] = preg_split($regex, $content); // contains all content parts around markers
+                                       $storeArr['k'] = $wrappedKeys; // contains all markers incl. ###
+                                       // Setting cache:
+                                       $this->substMarkerCache[$storeKey] = $storeArr;
+                                       // Storing the cached data:
+                                       $GLOBALS['TSFE']->sys_page->storeHash($storeKey, $storeArr, 'substMarkArrayCached');
                                }
                                $GLOBALS['TT']->setTSlogMessage('Parsing', 0);
                        }
@@ -1915,14 +1909,21 @@ class ContentObjectRenderer {
                        $content = '';
                        // Traversing the keyList array and merging the static and dynamic content
                        foreach ($storeArr['k'] as $n => $keyN) {
+                               // add content before marker
                                $content .= $storeArr['c'][$n];
                                if (!is_array($valueArr[$keyN])) {
+                                       // fetch marker replacement from $markContentArray or $subpartContentArray
                                        $content .= $valueArr[$keyN];
                                } else {
-                                       $content .= $valueArr[$keyN][(int)$wSCA_reg[$keyN] % 2];
+                                       if (!isset($wSCA_reg[$keyN])) {
+                                               $wSCA_reg[$keyN] = 0;
+                                       }
+                                       // fetch marker replacement from $wrappedSubpartContentArray
+                                       $content .= $valueArr[$keyN][$wSCA_reg[$keyN] % 2];
                                        $wSCA_reg[$keyN]++;
                                }
                        }
+                       // add remaining content
                        $content .= $storeArr['c'][count($storeArr['k'])];
                }
                $GLOBALS['TT']->pull();
index a660c36..d74133c 100644 (file)
@@ -13,6 +13,9 @@ namespace TYPO3\CMS\Frontend\Tests\Unit\ContentObject;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
+use TYPO3\CMS\Core\TimeTracker\NullTimeTracker;
+
 /**
  * Testcase for TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer
  *
@@ -46,9 +49,10 @@ class ContentObjectRendererTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->tsfe->tmpl = $this->template;
                $this->tsfe->config = array();
                $this->tsfe->page = array();
-               $sysPageMock = $this->getMock('TYPO3\\CMS\\Frontend\\Page\\PageRepository', array('getRawRecord'));
+               $sysPageMock = $this->getMock('TYPO3\\CMS\\Frontend\\Tests\\Unit\\ContentObject\\Fixtures\\PageRepositoryFixture', array('getRawRecord'));
                $this->tsfe->sys_page = $sysPageMock;
                $GLOBALS['TSFE'] = $this->tsfe;
+               $GLOBALS['TT'] = new NullTimeTracker();
                $GLOBALS['TSFE']->csConvObj = new \TYPO3\CMS\Core\Charset\CharsetConverter();
                $GLOBALS['TSFE']->renderCharset = 'utf-8';
                $GLOBALS['TYPO3_CONF_VARS']['SYS']['TYPO3\\CMS\\Core\\Charset\\CharsetConverter_utils'] = 'mbstring';
@@ -3395,6 +3399,218 @@ class ContentObjectRendererTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->assertEquals($expectedResult, $this->cObj->typoLink($linkText, $configuration));
        }
 
+       /**
+        * @return array
+        */
+       public function substituteMarkerArrayCachedReturnsExpectedContentDataProvider() {
+               return array(
+                       'no markers defined' => array(
+                               'dummy content with ###UNREPLACED### marker',
+                               array(),
+                               array(),
+                               array(),
+                               'dummy content with ###UNREPLACED### marker',
+                               false,
+                               false
+                       ),
+                       'no markers used' => array(
+                               'dummy content with no marker',
+                               array(
+                                       '###REPLACED###' => '_replaced_'
+                               ),
+                               array(),
+                               array(),
+                               'dummy content with no marker',
+                               true,
+                               false
+                       ),
+                       'one marker' => array(
+                               'dummy content with ###REPLACED### marker',
+                               array(
+                                       '###REPLACED###' => '_replaced_'
+                               ),
+                               array(),
+                               array(),
+                               'dummy content with _replaced_ marker'
+                       ),
+                       'one marker with lots of chars' => array(
+                               'dummy content with ###RE.:##-=_()LACED### marker',
+                               array(
+                                       '###RE.:##-=_()LACED###' => '_replaced_'
+                               ),
+                               array(),
+                               array(),
+                               'dummy content with _replaced_ marker'
+                       ),
+                       'markers which are special' => array(
+                               'dummy ###aa##.#######A### ######',
+                               array(
+                                       '###aa##.###' => 'content ',
+                                       '###A###' => 'is',
+                                       '######' => '-is not considered-'
+                               ),
+                               array(),
+                               array(),
+                               'dummy content #is ######'
+                       ),
+                       'two markers in content, but more defined' => array(
+                               'dummy ###CONTENT### with ###REPLACED### marker',
+                               array(
+                                       '###REPLACED###' => '_replaced_',
+                                       '###CONTENT###' => 'content',
+                                       '###NEVERUSED###' => 'bar'
+                               ),
+                               array(),
+                               array(),
+                               'dummy content with _replaced_ marker'
+                       ),
+                       'one subpart' => array(
+                               'dummy content with ###ASUBPART### around some text###ASUBPART###.',
+                               array(),
+                               array(
+                                       '###ASUBPART###' => 'some other text'
+                               ),
+                               array(),
+                               'dummy content with some other text.'
+                       ),
+                       'one wrapped subpart' => array(
+                               'dummy content with ###AWRAPPEDSUBPART### around some text###AWRAPPEDSUBPART###.',
+                               array(),
+                               array(),
+                               array(
+                                       '###AWRAPPEDSUBPART###' => array(
+                                               'more content',
+                                               'content'
+                                       )
+                               ),
+                               'dummy content with more content around some textcontent.'
+                       ),
+                       'one subpart with markers, not replaced recursively' => array(
+                               'dummy ###CONTENT### with ###ASUBPART### around ###SOME### text###ASUBPART###.',
+                               array(
+                                       '###CONTENT###' => 'content',
+                                       '###SOME###' => '-this should never make it into output-',
+                                       '###OTHER_NOT_REPLACED###' => '-this should never make it into output-'
+                               ),
+                               array(
+                                       '###ASUBPART###' => 'some ###OTHER_NOT_REPLACED### text'
+                               ),
+                               array(),
+                               'dummy content with some ###OTHER_NOT_REPLACED### text.'
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        * @dataProvider substituteMarkerArrayCachedReturnsExpectedContentDataProvider
+        *
+        * @param string $content
+        * @param array $markContentArray
+        * @param array $subpartContentArray
+        * @param array $wrappedSubpartContentArray
+        * @param string $expectedContent
+        * @param bool $shouldQueryCache
+        * @param bool $shouldStoreCache
+        */
+       public function substituteMarkerArrayCachedReturnsExpectedContent($content, array $markContentArray, array $subpartContentArray, array $wrappedSubpartContentArray, $expectedContent, $shouldQueryCache = true, $shouldStoreCache = true) {
+               /** @var \TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Fixtures\PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
+               $pageRepo = $this->tsfe->sys_page;
+               $pageRepo->resetCallCount();
+
+               $resultContent = $this->cObj->substituteMarkerArrayCached($content, $markContentArray, $subpartContentArray, $wrappedSubpartContentArray);
+
+               $this->assertSame((int)$shouldQueryCache, $pageRepo::$getHashCallCount, 'getHash call count mismatch');
+               $this->assertSame((int)$shouldStoreCache, $pageRepo::$storeHashCallCount, 'storeHash call count mismatch');
+               $this->assertSame($expectedContent, $resultContent);
+       }
+
+       /**
+        * @test
+        */
+       public function substituteMarkerArrayCachedRetrievesCachedValueFromRuntimeCache() {
+               /** @var \TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Fixtures\PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
+               $pageRepo = $this->tsfe->sys_page;
+               $pageRepo->resetCallCount();
+
+               $content = 'Please tell me this ###FOO###.';
+               $markContentArray = array(
+                       '###FOO###' => 'foo',
+                       '###NOTUSED###' => 'blub'
+               );
+               $storeKey = md5('substituteMarkerArrayCached_storeKey:' . serialize(array($content, array_keys($markContentArray))));
+               $this->cObj->substMarkerCache[$storeKey] = array(
+                       'c' => array(
+                               'Please tell me this ',
+                               '.'
+                       ),
+                       'k' => array(
+                               '###FOO###'
+                       ),
+               );
+               $resultContent = $this->cObj->substituteMarkerArrayCached($content, $markContentArray);
+               $this->assertSame(0, $pageRepo::$getHashCallCount);
+               $this->assertSame('Please tell me this foo.', $resultContent);
+       }
+
+       /**
+        * @test
+        */
+       public function substituteMarkerArrayCachedRetrievesCachedValueFromDbCache() {
+               /** @var \TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Fixtures\PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
+               $pageRepo = $this->tsfe->sys_page;
+               $pageRepo->resetCallCount();
+
+               $content = 'Please tell me this ###FOO###.';
+               $markContentArray = array(
+                       '###FOO###' => 'foo',
+                       '###NOTUSED###' => 'blub'
+               );
+               $pageRepo::$dbCacheContent = array(
+                       'c' => array(
+                               'Please tell me this ',
+                               '.'
+                       ),
+                       'k' => array(
+                               '###FOO###'
+                       ),
+               );
+               $resultContent = $this->cObj->substituteMarkerArrayCached($content, $markContentArray);
+               $this->assertSame(1, $pageRepo::$getHashCallCount, 'getHash call count mismatch');
+               $this->assertSame(0, $pageRepo::$storeHashCallCount, 'storeHash call count mismatch');
+               $this->assertSame('Please tell me this foo.', $resultContent);
+       }
+
+       /**
+        * @test
+        */
+       public function substituteMarkerArrayCachedStoresResultInCaches() {
+               /** @var \TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Fixtures\PageRepositoryFixture|\PHPUnit_Framework_MockObject_MockObject $pageRepo */
+               $pageRepo = $this->tsfe->sys_page;
+               $pageRepo->resetCallCount();
+
+               $content = 'Please tell me this ###FOO###.';
+               $markContentArray = array(
+                       '###FOO###' => 'foo',
+                       '###NOTUSED###' => 'blub'
+               );
+               $resultContent = $this->cObj->substituteMarkerArrayCached($content, $markContentArray);
+
+               $storeKey = md5('substituteMarkerArrayCached_storeKey:' . serialize(array($content, array_keys($markContentArray))));
+               $storeArr = array(
+                       'c' => array(
+                               'Please tell me this ',
+                               '.'
+                       ),
+                       'k' => array(
+                               '###FOO###'
+                       ),
+               );
+               $this->assertSame(1, $pageRepo::$getHashCallCount);
+               $this->assertSame('Please tell me this foo.', $resultContent);
+               $this->assertSame($storeArr, $this->cObj->substMarkerCache[$storeKey]);
+               $this->assertSame(1, $pageRepo::$storeHashCallCount);
+       }
 
        /**
         * @return \TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController
diff --git a/typo3/sysext/frontend/Tests/Unit/ContentObject/Fixtures/PageRepositoryFixture.php b/typo3/sysext/frontend/Tests/Unit/ContentObject/Fixtures/PageRepositoryFixture.php
new file mode 100644 (file)
index 0000000..67aa8a6
--- /dev/null
@@ -0,0 +1,42 @@
+<?php
+namespace TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Fixtures;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Frontend\Page\PageRepository;
+
+/**
+ * Fixture for TYPO3\CMS\Core\Utility\GeneralUtility
+ */
+class PageRepositoryFixture extends PageRepository {
+
+       public static $getHashCallCount = 0;
+       public static $storeHashCallCount = 0;
+       public static $dbCacheContent = null;
+
+       public static function getHash($hash) {
+               static::$getHashCallCount++;
+               return static::$dbCacheContent;
+       }
+
+       public static function storeHash($hash, $data, $ident, $lifetime = 0) {
+               static::$storeHashCallCount++;
+       }
+
+       public static function resetCallCount() {
+               static::$getHashCallCount = 0;
+               static::$storeHashCallCount = 0;
+               static::$dbCacheContent = null;
+       }
+}
\ No newline at end of file