Bug #44270

wrong result in substituteMarkerArrayCached

Added by Franz Holzinger almost 7 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Should have
Category:
-
Target version:
-
Start date:
2013-01-02
Due date:
% Done:

100%

TYPO3 Version:
4.7
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

After some usage of the function it ends up in an error message.

Core: Error handler (FE): PHP Warning: Invalid argument supplied for foreach() in /var/www/mydomain/typo3_src-4.7.7/typo3/sysext/cms/tslib/class.tslib_content.php line 1727

Which is the line:
foreach ($storeArr['k'] as $n => $keyN) {

The debug information is:

class.tslib_content.php 1678:
$storeArrDat = $GLOBALS['TSFE']->sys_page->getHash($storeKey);

a:2:{s:1:"c";b:0;s:1:"k";N;}
$storeArrDat after sys_page->getHash (string)

So the $storeArr is an array of k and c with empty values. This causes the next foreach to crash.

In such a case the "if (!isset($storeArrDat)) {" branch should have been reached.

However this still won't work because after some modification it ends up in:

Core: Error handler (FE): PHP Warning: preg_split(): Compilation failed: regular expression is too large at offset 32848 in /var/www/mydomain/typo3_src-4.7.7/typo3/sysext/cms/tslib/class.tslib_content.php line 1708

So for this issue: It should not throw the foreach error but another details error message in order to know about the wrong preg_split function.

patch-44270.diff View (1.07 KB) Franz Holzinger, 2015-09-01 16:12


Related issues

Related to TYPO3 Core - Bug #72224: problems introduced by bugfix 44270 Closed 2015-12-14
Related to TYPO3 Core - Bug #72252: substituteMarkerArrayCached not substitute $subpartContentArray Closed 2015-12-15
Related to TYPO3 Core - Bug #73598: Three hash (###) marker in JavaScript creates the problem in TYPO3 6.2.18 Closed 2016-02-22

Associated revisions

Revision ac77e263 (diff)
Added by Markus Klein about 4 years ago

[BUGFIX] Avoid overly large regex in substituteMarkerArrayCached

Fetch the actually used markers from the content and only
generate the replace regex for those.
This avoids problems where 1000 markers may be passed in,
but only 10 are actually used.

Resolves: #44270
Releases: master, 6.2
Change-Id: I05f60960949e945249b045a8ae8e8430f7d8f7e6
Reviewed-on: https://review.typo3.org/45097
Reviewed-by: Morton Jonuschat <>
Tested-by: Morton Jonuschat <>
Reviewed-by: Benni Mack <>
Tested-by: Benni Mack <>

Revision 62a406e3 (diff)
Added by Benni Mack about 4 years ago

[BUGFIX] Avoid overly large regex in substituteMarkerArrayCached

Fetch the actually used markers from the content and only
generate the replace regex for those.
This avoids problems where 1000 markers may be passed in,
but only 10 are actually used.

Resolves: #44270
Releases: master, 6.2
Change-Id: I05f60960949e945249b045a8ae8e8430f7d8f7e6
Reviewed-on: https://review.typo3.org/45132
Reviewed-by: Morton Jonuschat <>
Tested-by: Morton Jonuschat <>
Reviewed-by: Benni Mack <>
Tested-by: Benni Mack <>

History

#1 Updated by Mathias Schreiber almost 5 years ago

  • Status changed from New to Needs Feedback
  • Assignee set to Mathias Schreiber
  • Is Regression set to No

Hi Franz,

I have next to noone still using the ### logic.
Are you up to supplying a patch so we get this one figured out?

#2 Updated by Franz Holzinger over 4 years ago

This bug is still present in TYPO3 6.2

Core: Error handler (FE): PHP Warning: preg_match_all(): Compilation failed: regular expression is too large at offset 32814 in /path/typo3_src-6.2.11/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php line 1881

Core: Error handler (FE): PHP Warning: Invalid argument supplied for foreach() in /path/typo3_src-6.2.11/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php line 1896

I will try to add a patch to this bug tracker.

#3 Updated by Alexander Opitz over 4 years ago

Hi Franz,

what's the state of this issue?

#4 Updated by Franz Holzinger over 4 years ago

I will write a patch in September 2015.

#5 Updated by Franz Holzinger over 4 years ago

This patch solves this issue for most cases. In a normal case this error message occurs because the TYPO3 extension calls substituteMarkerArrayCached with a big array $markContentArray. Many markers shall be replaced if found. However most of those markers will never be existent on the page.
The solution checks for the list of really existing markers in the content. Then only those are used for the following regular expression call. The other markers won't be needed on this page or for the output of this TYPO3 extension. By this way the problematic preg_split will now have a much smaller regular expression: about 100 markers instead of 1200 before.

If there is a case which has more than 1024 used markers on a page, then this patch would still fail. I could write another patch if this is really necessary. Then the content must be divided into several parts.

#6 Updated by Alexander Opitz over 4 years ago

  • Assignee changed from Mathias Schreiber to Franz Holzinger

Can you please push this patch to Gerrit?

#7 Updated by Gerrit Code Review about 4 years ago

  • Status changed from Needs Feedback to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/45097

#8 Updated by Gerrit Code Review about 4 years ago

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/45132

#9 Updated by Markus Klein about 4 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#10 Updated by Franz Holzinger about 4 years ago

I have found a case, where the function runs into a PHP warning.

Core: Error handler (FE): PHP Warning: Invalid argument supplied for foreach() in /var/www/web50/web/typo3_src-6.1.12/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php line 1823

$storeArrDat = $GLOBALS['TSFE']->sys_page->getHash($storeKey);
$storeArrDat:
a:0:{}
// Traversing the keyList array and merging the static and dynamic content
foreach ($storeArr['k'] as $n => $keyN) {

$storeArr is empty in this case.

No PHP warning should be given in this case.

#11 Updated by Franz Holzinger about 4 years ago

I have found another error related to the last patch.
If you have a $content parameter for substituteMarkerArrayCached which has no marker at all, then an empty result is given back. This is wrong. The full $content must be returned.
Do not store into the cache with substMarkerCache if the result of preg_match_all is 0 or FALSE or $usedMarkers is empty. This will solve this issue.

$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, serialize($storeArr), 'substMarkArrayCached');
$GLOBALS['TT']->setTSlogMessage('Parsing', 0);
}
}
} else {
// Unserializing
$storeArr = unserialize($storeArrDat);
// Setting cache:
$this->substMarkerCache[$storeKey] = $storeArr;

The $content must be returned without any change if no marker is inside of it.

if (
isset($storeArr['k'])
) {
$content = '';
// Traversing the keyList array and merging the static and dynamic content
foreach ($storeArr['k'] as $n => $keyN) {
$content .= $storeArr['c'][$n];
if (!is_array($valueArr[$keyN])) {
$content .= $valueArr[$keyN];
} else {
$content .= $valueArr[$keyN][intval($wSCA_reg[$keyN]) % 2];
$wSCA_reg[$keyN]++;
}
}
$content .= $storeArr['c'][count($storeArr['k'])];
$GLOBALS['TT']->pull();
}
return $content;
}

#12 Updated by Alexander Opitz almost 4 years ago

@Franz:

Please create a new issue and referee to this one. Thanks.

#13 Updated by Morten Haggren over 3 years ago

Markus Klein wrote:

Applied in changeset ac77e26331d94ed0521b30fdb5e70fe67fd1fffa.

This patch has the unfortunate side effect of requiring markers to be wrapped in ###, where as before you were free to use any or none.

Not only does this break old addons using that feature with no warning, but the requirement does not appear in the API

http://api.typo3.org/typo3cms/62/html/class_t_y_p_o3_1_1_c_m_s_1_1_frontend_1_1_content_object_1_1_content_object_renderer.html#a24476b64ecbbc44c0b974980ad20a70f

#14 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF