From 4884fb154f26a469cc524a7f85dbb6ee845840c7 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Tue, 22 Aug 2017 16:19:56 +0900 Subject: [PATCH] Issue #2849674 by Lendude, mxh, tacituseu, podarok, andypost, pingwin4eg, axel.rutz, catch: Complex job in ViewExecutable::unserialize() causes data corruption --- core/modules/views/src/ViewExecutable.php | 97 ++++++++++++------- .../tests/src/Kernel/ViewExecutableTest.php | 31 ++++++ 2 files changed, 93 insertions(+), 35 deletions(-) diff --git a/core/modules/views/src/ViewExecutable.php b/core/modules/views/src/ViewExecutable.php index b1861f7b36..f13c0e752d 100644 --- a/core/modules/views/src/ViewExecutable.php +++ b/core/modules/views/src/ViewExecutable.php @@ -4,7 +4,6 @@ use Drupal\Component\Utility\Html; use Drupal\Component\Utility\Tags; -use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\Routing\RouteProviderInterface; use Drupal\Core\Session\AccountInterface; use Drupal\views\Plugin\views\display\DisplayRouterInterface; @@ -17,9 +16,14 @@ * * 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. + * + * This class does not implement the Serializable interface since problems + * occurred when using the serialize method. + * + * @see https://www.drupal.org/node/2849674 + * @see https://bugs.php.net/bug.php?id=66052 */ -class ViewExecutable implements \Serializable { - use DependencySerializationTrait; +class ViewExecutable { /** * The config entity in which the view is stored. @@ -434,6 +438,13 @@ class ViewExecutable implements \Serializable { */ protected $baseEntityType; + /** + * Holds all necessary data for proper unserialization. + * + * @var array + */ + protected $serializationData; + /** * Constructs a new ViewExecutable object. * @@ -2466,52 +2477,68 @@ public function getDependencies() { } /** - * {@inheritdoc} - */ - public function serialize() { - return serialize([ - // Only serialize the storage entity ID. - $this->storage->id(), - $this->current_display, - $this->args, - $this->current_page, - $this->exposed_input, - $this->exposed_raw_input, - $this->exposed_data, - $this->dom_id, - $this->executed, - ]); + * Magic method implementation to serialize the view executable. + * + * @return array + * The names of all variables that should be serialized. + */ + public function __sleep() { + // Limit to only the required data which is needed to properly restore the + // state during unserialization. + $this->serializationData = [ + 'storage' => $this->storage->id(), + 'views_data' => $this->viewsData->_serviceId, + 'route_provider' => $this->routeProvider->_serviceId, + 'current_display' => $this->current_display, + 'args' => $this->args, + 'current_page' => $this->current_page, + 'exposed_input' => $this->exposed_input, + 'exposed_raw_input' => $this->exposed_raw_input, + 'exposed_data' => $this->exposed_data, + 'dom_id' => $this->dom_id, + 'executed' => $this->executed, + ]; + return ['serializationData']; } /** - * {@inheritdoc} + * Magic method implementation to unserialize the view executable. */ - public function unserialize($serialized) { - list($storage, $current_display, $args, $current_page, $exposed_input, $exposed_raw_input, $exposed_data, $dom_id, $executed) = unserialize($serialized); - - // There are cases, like in testing, where we don't have a container + public function __wakeup() { + // There are cases, like in testing where we don't have a container // available. - if (\Drupal::hasContainer()) { - $this->setRequest(\Drupal::request()); - $this->user = \Drupal::currentUser(); + if (\Drupal::hasContainer() && !empty($this->serializationData)) { + // Load and reference the storage. + $this->storage = \Drupal::entityTypeManager()->getStorage('view') + ->load($this->serializationData['storage']); + $this->storage->set('executable', $this); - $this->storage = \Drupal::entityManager()->getStorage('view')->load($storage); + // Attach all necessary services. + $this->user = \Drupal::currentUser(); + $this->viewsData = \Drupal::service($this->serializationData['views_data']); + $this->routeProvider = \Drupal::service($this->serializationData['route_provider']); - $this->setDisplay($current_display); - $this->setArguments($args); - $this->setCurrentPage($current_page); - $this->setExposedInput($exposed_input); - $this->exposed_data = $exposed_data; - $this->exposed_raw_input = $exposed_raw_input; - $this->dom_id = $dom_id; + // Restore the state of this executable. + if ($request = \Drupal::request()) { + $this->setRequest($request); + } + $this->setDisplay($this->serializationData['current_display']); + $this->setArguments($this->serializationData['args']); + $this->setCurrentPage($this->serializationData['current_page']); + $this->setExposedInput($this->serializationData['exposed_input']); + $this->exposed_data = $this->serializationData['exposed_data']; + $this->exposed_raw_input = $this->serializationData['exposed_raw_input']; + $this->dom_id = $this->serializationData['dom_id']; $this->initHandlers(); // If the display was previously executed, execute it now. - if ($executed) { + if ($this->serializationData['executed']) { $this->execute($this->current_display); } } + // Unset serializationData since it serves no further purpose. + unset($this->serializationData); } } diff --git a/core/modules/views/tests/src/Kernel/ViewExecutableTest.php b/core/modules/views/tests/src/Kernel/ViewExecutableTest.php index f843892e5d..a568363bb9 100644 --- a/core/modules/views/tests/src/Kernel/ViewExecutableTest.php +++ b/core/modules/views/tests/src/Kernel/ViewExecutableTest.php @@ -487,6 +487,37 @@ public function testSerialization() { $this->assertIdentical($unserialized->current_display, 'page_1', 'The expected display was set on the unserialized view.'); $this->assertIdentical($unserialized->args, ['test'], 'The expected argument was set on the unserialized view.'); $this->assertIdentical($unserialized->getCurrentPage(), 2, 'The expected current page was set on the unserialized view.'); + + // Get the definition of node's nid field, for example. Only get it not from + // the field manager directly, but from the item data definition. It should + // be the same base field definition object (the field and item definitions + // refer to each other). + // See https://bugs.php.net/bug.php?id=66052 + $field_manager = $this->container->get('entity_field.manager'); + $nid_definition_before = $field_manager->getBaseFieldDefinitions('node')['nid'] + ->getItemDefinition() + ->getFieldDefinition(); + + // Load and execute a view. + $view_entity = View::load('content'); + $view_executable = $view_entity->getExecutable(); + $view_executable->execute('page_1'); + + // Reset the static cache. Don't use clearCachedFieldDefinitions() since + // that clears the persistent cache and we need to get the serialized cache + // data. + $field_manager->useCaches(FALSE); + $field_manager->useCaches(TRUE); + + // Serialize the ViewExecutable as part of other data. + unserialize(serialize(['SOMETHING UNEXPECTED', $view_executable])); + + // Make sure the serialisation of the ViewExecutable didn't influence the + // field definitions. + $nid_definition_after = $field_manager->getBaseFieldDefinitions('node')['nid'] + ->getItemDefinition() + ->getFieldDefinition(); + $this->assertEquals($nid_definition_before->getPropertyDefinitions(), $nid_definition_after->getPropertyDefinitions()); } } -- 2.24.1