Commit 3a6f5da5 authored by alexpott's avatar alexpott

Issue #2463321 by amateescu: Serializing the database connection is dangerous...

Issue #2463321 by amateescu: Serializing the database connection is dangerous and error-prone, make it unserializable again
parent 996eb163
......@@ -914,7 +914,7 @@ function _batch_queue($batch_set) {
$class = $batch_set['queue']['class'];
if (!isset($queues[$class][$name])) {
$queues[$class][$name] = new $class($name, Database::getConnection());
$queues[$class][$name] = new $class($name, \Drupal::database());
}
return $queues[$class][$name];
}
......
......@@ -7,9 +7,6 @@
namespace Drupal\Core\Database;
use Drupal\Core\Database\TransactionNoActiveException;
use Drupal\Core\Database\TransactionOutOfOrderException;
/**
* Base Database API class.
*
......@@ -20,7 +17,7 @@
*
* @see http://php.net/manual/book.pdo.php
*/
abstract class Connection implements \Serializable {
abstract class Connection {
/**
* The database target this connection is for.
......@@ -1297,35 +1294,10 @@ public function quote($string, $parameter_type = \PDO::PARAM_STR) {
}
/**
* {@inheritdoc}
*/
public function serialize() {
$connection = clone $this;
// Don't serialize the PDO connection as well as everything else which
// depends on settings.php.
unset($connection->connection, $connection->connectionOptions, $connection->schema, $connection->prefixes, $connection->prefixReplace, $connection->driverClasses);
return serialize(get_object_vars($connection));
}
/**
* {@inheritdoc}
* Prevents the database connection from being serialized.
*/
public function unserialize($serialized) {
$data = unserialize($serialized);
foreach ($data as $key => $value) {
$this->{$key} = $value;
}
$this->connectionOptions = Database::getConnectionInfo($this->key)[$this->target];
// Re-establish the PDO connection using the original options.
$this->connection = static::open($this->connectionOptions);
// Re-set a Statement class if necessary.
if (!empty($this->statementClass)) {
$this->connection->setAttribute(\PDO::ATTR_STATEMENT_CLASS, array($this->statementClass, array($this)));
}
$this->setPrefix(isset($this->connectionOptions['prefix']) ? $this->connectionOptions['prefix'] : '');
public function __sleep() {
throw new \LogicException('The database connection is not serializable. This probably means you are serializing an object that has an indirect reference to the database connection. Adjust your code so that is not necessary. Alternatively, look at DependencySerializationTrait as a temporary solution.');
}
}
......@@ -10,6 +10,7 @@
use Drupal\Component\Serialization\SerializationInterface;
use Drupal\Core\Database\Query\Merge;
use Drupal\Core\Database\Connection;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
/**
* Defines a default key/value store implementation.
......@@ -19,6 +20,8 @@
*/
class DatabaseStorage extends StorageBase {
use DependencySerializationTrait;
/**
* The serialization class to use.
*
......
......@@ -30,7 +30,7 @@ class Batch extends DatabaseQueue {
* to be claimed repeatedly until it is deleted.
*/
public function claimItem($lease_time = 0) {
$item = db_query_range('SELECT data, item_id FROM {queue} q WHERE name = :name ORDER BY item_id ASC', 0, 1, array(':name' => $this->name))->fetchObject();
$item = $this->connection->queryRange('SELECT data, item_id FROM {queue} q WHERE name = :name ORDER BY item_id ASC', 0, 1, array(':name' => $this->name))->fetchObject();
if ($item) {
$item->data = unserialize($item->data);
return $item;
......@@ -49,7 +49,7 @@ public function claimItem($lease_time = 0) {
*/
public function getAllItems() {
$result = array();
$items = db_query('SELECT data FROM {queue} q WHERE name = :name ORDER BY item_id ASC', array(':name' => $this->name))->fetchAll();
$items = $this->connection->query('SELECT data FROM {queue} q WHERE name = :name ORDER BY item_id ASC', array(':name' => $this->name))->fetchAll();
foreach ($items as $item) {
$result[] = unserialize($item->data);
}
......
......@@ -9,6 +9,7 @@
use Drupal\Component\Utility\Crypt;
use Drupal\Core\Database\Connection;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Drupal\Core\Site\Settings;
use Drupal\Core\Utility\Error;
use Symfony\Component\HttpFoundation\RequestStack;
......@@ -19,6 +20,8 @@
*/
class SessionHandler extends AbstractProxy implements \SessionHandlerInterface {
use DependencySerializationTrait;
/**
* The request stack.
*
......
......@@ -9,6 +9,7 @@
use Drupal\Component\Utility\Crypt;
use Drupal\Core\Database\Connection;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;
......@@ -31,6 +32,8 @@
*/
class SessionManager extends NativeSessionStorage implements SessionManagerInterface {
use DependencySerializationTrait;
/**
* The request stack.
*
......
......@@ -63,7 +63,7 @@ private function __clone() {
* Prevents settings from being serialized.
*/
public function __sleep() {
throw new \BadMethodCallException('Settings cannot be serialized.');
throw new \LogicException('Settings can not be serialized. This probably means you are serializing an object that has an indirect reference to the Settings object. Adjust your code so that is not necessary.');
}
/**
......
......@@ -220,84 +220,6 @@ function testOpenSelectQueryClose() {
$this->assertNoConnection($id);
}
/**
* Tests the serialization and unserialization of a database connection.
*/
public function testConnectionSerialization() {
$db = Database::getConnection('default', 'default');
try {
$serialized = serialize($db);
$this->pass('The database connection can be serialized.');
$unserialized = unserialize($serialized);
$this->assertTrue(get_class($unserialized) === get_class($db));
}
catch (\Exception $e) {
$this->fail('The database connection cannot be serialized.');
}
// Ensure that all properties on the unserialized object are the same.
$db_reflection = new \ReflectionObject($db);
$unserialized_reflection = new \ReflectionObject($unserialized);
foreach ($db_reflection->getProperties() as $value) {
$value->setAccessible(TRUE);
// Skip properties that are lazily populated on access.
if ($value->getName() === 'driverClasses' || $value->getName() === 'schema') {
continue;
}
$unserialized_property = $unserialized_reflection->getProperty($value->getName());
$unserialized_property->setAccessible(TRUE);
// For the PDO object, just check the statement class attribute.
if ($value->getName() == 'connection') {
$db_statement_class = $unserialized_property->getValue($db)->getAttribute(\PDO::ATTR_STATEMENT_CLASS);
$unserialized_statement_class = $unserialized_property->getValue($unserialized)->getAttribute(\PDO::ATTR_STATEMENT_CLASS);
// Assert the statement class.
$this->assertEqual($unserialized_statement_class[0], $db_statement_class[0]);
// Assert the connection argument that is passed into the statement.
$this->assertEqual(get_class($unserialized_statement_class[1][0]), get_class($db_statement_class[1][0]));
}
else {
$actual = $unserialized_property->getValue($unserialized);
$expected = $value->getValue($db);
$this->assertEqual($actual, $expected, vsprintf('Unserialized Connection property %s value %s is equal to expected %s', array(
var_export($value->getName(), TRUE),
is_object($actual) ? print_r($actual, TRUE) : var_export($actual, TRUE),
is_object($expected) ? print_r($expected, TRUE) : var_export($expected, TRUE),
)));
}
}
// By using "key", we ensure that its not a key used in the serialized PHP.
$not_serialized_properties = ['"connection"', '"connectionOptions"', '"schema"', '"prefixes"', '"prefixReplace"', '"driverClasses"'];
foreach ($not_serialized_properties as $property) {
$this->assertIdentical(FALSE, strpos($serialized, $property));
}
// Serialize the DB connection again, but this time change the connection
// information under the hood.
$serialized = serialize($db);
$db_connection_info = Database::getAllConnectionInfo();
// Use reflection to empty out $databaseInfo.
$reflection_class = new \ReflectionClass('Drupal\Core\Database\Database');
$database_info_reflection = $reflection_class->getProperty('databaseInfo');
$database_info_reflection->setAccessible(TRUE);
$database_info_reflection->setValue(NULL, []);
// Setup a different DB connection which should be picked up after the
// unserialize.
$db_connection_info['default']['default']['extra'] = 'value';
Database::setMultipleConnectionInfo($db_connection_info);
/** @var \Drupal\Core\Database\Connection $db */
$db = unserialize($serialized);
$this->assertEqual($db->getConnectionOptions()['extra'], 'value');
}
/**
* Tests pdo options override.
*/
......
......@@ -672,12 +672,4 @@ function testRequiredAttribute() {
$this->assertTrue(!empty($element), 'The textarea has the proper required attribute.');
}
/**
* Tests a form with a form state storing a database connection.
*/
public function testFormStateDatabaseConnection() {
$this->assertNoText('Database connection found');
$this->drupalPostForm('form-test/form_state-database', array(), t('Submit'));
$this->assertText('Database connection found');
}
}
......@@ -426,14 +426,6 @@ form_test.group_vertical_tabs:
requirements:
_access: 'TRUE'
form_test.form_state_database:
path: '/form-test/form_state-database'
defaults:
_form: '\Drupal\form_test\Form\FormTestFormStateDatabaseForm'
_title: 'Form state with a database connection'
requirements:
_access: 'TRUE'
form_test.two_instances:
path: '/form-test/two-instances-of-same-form'
defaults:
......
<?php
/**
* @file
* Contains \Drupal\form_test\Form\FormTestFormStateDatabaseForm.
*/
namespace Drupal\form_test\Form;
use Drupal\Core\Database\Database;
use Drupal\Core\Form\FormBase;
use Drupal\Core\Form\FormStateInterface;
/**
* Builds a form which gets the database connection stored in the form state.
*/
class FormTestFormStateDatabaseForm extends FormBase {
/**
* {@inheritdoc}
*/
public function getFormId() {
return 'form_test_form_state_database';
}
/**
* {@inheritdoc}
*/
public function buildForm(array $form, FormStateInterface $form_state) {
$form['text'] = array(
'#type' => 'textfield',
'#title' => t('Text field'),
);
$form['test_submit'] = array(
'#type' => 'submit',
'#value' => t('Submit'),
);
$db = Database::getConnection('default');
$form_state->set('database', $db);
$form_state->set('database_class', get_class($db));
if ($form_state->has('database_connection_found')) {
$form['database']['#markup'] = 'Database connection found';
}
return $form;
}
/**
* {@inheritdoc}
*/
public function submitForm(array &$form, FormStateInterface $form_state) {
$form_state->setCached();
$form_state->setRebuild();
$database_class = $form_state->get('database_class');
if ($form_state->get('database') instanceof $database_class) {
$form_state->set('database_connection_found', TRUE);
}
}
}
......@@ -112,7 +112,7 @@ public function providerTestGetHashSaltEmpty() {
*
* @covers ::__sleep
*
* @expectedException \BadMethodCallException
* @expectedException \LogicException
*/
public function testSerialize() {
serialize(new Settings([]));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment