From 582d2e9e270fa99e713142f9c0f9e0cf35ea3ab8 Mon Sep 17 00:00:00 2001
From: Mathias Schreiber <mathias.schreiber@wmdb.de>
Date: Tue, 8 Mar 2016 16:07:41 +0100
Subject: [PATCH] [TASK] Remove igbinary from cache framework
The patch removes igbinary serializer support from the cache framework.
When igbinary support was added back then in PHP 5.3 times, it had the
potential to serialize data quicker than the default serializer.
Unfortunately, other promises aren't held:
* The module found no general acceptance within the PHP community and
isn't supported very well. PHP 7 is still not officially supported.
* Last release at the time of this writing was in 2014-08
* The module must still be compiled from source and no recent
distribution ever packaged it by default
* The maintenance load on devOps side is high: The module must be
recompiled with each minor php release
* In case the module is not updated, it throws errors at a central place
of the system and can easily brick a whole instance
* The module found no huge acceptance by hosters and is only very rarely
used in real life instances
* A performance impact is only measurable in very small and highly
specialized use cases, it typically plays no role in casual frontend
or backend requests.
We've seen several life systems in the wild lately with sloppy hosters
not maintaining the igbinary module within their infrastructure properly.
The current implementation with VariableFrontend dynamically detecting
and then force using the module leads to hard crashes in those situations.
The main issue is that serializing is done via PHPs serializer and
unserializing using igbinary then fails, effectively rendering the
entire installation bricked.
To come by those situations, it is considered more important to deliver
a stable product than a product that is quicker in more theoretical use
cases.
Thus, the support for this module is kicked from the standard cache
frontend. In case igbinary still gives significant boost for specialized
specific instances, an admin can still configure a VariableFrontend that
uses this serializer.
Resolves: #74501
Releases: master
Change-Id: I8d4407382c941f66e8c2747a82ec4464b95476f8
Reviewed-on: https://review.typo3.org/47157
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Tested-by: Morton Jonuschat <m.jonuschat@mojocode.de>
---
.../Classes/Cache/Backend/FileBackend.php | 13 +---
.../Cache/Backend/SimpleFileBackend.php | 18 ------
.../Cache/Frontend/VariableFrontend.php | 42 ++++---------
.../Cache/Frontend/VariableFrontendTest.php | 59 -------------------
4 files changed, 15 insertions(+), 117 deletions(-)
diff --git a/typo3/sysext/core/Classes/Cache/Backend/FileBackend.php b/typo3/sysext/core/Classes/Cache/Backend/FileBackend.php
index cb7a191debed..9e85fa7a27a2 100644
--- a/typo3/sysext/core/Classes/Cache/Backend/FileBackend.php
+++ b/typo3/sysext/core/Classes/Cache/Backend/FileBackend.php
@@ -78,11 +78,7 @@ class FileBackend extends \TYPO3\CMS\Core\Cache\Backend\SimpleFileBackend implem
$this->cacheEntryIdentifiers[$entryIdentifier] = true;
file_put_contents($this->cacheDirectory . $entryIdentifier . $this->cacheEntryFileExtension, $this->get($entryIdentifier));
}
- if ($this->useIgBinary === true) {
- file_put_contents($this->cacheDirectory . 'FrozenCache.data', igbinary_serialize($this->cacheEntryIdentifiers));
- } else {
- file_put_contents($this->cacheDirectory . 'FrozenCache.data', serialize($this->cacheEntryIdentifiers));
- }
+ file_put_contents($this->cacheDirectory . 'FrozenCache.data', serialize($this->cacheEntryIdentifiers));
$this->frozen = true;
}
@@ -115,11 +111,7 @@ class FileBackend extends \TYPO3\CMS\Core\Cache\Backend\SimpleFileBackend implem
parent::setCache($cache);
if (file_exists($this->cacheDirectory . 'FrozenCache.data')) {
$this->frozen = true;
- if ($this->useIgBinary === true) {
- $this->cacheEntryIdentifiers = igbinary_unserialize(file_get_contents($this->cacheDirectory . 'FrozenCache.data'));
- } else {
- $this->cacheEntryIdentifiers = unserialize(file_get_contents($this->cacheDirectory . 'FrozenCache.data'));
- }
+ $this->cacheEntryIdentifiers = unserialize(file_get_contents($this->cacheDirectory . 'FrozenCache.data'));
}
}
@@ -394,7 +386,6 @@ class FileBackend extends \TYPO3\CMS\Core\Cache\Backend\SimpleFileBackend implem
return false;
}
} else {
- $pathAndFilename = $this->cacheDirectory . $entryIdentifier . $this->cacheEntryFileExtension;
if ($entryIdentifier !== basename($entryIdentifier)) {
throw new \InvalidArgumentException('The specified entry identifier must not contain a path segment.', 1282073036);
}
diff --git a/typo3/sysext/core/Classes/Cache/Backend/SimpleFileBackend.php b/typo3/sysext/core/Classes/Cache/Backend/SimpleFileBackend.php
index 52d6ed0588fe..52ec30a47eb4 100644
--- a/typo3/sysext/core/Classes/Cache/Backend/SimpleFileBackend.php
+++ b/typo3/sysext/core/Classes/Cache/Backend/SimpleFileBackend.php
@@ -62,24 +62,6 @@ class SimpleFileBackend extends \TYPO3\CMS\Core\Cache\Backend\AbstractBackend im
*/
protected $frozen = false;
- /**
- * If the extension "igbinary" is installed, use it for increased performance.
- * Caching the result of extension_loaded() here is faster than calling extension_loaded() multiple times.
- *
- * @var bool
- */
- protected $useIgBinary = false;
-
- /**
- * Initializes this cache frontend
- *
- * @return void
- */
- public function initializeObject()
- {
- $this->useIgBinary = extension_loaded('igbinary');
- }
-
/**
* Sets a reference to the cache frontend which uses this backend and
* initializes the default cache directory.
diff --git a/typo3/sysext/core/Classes/Cache/Frontend/VariableFrontend.php b/typo3/sysext/core/Classes/Cache/Frontend/VariableFrontend.php
index c154630d164b..038560832ade 100644
--- a/typo3/sysext/core/Classes/Cache/Frontend/VariableFrontend.php
+++ b/typo3/sysext/core/Classes/Cache/Frontend/VariableFrontend.php
@@ -18,29 +18,9 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
* A cache frontend for any kinds of PHP variables
- *
- * This file is a backport from FLOW3
- * @api
*/
class VariableFrontend extends AbstractFrontend
{
- /**
- * If the extension "igbinary" is installed, use it for increased performance.
- * Caching the result of extension_loaded() here is faster than calling extension_loaded() multiple times.
- *
- * @var bool
- */
- protected $useIgBinary = false;
-
- /**
- * Initializes this cache frontend
- *
- * @return void
- */
- public function initializeObject()
- {
- $this->useIgBinary = extension_loaded('igbinary');
- }
/**
* Saves the value of a PHP variable in the cache. Note that the variable
@@ -57,7 +37,10 @@ class VariableFrontend extends AbstractFrontend
public function set($entryIdentifier, $variable, array $tags = array(), $lifetime = null)
{
if (!$this->isValidEntryIdentifier($entryIdentifier)) {
- throw new \InvalidArgumentException('"' . $entryIdentifier . '" is not a valid cache entry identifier.', 1233058264);
+ throw new \InvalidArgumentException(
+ '"' . $entryIdentifier . '" is not a valid cache entry identifier.',
+ 1233058264
+ );
}
foreach ($tags as $tag) {
if (!$this->isValidTag($tag)) {
@@ -75,17 +58,14 @@ class VariableFrontend extends AbstractFrontend
GeneralUtility::callUserFunction($_funcRef, $params, $this);
}
}
- if ($this->useIgBinary === true) {
- $this->backend->set($entryIdentifier, igbinary_serialize($variable), $tags, $lifetime);
- } else {
- $this->backend->set($entryIdentifier, serialize($variable), $tags, $lifetime);
- }
+ $this->backend->set($entryIdentifier, serialize($variable), $tags, $lifetime);
}
/**
* Finds and returns a variable value from the cache.
*
* @param string $entryIdentifier Identifier of the cache entry to fetch
+ *
* @return mixed The value
* @throws \InvalidArgumentException if the identifier is not valid
* @api
@@ -93,13 +73,16 @@ class VariableFrontend extends AbstractFrontend
public function get($entryIdentifier)
{
if (!$this->isValidEntryIdentifier($entryIdentifier)) {
- throw new \InvalidArgumentException('"' . $entryIdentifier . '" is not a valid cache entry identifier.', 1233058294);
+ throw new \InvalidArgumentException(
+ '"' . $entryIdentifier . '" is not a valid cache entry identifier.',
+ 1233058294
+ );
}
$rawResult = $this->backend->get($entryIdentifier);
if ($rawResult === false) {
return false;
} else {
- return $this->useIgBinary === true ? igbinary_unserialize($rawResult) : unserialize($rawResult);
+ return unserialize($rawResult);
}
}
@@ -107,6 +90,7 @@ class VariableFrontend extends AbstractFrontend
* Finds and returns all cache entries which are tagged by the specified tag.
*
* @param string $tag The tag to search for
+ *
* @return array An array with the content of all matching entries. An empty array if no entries matched
* @throws \InvalidArgumentException if the tag is not valid
* @api
@@ -121,7 +105,7 @@ class VariableFrontend extends AbstractFrontend
foreach ($identifiers as $identifier) {
$rawResult = $this->backend->get($identifier);
if ($rawResult !== false) {
- $entries[] = $this->useIgBinary === true ? igbinary_unserialize($rawResult) : unserialize($rawResult);
+ $entries[] = unserialize($rawResult);
}
}
return $entries;
diff --git a/typo3/sysext/core/Tests/Unit/Cache/Frontend/VariableFrontendTest.php b/typo3/sysext/core/Tests/Unit/Cache/Frontend/VariableFrontendTest.php
index c7087b92ab8e..2471824653be 100644
--- a/typo3/sysext/core/Tests/Unit/Cache/Frontend/VariableFrontendTest.php
+++ b/typo3/sysext/core/Tests/Unit/Cache/Frontend/VariableFrontendTest.php
@@ -72,24 +72,6 @@ class VariableFrontendTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
$cache->set('VariableCacheTest', $theString, array(), $theLifetime);
}
- /**
- * @test
- */
- public function setUsesIgBinarySerializeIfAvailable()
- {
- if (!extension_loaded('igbinary')) {
- $this->markTestSkipped('Cannot test igbinary support, because igbinary is not installed.');
- }
-
- $theString = 'Just some value';
- $backend = $this->getMock(\TYPO3\CMS\Core\Cache\Backend\AbstractBackend::class, array('get', 'set', 'has', 'remove', 'findIdentifiersByTag', 'flush', 'flushByTag', 'collectGarbage'), array(), '', false);
- $backend->expects($this->once())->method('set')->with($this->equalTo('VariableCacheTest'), $this->equalTo(igbinary_serialize($theString)));
-
- $cache = new \TYPO3\CMS\Core\Cache\Frontend\VariableFrontend('VariableFrontend', $backend);
- $cache->initializeObject();
- $cache->set('VariableCacheTest', $theString);
- }
-
/**
* @test
*/
@@ -127,25 +109,6 @@ class VariableFrontendTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
$this->assertFalse($cache->get('VariableCacheTest'), 'The returned value was not the FALSE.');
}
- /**
- * @test
- */
- public function getUsesIgBinaryIfAvailable()
- {
- if (!extension_loaded('igbinary')) {
- $this->markTestSkipped('Cannot test igbinary support, because igbinary is not installed.');
- }
-
- $theArray = array('Just some value', 'and another one.');
- $backend = $this->getMock(\TYPO3\CMS\Core\Cache\Backend\AbstractBackend::class, array('get', 'set', 'has', 'remove', 'findIdentifiersByTag', 'flush', 'flushByTag', 'collectGarbage'), array(), '', false);
- $backend->expects($this->once())->method('get')->will($this->returnValue(igbinary_serialize($theArray)));
-
- $cache = new \TYPO3\CMS\Core\Cache\Frontend\VariableFrontend('VariableFrontend', $backend);
- $cache->initializeObject();
-
- $this->assertEquals($theArray, $cache->get('VariableCacheTest'), 'The returned value was not the expected unserialized array.');
- }
-
/**
* @test
*/
@@ -201,26 +164,4 @@ class VariableFrontendTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
$cache = new \TYPO3\CMS\Core\Cache\Frontend\VariableFrontend('VariableFrontend', $backend);
$this->assertEquals($entries, $cache->getByTag($tag), 'Did not receive the expected entries');
}
-
- /**
- * @test
- */
- public function getByTagUsesIgBinaryIfAvailable()
- {
- if (!extension_loaded('igbinary')) {
- $this->markTestSkipped('Cannot test igbinary support, because igbinary is not installed.');
- }
-
- $tag = 'sometag';
- $identifiers = array('one', 'two');
- $entries = array('one value', 'two value');
- $backend = $this->getMock(\TYPO3\CMS\Core\Cache\Backend\AbstractBackend::class, array('get', 'set', 'has', 'remove', 'findIdentifiersByTag', 'flush', 'flushByTag', 'collectGarbage'), array(), '', false);
-
- $backend->expects($this->once())->method('findIdentifiersByTag')->with($this->equalTo($tag))->will($this->returnValue($identifiers));
- $backend->expects($this->exactly(2))->method('get')->will($this->onConsecutiveCalls(igbinary_serialize('one value'), igbinary_serialize('two value')));
-
- $cache = new \TYPO3\CMS\Core\Cache\Frontend\VariableFrontend('VariableFrontend', $backend);
- $cache->initializeObject();
- $this->assertEquals($entries, $cache->getByTag($tag), 'Did not receive the expected entries');
- }
}
--
GitLab