Commit 0fafbc39 authored by alexpott's avatar alexpott
Browse files

Issue #2428695 by amateescu, daffie: SQLite date handling is wrongly...

Issue #2428695 by amateescu, daffie: SQLite date handling is wrongly implemented and arguments handling needs override
parent 7d31cc86
......@@ -68,9 +68,6 @@ class Connection extends DatabaseConnection {
public function __construct(\PDO $connection, array $connection_options) {
parent::__construct($connection, $connection_options);
// We don't need a specific PDOStatement class here, we simulate it below.
$this->statementClass = NULL;
// This driver defaults to transaction support, except if explicitly passed FALSE.
$this->transactionSupport = $this->transactionalDDLSupport = !isset($connection_options['transactions']) || $connection_options['transactions'] !== FALSE;
......@@ -270,14 +267,71 @@ public static function sqlFunctionRegexp($pattern, $subject) {
}
/**
* SQLite-specific implementation of DatabaseConnection::prepare().
*
* We don't use prepared statements at all at this stage. We just create
* a Statement object, that will create a PDOStatement
* using the semi-private PDOPrepare() method below.
* {@inheritdoc}
*/
public function prepare($statement, array $driver_options = array()) {
return new Statement($this->connection, $this, $statement, $driver_options);
protected function expandArguments(&$query, &$args) {
$modified = parent::expandArguments($query, $args);
// The PDO SQLite driver always replaces placeholders with strings, which
// breaks numeric expressions (e.g., COUNT(*) >= :count). Replace numeric
// placeholders in the query to work around this bug.
// @see http://bugs.php.net/bug.php?id=45259
if (empty($args)) {
return $modified;
}
// Check if $args is a simple numeric array.
if (range(0, count($args) - 1) === array_keys($args)) {
// In that case, we have unnamed placeholders.
$count = 0;
$new_args = array();
foreach ($args as $value) {
if (is_float($value) || is_int($value) || is_numeric($value)) {
if (is_float($value)) {
// Force the conversion to float so as not to loose precision
// in the automatic cast.
$value = sprintf('%F', $value);
}
$query = substr_replace($query, $value, strpos($query, '?'), 1);
}
else {
$placeholder = ':db_statement_placeholder_' . $count++;
$query = substr_replace($query, $placeholder, strpos($query, '?'), 1);
$new_args[$placeholder] = $value;
}
}
$args = $new_args;
$modified = TRUE;
}
// Otherwise this is using named placeholders.
else {
foreach ($args as $placeholder => $value) {
if (is_float($value) || is_int($value) || is_numeric($value)) {
if (is_float($value)) {
// Force the conversion to float so as not to loose precision
// in the automatic cast.
$value = sprintf('%F', $value);
}
// We will remove this placeholder from the query as PDO throws an
// exception if the number of placeholders in the query and the
// arguments does not match.
unset($args[$placeholder]);
// PDO allows placeholders to not be prefixed by a colon. See
// http://marc.info/?l=php-internals&m=111234321827149&w=2 for
// more.
if ($placeholder[0] != ':') {
$placeholder = ":$placeholder";
}
// When replacing the placeholders, make sure we search for the
// exact placeholder. For example, if searching for
// ':db_placeholder_1', do not replace ':db_placeholder_11'.
$query = preg_replace('/' . preg_quote($placeholder, '/') . '\b/', $value, $query);
$modified = TRUE;
}
}
}
return $modified;
}
public function queryRange($query, $from, $count, array $args = array(), array $options = array()) {
......
<?php
/**
* @file
* Definition of Drupal\Core\Database\Driver\sqlite\Statement
*/
namespace Drupal\Core\Database\Driver\sqlite;
use Drupal\Core\Database\StatementPrefetch;
use Drupal\Core\Database\StatementInterface;
/**
* Specific SQLite implementation of DatabaseConnection.
*
* The PDO SQLite driver only closes SELECT statements when the PDOStatement
* destructor is called and SQLite does not allow data change (INSERT,
* UPDATE etc) on a table which has open SELECT statements. This is a
* user-space mock of PDOStatement that buffers all the data and doesn't
* have those limitations.
*/
class Statement extends StatementPrefetch implements \Iterator, StatementInterface {
/**
* SQLite specific implementation of getStatement().
*
* The PDO SQLite layer doesn't replace numeric placeholders in queries
* correctly, and this makes numeric expressions (such as COUNT(*) >= :count)
* fail. We replace numeric placeholders in the query ourselves to work
* around this bug.
*
* See http://bugs.php.net/bug.php?id=45259 for more details.
*/
protected function getStatement($query, &$args = array()) {
if (count($args)) {
// Check if $args is a simple numeric array.
if (range(0, count($args) - 1) === array_keys($args)) {
// In that case, we have unnamed placeholders.
$count = 0;
$new_args = array();
foreach ($args as $value) {
if (is_float($value) || is_int($value)) {
if (is_float($value)) {
// Force the conversion to float so as not to loose precision
// in the automatic cast.
$value = sprintf('%F', $value);
}
$query = substr_replace($query, $value, strpos($query, '?'), 1);
}
else {
$placeholder = ':db_statement_placeholder_' . $count++;
$query = substr_replace($query, $placeholder, strpos($query, '?'), 1);
$new_args[$placeholder] = $value;
}
}
$args = $new_args;
}
else {
// Else, this is using named placeholders.
foreach ($args as $placeholder => $value) {
if (is_float($value) || is_int($value)) {
if (is_float($value)) {
// Force the conversion to float so as not to loose precision
// in the automatic cast.
$value = sprintf('%F', $value);
}
// We will remove this placeholder from the query as PDO throws an
// exception if the number of placeholders in the query and the
// arguments does not match.
unset($args[$placeholder]);
// PDO allows placeholders to not be prefixed by a colon. See
// http://marc.info/?l=php-internals&m=111234321827149&w=2 for
// more.
if ($placeholder[0] != ':') {
$placeholder = ":$placeholder";
}
// When replacing the placeholders, make sure we search for the
// exact placeholder. For example, if searching for
// ':db_placeholder_1', do not replace ':db_placeholder_11'.
$query = preg_replace('/' . preg_quote($placeholder) . '\b/', $value, $query);
}
}
}
}
return $this->dbh->prepare($query);
}
public function execute($args = array(), $options = array()) {
try {
$return = parent::execute($args, $options);
}
catch (\PDOException $e) {
if (!empty($e->errorInfo[1]) && $e->errorInfo[1] === 17) {
// The schema has changed. SQLite specifies that we must resend the query.
$return = parent::execute($args, $options);
}
else {
// Rethrow the exception.
throw $e;
}
}
// In some weird cases, SQLite will prefix some column names by the name
// of the table. We post-process the data, by renaming the column names
// using the same convention as MySQL and PostgreSQL.
$rename_columns = array();
foreach ($this->columnNames as $k => $column) {
// In some SQLite versions, SELECT DISTINCT(field) will return "(field)"
// instead of "field".
if (preg_match("/^\((.*)\)$/", $column, $matches)) {
$rename_columns[$column] = $matches[1];
$this->columnNames[$k] = $matches[1];
$column = $matches[1];
}
// Remove "table." prefixes.
if (preg_match("/^.*\.(.*)$/", $column, $matches)) {
$rename_columns[$column] = $matches[1];
$this->columnNames[$k] = $matches[1];
}
}
if ($rename_columns) {
// DatabaseStatementPrefetch already extracted the first row,
// put it back into the result set.
if (isset($this->currentRow)) {
$this->data[0] = &$this->currentRow;
}
// Then rename all the columns across the result set.
foreach ($this->data as $k => $row) {
foreach ($rename_columns as $old_column => $new_column) {
$this->data[$k][$new_column] = $this->data[$k][$old_column];
unset($this->data[$k][$old_column]);
}
}
// Finally, extract the first row again.
$this->currentRow = $this->data[0];
unset($this->data[0]);
}
return $return;
}
}
......@@ -18,15 +18,47 @@
*/
class DatabaseExceptionWrapperTest extends KernelTestBase {
function testDatabaseExceptionWrapper() {
/**
* Tests the expected database exception thrown for prepared statements.
*/
public function testPreparedStatement() {
$connection = Database::getConnection();
$query = $connection->prepare('bananas');
try {
// SQLite validates the syntax upon preparing a statement already.
// @throws \PDOException
$query = $connection->prepare('bananas');
// MySQL only validates the syntax upon trying to execute a query.
// @throws \Drupal\Core\Database\DatabaseExceptionWrapper
$connection->query($query);
$this->fail('The expected exception is not thrown.');
$this->fail('Expected PDOException or DatabaseExceptionWrapper, none was thrown.');
}
catch (\PDOException $e) {
$this->pass('Expected PDOException was thrown.');
}
catch (DatabaseExceptionWrapper $e) {
$this->pass('Expected DatabaseExceptionWrapper was thrown.');
}
catch (\Exception $e) {
$this->fail("Thrown exception is not a PDOException:\n" . (string) $e);
}
}
/**
* Tests the expected database exception thrown for inexistent tables.
*/
public function testQueryThrowsDatabaseExceptionWrapperException() {
$connection = Database::getConnection();
try {
$connection->query('SELECT * FROM {does_not_exist}');
$this->fail('Expected PDOException, none was thrown.');
}
catch (DatabaseExceptionWrapper $e) {
$this->pass('The expected exception has been thrown.');
$this->pass('Expected DatabaseExceptionWrapper was thrown.');
}
catch (\Exception $e) {
$this->fail("Thrown exception is not a DatabaseExceptionWrapper:\n" . (string) $e);
}
}
......
......@@ -66,4 +66,20 @@ public function testArrayArgumentsSQLInjection() {
$this->assertFalse($result, 'SQL injection attempt did not result in a row being inserted in the database table.');
}
/**
* Tests numeric query parameter expansion in expressions.
*
* @see \Drupal\Core\Database\Driver\sqlite\Connection::expandArguments()
* @see http://bugs.php.net/bug.php?id=45259
*/
public function testNumericExpressionSubstitution() {
$count = db_query('SELECT COUNT(*) >= 3 FROM {test}')->fetchField();
$this->assertEqual((bool) $count, TRUE);
$count = db_query('SELECT COUNT(*) >= :count FROM {test}', array(
':count' => 3,
))->fetchField();
$this->assertEqual((bool) $count, TRUE);
}
}
......@@ -1679,7 +1679,7 @@ public function getDateField($field) {
break;
case 'sqlite':
if (!empty($offset)) {
$field = "($field + '$offset_seconds')";
$field = "($field + $offset_seconds)";
}
break;
}
......@@ -1793,7 +1793,18 @@ public function getDateFormat($field, $format) {
'A' => '',
);
$format = strtr($format, $replace);
return "strftime('$format', $field, 'unixepoch')";
$expression = "strftime('$format', $field, 'unixepoch')";
// The expression yields a string, but the comparison value is an
// integer in case the comparison value is a float, integer, or numeric.
// All of the above SQLite format tokens only produce integers. However,
// the given $format may contain 'Y-m-d', which results in a string.
// @see \Drupal\Core\Database\Driver\sqlite\Connection::expandArguments()
// @see http://www.sqlite.org/lang_datefunc.html
// @see http://www.sqlite.org/lang_expr.html#castexpr
if (preg_match('/^(?:%\w)+$/', $format)) {
$expression = "CAST($expression AS NUMERIC)";
}
return $expression;
}
}
......
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