Bug #65708

ReflectionService updates unchanged data

Added by Leendert van Beelen over 4 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Extbase
Target version:
-
Start date:
2015-03-12
Due date:
% Done:

0%

TYPO3 Version:
8
PHP Version:
7.0
Tags:
Complexity:
hard
Is Regression:
No
Sprint Focus:

Description

The extbase ReflectionService is not protected against use before initialization. When it is used before being initialized the dataCacheNeedsUpdate flag is set, which results in a database update when shutdown method is called.

To reproduce the problem,
1) create a page with the following typoscript setup:

lib.content = CONTENT
lib.content {
    table = tt_content
     select.where = colPos=0
}

page = PAGE
page {
    10 = FLUIDTEMPLATE
    10 {
        file = site/Templates/index.html
    }
}

2) The file site/Templates/index.html should contain this html: <f:cObject typoscriptObjectPath="lib.content"/>
3) Place an extbase plugin on the page in the column with colPos 0 (usually the main content)

What happens when rendering this page is that the fluid viewhelper uses the ReflectionService without initialization and dataCacheNeedsUpdate is set to TRUE. Next the extbase plugin is instantiated and initialize method is called, that loads the data from the cache. When the extbase plugin is done it calls the shutdown method of the ReflectionService, which is going to call $this->saveToCache(); because the viewhelper set the dataCacheNeedsUpdate flag before.

I think the ReflectionService shouldn't be a singleton, because it's initialized for a specific extension it should have an object for each extension.

PoC_uncached_AbsctractViewHelper_reflection_access.patch View (47 KB) Oliver Eglseder, 2017-02-09 17:59

History

#1 Updated by Oliver Eglseder over 2 years ago

  • TYPO3 Version changed from 6.2 to 8
  • PHP Version changed from 5.3 to 7.0
  • Complexity set to medium

[UPDATE] This issue is still reproducible on the current master (8.6.0-dev)

Description of symptoms:
One of our customers runs a MySQL replication based on the binary logs of MySQL. One day the growth of the binary logs became so immense that the file system partition eventually exceed its capacity and the production website went offline with a lot of different errors. (We're talking about 10 GB growth per day with 5 days of hold-back time on a 50 GB partition for all MySQL related stuff. The whole TYPO3 DB itself is just about 1 GB in size)

Technical Problem:
The class \TYPO3\CMS\Extbase\Reflection\ReflectionService rewrites all ReflectionData_* entries from the table cf_extbase_reflection which were read during rendering of the page, regardless if they were modified or not.
As Leendert van Beelen pointed out this happens when the property $dataCacheNeedsUpdate is set to true. This occurs every time \TYPO3\CMS\Fluid\Core\ViewHelper\AbstractViewHelper::registerRenderMethodArguments is called (which seems to happen each time a ViewHelper is beeing initialized) since the method will call \TYPO3\CMS\Extbase\Reflection\ReflectionService::getMethodParameters and \TYPO3\CMS\Extbase\Reflection\ReflectionService::getMethodTagsValues on the uninitialized ReflectionService.
The results of the reflection are, however, not stored in any cache and will therefore be called on each request.

How to (theoretically) solve this:
I've thought about three different ways which might solve this problem in an accurate manner:

1. Do not make ReflectionService a Singleton (as pointed out by Leendert van Beelen)
2. Do not rely on ReflectionService in AbstractViewHelper::registerRenderMethodArguments, since the results aren't stored anyway.
3. Proper initialization of the ReflectionService in AbstractViewHelper with setting of dataCache (so the reflection results will be stored)

I think of the first option as the best one for a couple of reasons:
a) setDataCache(...) must be called to make this thing work properly
b) initialize() "should" be called for each instance, too
c) the cacheIdentifier is stored in the object, but is not valid for the whole request but only the "currently rendered extbase plugin".
d) the cached data shrinks in size because not every reflection information which might have been set before the object was initialized will be saved
e) shutdown()/saveToCache() will be bound to the cacheIdentifier. ReflectionData will only be updated if [data of this specific identifier] was touched. Any other untouched cache entrie will not be unnecessarily re-persisted.

A fourth option might be to ensure the ReflectionService can't be used improperly, but this would require an API change.

How to reproduce the Problem:
1. Clone TYPO3 dev-master and install it.
2. Require any extbase extension (like News, but it doesn't really matter. It just needs to have a Frontend Plugin)
3. Create one Page with a TypoScript Template. Include fluid_styled_content and change page.10 setup to: page.10 < styles.content.get
4. Insert an extbase plugin on the page. Show it in the frontend.
5. Reload the page multiple times and watch the UID of the ReflectionData_News (in this case) increment. Each increment of the UID means this cache entry was rewritten.

Some more information:
Sometimes the UID increases by 2 or more. This might happen if there are multiple extbase plugins of the same extension used on one page (Happened on our customers page)

#2 Updated by Oliver Eglseder over 2 years ago

I have to recall my first solution proposal, just deleting the SingletonInterface will not rewrite the whole cache but also prevent caching anything else.

Not speaking about the implementation itself but about the intention of the ReflectionService: It is not built to be used outside of an Extbase request, because it heavily relies on the value of extensionName. This fact kinda makes the class \TYPO3\CMS\Fluid\Core\ViewHelper\AbstractViewHelper useless for Fluid rendering through the \TYPO3\CMS\Frontend\ContentObject\FluidTemplateContentObject.

But i could figure out another solution:
Extbase can still use the ReflectionService which implements the SingletonInterface, however AbstractViewHelper::registerRenderMethodArguments should rely on a ReflectionService implementation without the SingletonInterface. This could be achieved by copying the ReflectionService implementation to another class which the Service inherits. AbstractViewHelper will then rely on the base class which does not implement the SingletonInterface.
This leads to uncached calls to ReflectionService::getMethodParameters for any ViewHelper but as you might have guessed they aren't cached currently anyway. They are just "controlled" uncached and will not smash any other cache.

I actually tested this solution and can confirm that it works properly. A Patch providing a rudimentary solution is attached.

P.S.: Set the complexity to hard since there's no "nice" way to get around the dependency to an Extbase request in the ReflectionService for the FluidTemplateContentObject. The first 5 possible solutions that came to my mind did not work.

#3 Updated by Oliver Eglseder about 2 years ago

I can not reproduce this Problem in TYPO3 v9 (except someone still relies on the legacy ViewHelper argument registration in a ViewHelper used in the Template), it is though still reproducible in 8.7.4

#4 Updated by Susanne Moog almost 2 years ago

  • Status changed from New to Closed

This bug is fixed in master - due to the massive rewrite of the ReflectionService that fix will not be backported to earlier TYPO3 versions.

Also available in: Atom PDF