From 1cce8991af8d75ab9c3297e094433c444e9ad469 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Tue, 11 Feb 2014 09:42:30 +0000 Subject: [PATCH] Issue #2190643 by Berdir, amateescu, Xano, yched: Serializing the container is a very very bad idea, let's prevent it?. --- core/lib/Drupal/Core/Config/Config.php | 3 +- .../Drupal/Core/Controller/FormController.php | 3 +- .../Core/DependencyInjection/Container.php | 8 +++ .../DependencyInjection/ContainerBuilder.php | 21 ++++++++ .../DependencySerialization.php | 6 +++ .../Core/Entity/EntityControllerBase.php | 3 +- .../Drupal/Core/Language/LanguageManager.php | 3 +- core/lib/Drupal/Core/Plugin/PluginBase.php | 52 +++++++++++++++++++ .../views/lib/Drupal/views/ViewExecutable.php | 3 +- .../ContainerBuilderTest.php | 16 +++++- .../DependencySerializationTest.php | 33 ++++++++---- 11 files changed, 136 insertions(+), 15 deletions(-) diff --git a/core/lib/Drupal/Core/Config/Config.php b/core/lib/Drupal/Core/Config/Config.php index 68502c67a94e..89220498a536 100644 --- a/core/lib/Drupal/Core/Config/Config.php +++ b/core/lib/Drupal/Core/Config/Config.php @@ -11,6 +11,7 @@ use Drupal\Component\Utility\String; use Drupal\Core\Config\ConfigNameException; use Drupal\Core\Config\Schema\SchemaIncompleteException; +use Drupal\Core\DependencyInjection\DependencySerialization; use Drupal\Core\TypedData\PrimitiveInterface; use Drupal\Core\TypedData\Type\FloatInterface; use Drupal\Core\TypedData\Type\IntegerInterface; @@ -20,7 +21,7 @@ /** * Defines the default configuration object. */ -class Config { +class Config extends DependencySerialization { /** * The maximum length of a configuration object name. diff --git a/core/lib/Drupal/Core/Controller/FormController.php b/core/lib/Drupal/Core/Controller/FormController.php index 271de99f39eb..5296a4b23877 100644 --- a/core/lib/Drupal/Core/Controller/FormController.php +++ b/core/lib/Drupal/Core/Controller/FormController.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Controller; +use Drupal\Core\DependencyInjection\DependencySerialization; use Drupal\Core\Form\FormBuilderInterface; use Symfony\Component\HttpFoundation\Request; @@ -15,7 +16,7 @@ * * @todo Make this a trait in PHP 5.4. */ -abstract class FormController { +abstract class FormController extends DependencySerialization { /** * The form definition. The format may vary depending on the child class. diff --git a/core/lib/Drupal/Core/DependencyInjection/Container.php b/core/lib/Drupal/Core/DependencyInjection/Container.php index 1869bfcc4593..0d4c972e9658 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Container.php +++ b/core/lib/Drupal/Core/DependencyInjection/Container.php @@ -27,4 +27,12 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE return $service; } + /** + * {@inheritdoc} + */ + public function __sleep() { + trigger_error('The container was serialized.', E_USER_ERROR); + return array_keys(get_object_vars($this)); + } + } diff --git a/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php index 736680d5a693..56f1627ed59c 100644 --- a/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php +++ b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php @@ -28,6 +28,19 @@ class ContainerBuilder extends SymfonyContainerBuilder { public function addObjectResource($object) { } + /** + * {@inheritdoc} + */ + public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE) { + $service = parent::get($id, $invalidBehavior); + // Some services are called but do not exist, so the parent returns nothing. + if (is_object($service)) { + $service->_serviceId = $id; + } + + return $service; + } + /** * Overrides Symfony\Component\DependencyInjection\ContainerBuilder::set(). * @@ -89,4 +102,12 @@ protected function callMethod($service, $call) { call_user_func_array(array($service, $call[0]), $this->resolveServices($this->getParameterBag()->resolveValue($call[1]))); } + /** + * {@inheritdoc} + */ + public function __sleep() { + trigger_error('The container was serialized.', E_USER_ERROR); + return array_keys(get_object_vars($this)); + } + } diff --git a/core/lib/Drupal/Core/DependencyInjection/DependencySerialization.php b/core/lib/Drupal/Core/DependencyInjection/DependencySerialization.php index feb50f15c92c..256ce745d4a1 100644 --- a/core/lib/Drupal/Core/DependencyInjection/DependencySerialization.php +++ b/core/lib/Drupal/Core/DependencyInjection/DependencySerialization.php @@ -6,6 +6,7 @@ */ namespace Drupal\Core\DependencyInjection; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Provides a dependency injection friendly methods for serialization. @@ -33,6 +34,11 @@ public function __sleep() { $this->_serviceIds[$key] = $value->_serviceId; unset($vars[$key]); } + // Special case the container, which might not have a service ID. + elseif ($value instanceof ContainerInterface) { + $this->_serviceIds[$key] = 'service_container'; + unset($vars[$key]); + } } return array_keys($vars); diff --git a/core/lib/Drupal/Core/Entity/EntityControllerBase.php b/core/lib/Drupal/Core/Entity/EntityControllerBase.php index f468303ebf61..7b60766308cd 100644 --- a/core/lib/Drupal/Core/Entity/EntityControllerBase.php +++ b/core/lib/Drupal/Core/Entity/EntityControllerBase.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Entity; +use Drupal\Core\DependencyInjection\DependencySerialization; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\StringTranslation\TranslationInterface; @@ -15,7 +16,7 @@ * * @todo Convert this to a trait. */ -abstract class EntityControllerBase { +abstract class EntityControllerBase extends DependencySerialization { /** * The translation manager service. diff --git a/core/lib/Drupal/Core/Language/LanguageManager.php b/core/lib/Drupal/Core/Language/LanguageManager.php index 2bd3f202a757..6ddf67310d09 100644 --- a/core/lib/Drupal/Core/Language/LanguageManager.php +++ b/core/lib/Drupal/Core/Language/LanguageManager.php @@ -8,12 +8,13 @@ namespace Drupal\Core\Language; use Drupal\Component\Utility\String; +use Drupal\Core\DependencyInjection\DependencySerialization; use Drupal\Core\StringTranslation\TranslationInterface; /** * Class responsible for providing language support on language-unaware sites. */ -class LanguageManager implements LanguageManagerInterface { +class LanguageManager extends DependencySerialization implements LanguageManagerInterface { /** * The string translation service. diff --git a/core/lib/Drupal/Core/Plugin/PluginBase.php b/core/lib/Drupal/Core/Plugin/PluginBase.php index 03f952a6426d..f0bce24488a3 100644 --- a/core/lib/Drupal/Core/Plugin/PluginBase.php +++ b/core/lib/Drupal/Core/Plugin/PluginBase.php @@ -9,12 +9,23 @@ use Drupal\Component\Plugin\PluginBase as ComponentPluginBase; use Drupal\Core\StringTranslation\TranslationInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Base class for plugins supporting metadata inspection and translation. */ abstract class PluginBase extends ComponentPluginBase { + /** + * An array of service IDs keyed by property name used for serialization. + * + * @todo Remove when Drupal\Core\DependencyInjection\DependencySerialization + * is converted to a trait. + * + * @var array + */ + protected $_serviceIds = array(); + /** * The translation manager service. * @@ -58,4 +69,45 @@ public function setTranslationManager(TranslationInterface $translation_manager) return $this; } + /** + * {@inheritdoc} + * + * @todo Remove when Drupal\Core\DependencyInjection\DependencySerialization + * is converted to a trait. + */ + public function __sleep() { + $this->_serviceIds = array(); + $vars = get_object_vars($this); + foreach ($vars as $key => $value) { + if (is_object($value) && isset($value->_serviceId)) { + // If a class member was instantiated by the dependency injection + // container, only store its ID so it can be used to get a fresh object + // on unserialization. + $this->_serviceIds[$key] = $value->_serviceId; + unset($vars[$key]); + } + // Special case the container, which might not have a service ID. + elseif ($value instanceof ContainerInterface) { + $this->_serviceIds[$key] = 'service_container'; + unset($vars[$key]); + } + } + + return array_keys($vars); + } + + /** + * {@inheritdoc} + * + * @todo Remove when Drupal\Core\DependencyInjection\DependencySerialization + * is converted to a trait. + */ + public function __wakeup() { + $container = \Drupal::getContainer(); + foreach ($this->_serviceIds as $key => $service_id) { + $this->$key = $container->get($service_id); + } + unset($this->_serviceIds); + } + } diff --git a/core/modules/views/lib/Drupal/views/ViewExecutable.php b/core/modules/views/lib/Drupal/views/ViewExecutable.php index 9d9c857ed325..2fd33bfe8aab 100644 --- a/core/modules/views/lib/Drupal/views/ViewExecutable.php +++ b/core/modules/views/lib/Drupal/views/ViewExecutable.php @@ -7,6 +7,7 @@ namespace Drupal\views; +use Drupal\Core\DependencyInjection\DependencySerialization; use Drupal\Core\Session\AccountInterface; use Drupal\views\Plugin\views\query\QueryPluginBase; use Drupal\views\ViewStorageInterface; @@ -25,7 +26,7 @@ * An object to contain all of the data to generate a view, plus the member * functions to build the view query, execute the query and render the output. */ -class ViewExecutable { +class ViewExecutable extends DependencySerialization { /** * The config entity in which the view is stored. diff --git a/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php b/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php index 3ada9324bb05..fcc898f34759 100644 --- a/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php @@ -14,7 +14,7 @@ require_once __DIR__ . '../../../../../../vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php'; /** - * Dependency injection container builder. + * Tests the dependency injection container builder overrides of Drupal. * * @see \Drupal\Core\DependencyInjection\ContainerBuilder */ @@ -48,4 +48,18 @@ public function testSetOnSynchronizedService() { $this->assertSame($baz, $container->get('bar')->getBaz()); } + /** + * Tests the get method. + * + * @see \Drupal\Core\DependencyInjection\Container::get() + */ + public function testGet() { + $container = new ContainerBuilder(); + $container->register('bar', 'BarClass'); + + $result = $container->get('bar'); + $this->assertTrue($result instanceof \BarClass); + $this->assertEquals('bar', $result->_serviceId); + } + } diff --git a/core/tests/Drupal/Tests/Core/DependencyInjection/DependencySerializationTest.php b/core/tests/Drupal/Tests/Core/DependencyInjection/DependencySerializationTest.php index d6af542d4299..1cf99cc081ce 100644 --- a/core/tests/Drupal/Tests/Core/DependencyInjection/DependencySerializationTest.php +++ b/core/tests/Drupal/Tests/Core/DependencyInjection/DependencySerializationTest.php @@ -7,9 +7,11 @@ namespace Drupal\Tests\Core\DependencyInjection; +use Drupal\Core\DependencyInjection\Container; use Drupal\Core\DependencyInjection\DependencySerialization; use Drupal\Tests\UnitTestCase; -use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerAwareInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Tests the dependency serialization base class. @@ -36,20 +38,19 @@ public function testSerialization() { // Create a pseudo service and dependency injected object. $service = new \stdClass(); $service->_serviceId = 'test_service'; - $container = $this->getMock('Drupal\Core\DependencyInjection\Container'); - $container->expects($this->exactly(1)) - ->method('get') - ->with('test_service') - ->will($this->returnValue($service)); + $container = new Container(); + $container->set('test_service', $service); + $container->set('service_container', $container); \Drupal::setContainer($container); $dependencySerialization = new TestClass($service); + $dependencySerialization->setContainer($container); $string = serialize($dependencySerialization); $object = unserialize($string); - // The original object got _serviceIds added so let's remove it to check - // equality + // The original object got _serviceIds added so removing it to check + // equality. unset($dependencySerialization->_serviceIds); // Ensure dependency injected object remains the same after serialization. @@ -61,6 +62,7 @@ public function testSerialization() { // Ensure that both the service and the variable are in the unserialized // object. $this->assertSame($service, $object->service); + $this->assertSame($container, $object->container); } } @@ -68,7 +70,7 @@ public function testSerialization() { /** * Defines a test class which has a single service as dependency. */ -class TestClass extends DependencySerialization { +class TestClass extends DependencySerialization implements ContainerAwareInterface { /** * A test service. @@ -77,6 +79,13 @@ class TestClass extends DependencySerialization { */ public $service; + /** + * The container. + * + * @var \Symfony\Component\DependencyInjection\ContainerInterface + */ + public $container; + /** * {@inheritdoc} * @@ -94,4 +103,10 @@ public function __construct(\stdClass $service) { $this->service = $service; } + /** + * {@inheritdoc} + */ + public function setContainer(ContainerInterface $container = NULL) { + $this->container = $container; + } } -- GitLab