Commit f46d6ad9 authored by alexpott's avatar alexpott
Browse files

Issue #2358079 by pwolanin, Berdir: Require a specific placeholder format in...

Issue #2358079 by pwolanin, Berdir: Require a specific placeholder format in db_query() in order to trigger argument expansion, and require explicit 'IN' parameter for conditions
parent 58a891fa
......@@ -246,7 +246,9 @@
* @param $query
* The prepared statement query to run. Although it will accept both named and
* unnamed placeholders, named placeholders are strongly preferred as they are
* more self-documenting.
* more self-documenting. If the argument corresponding to a placeholder is
* an array of values to be expanded, e.g. for an IN query, the placeholder
* should be named with a trailing bracket like :example[]
* @param $args
* An array of values to substitute into the query. If the query uses named
* placeholders, this is an associative array in any order. If the query uses
......
......@@ -86,7 +86,7 @@ public function getMultiple(&$cids, $allow_invalid = FALSE) {
// ::select() is a much smaller proportion of the request.
$result = array();
try {
$result = $this->connection->query('SELECT cid, data, created, expire, serialized, tags, checksum FROM {' . $this->connection->escapeTable($this->bin) . '} WHERE cid IN (:cids)', array(':cids' => array_keys($cid_mapping)));
$result = $this->connection->query('SELECT cid, data, created, expire, serialized, tags, checksum FROM {' . $this->connection->escapeTable($this->bin) . '} WHERE cid IN ( :cids[] )', array(':cids[]' => array_keys($cid_mapping)));
}
catch (\Exception $e) {
// Nothing to do.
......
......@@ -108,7 +108,7 @@ public function calculateChecksum(array $tags) {
if ($query_tags) {
$db_tags = array();
try {
$db_tags = $this->connection->query('SELECT tag, invalidations FROM {cachetags} WHERE tag IN (:tags)', array(':tags' => $query_tags))
$db_tags = $this->connection->query('SELECT tag, invalidations FROM {cachetags} WHERE tag IN ( :tags[] )', array(':tags[]' => $query_tags))
->fetchAllKeyed();
$this->tagCache += $db_tags;
}
......
......@@ -107,7 +107,7 @@ public function read($name) {
public function readMultiple(array $names) {
$list = array();
try {
$list = $this->connection->query('SELECT name, data FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name IN (:names)', array(':collection' => $this->collection, ':names' => $names), $this->options)->fetchAllKeyed();
$list = $this->connection->query('SELECT name, data FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name IN ( :names[] )', array(':collection' => $this->collection, ':names[]' => $names), $this->options)->fetchAllKeyed();
foreach ($list as &$data) {
$data = $this->decode($data);
}
......
......@@ -517,6 +517,7 @@ protected function filterComment($comment = '') {
*
* @throws \PDOException
* @throws \Drupal\Core\Database\IntegrityConstraintViolationException
* @throws \InvalidArgumentException
*/
public function query($query, array $args = array(), $options = array()) {
......@@ -581,38 +582,48 @@ public function query($query, array $args = array(), $options = array()) {
* Drupal supports an alternate syntax for doing arrays of values. We
* therefore need to expand them out into a full, executable query string.
*
* @param $query
* @param string $query
* The query string to modify.
* @param $args
* @param array $args
* The arguments for the query.
*
* @return
* @return bool
* TRUE if the query was modified, FALSE otherwise.
*/
protected function expandArguments(&$query, &$args) {
$modified = FALSE;
// If the placeholder value to insert is an array, assume that we need
// to expand it out into a comma-delimited set of placeholders.
foreach (array_filter($args, 'is_array') as $key => $data) {
// If the placeholder indicated the value to use is an array, we need to
// expand it out into a comma-delimited set of placeholders.
foreach ($args as $key => $data) {
$is_bracket_placeholder = substr($key, -2) === '[]';
$is_array_data = is_array($data);
if ($is_bracket_placeholder && !$is_array_data) {
throw new \InvalidArgumentException('Placeholders with a trailing [] can only be expanded with an array of values.');
}
elseif (!$is_bracket_placeholder) {
if ($is_array_data) {
throw new \InvalidArgumentException('Placeholders must have a trailing [] if they are to be expanded with an array of values.');
}
// Scalar placeholder - does not need to be expanded.
continue;
}
// Handle expansion of arrays.
$key_name = str_replace('[]', '__', $key);
$new_keys = array();
// We require placeholders to have trailing brackets if the developer
// intends them to be expanded to an array to make the intent explicit.
foreach (array_values($data) as $i => $value) {
// This assumes that there are no other placeholders that use the same
// name. For example, if the array placeholder is defined as :example
// name. For example, if the array placeholder is defined as :example[]
// and there is already an :example_2 placeholder, this will generate
// a duplicate key. We do not account for that as the calling code
// is already broken if that happens.
$new_keys[$key . '_' . $i] = $value;
$new_keys[$key_name . $i] = $value;
}
// Update the query with the new placeholders.
// preg_replace is necessary to ensure the replacement does not affect
// placeholders that start with the same exact text. For example, if the
// query contains the placeholders :foo and :foobar, and :foo has an
// array of values, using str_replace would affect both placeholders,
// but using the following preg_replace would only affect :foo because
// it is followed by a non-word character.
$query = preg_replace('#' . $key . '\b#', implode(', ', array_keys($new_keys)), $query);
$query = str_replace($key, implode(', ', array_keys($new_keys)), $query);
// Update the args array with the new placeholders.
unset($args[$key]);
......
......@@ -68,14 +68,9 @@ public function count() {
/**
* Implements Drupal\Core\Database\Query\ConditionInterface::condition().
*/
public function condition($field, $value = NULL, $operator = NULL) {
if (!isset($operator)) {
if (is_array($value)) {
$operator = 'IN';
}
else {
$operator = '=';
}
public function condition($field, $value = NULL, $operator = '=') {
if (empty($operator)) {
$operator = '=';
}
if (empty($value) && is_array($value)) {
throw new InvalidQueryException(sprintf("Query condition '%s %s ()' cannot be empty.", $field, $operator));
......@@ -211,7 +206,7 @@ public function compile(Connection $connection, PlaceholderInterface $queryPlace
// We assume that if there is a delimiter, then the value is an
// array. If not, it is a scalar. For simplicity, we first convert
// up to an array so that we can build the placeholders in the same way.
elseif (!$operator['delimiter']) {
elseif (!$operator['delimiter'] && !is_array($condition['value'])) {
$condition['value'] = array($condition['value']);
}
if ($operator['use_value']) {
......
......@@ -46,8 +46,7 @@ interface ConditionInterface {
* the array is dependent on the $operator.
* @param $operator
* The comparison operator, such as =, <, or >=. It also accepts more
* complex options such as IN, LIKE, LIKE BINARY, or BETWEEN. Defaults to IN
* if $value is an array, and = otherwise.
* complex options such as IN, LIKE, LIKE BINARY, or BETWEEN. Defaults to =.
*
* @return \Drupal\Core\Database\Query\ConditionInterface
* The called object.
......@@ -55,7 +54,7 @@ interface ConditionInterface {
* @see \Drupal\Core\Database\Query\ConditionInterface::isNull()
* @see \Drupal\Core\Database\Query\ConditionInterface::isNotNull()
*/
public function condition($field, $value = NULL, $operator = NULL);
public function condition($field, $value = NULL, $operator = '=');
/**
* Adds an arbitrary WHERE clause to the query.
......
......@@ -54,7 +54,7 @@ public function __construct(Connection $connection, $table, array $options = arr
/**
* Implements Drupal\Core\Database\Query\ConditionInterface::condition().
*/
public function condition($field, $value = NULL, $operator = NULL) {
public function condition($field, $value = NULL, $operator = '=') {
$this->condition->condition($field, $value, $operator);
return $this;
}
......
......@@ -342,7 +342,7 @@ public function key($field, $value = NULL) {
/**
* Implements Drupal\Core\Database\Query\ConditionInterface::condition().
*/
public function condition($field, $value = NULL, $operator = NULL) {
public function condition($field, $value = NULL, $operator = '=') {
$this->condition->condition($field, $value, $operator);
return $this;
}
......
......@@ -191,7 +191,7 @@ public function getMetaData($key) {
/**
* {@inheritdoc}
*/
public function condition($field, $value = NULL, $operator = NULL) {
public function condition($field, $value = NULL, $operator = '=') {
$this->where->condition($field, $value, $operator);
return $this;
}
......
......@@ -88,7 +88,7 @@ public function getMetaData($key) {
/* Implementations of Drupal\Core\Database\Query\ConditionInterface for the WHERE clause. */
public function condition($field, $value = NULL, $operator = NULL) {
public function condition($field, $value = NULL, $operator = '=') {
$this->query->condition($field, $value, $operator);
return $this;
}
......
......@@ -83,7 +83,7 @@ public function __construct(Connection $connection, $table, array $options = arr
/**
* Implements Drupal\Core\Database\Query\ConditionInterface::condition().
*/
public function condition($field, $value = NULL, $operator = NULL) {
public function condition($field, $value = NULL, $operator = '=') {
$this->condition->condition($field, $value, $operator);
return $this;
}
......
......@@ -454,7 +454,8 @@ abstract protected function doSave($id, EntityInterface $entity);
*/
protected function buildPropertyQuery(QueryInterface $entity_query, array $values) {
foreach ($values as $name => $value) {
$entity_query->condition($name, $value);
// Cast scalars to array so we can consistently use an IN condition.
$entity_query->condition($name, (array) $value, 'IN');
}
}
......
......@@ -667,7 +667,7 @@ protected function attachPropertyData(array &$entities) {
$table = $this->revisionDataTable ?: $this->dataTable;
$query = $this->database->select($table, 'data', array('fetch' => \PDO::FETCH_ASSOC))
->fields('data')
->condition($this->idKey, array_keys($entities))
->condition($this->idKey, array_keys($entities), 'IN')
->orderBy('data.' . $this->idKey);
if ($this->revisionDataTable) {
......@@ -676,7 +676,7 @@ protected function attachPropertyData(array &$entities) {
foreach ($entities as $values) {
$revision_ids[] = is_object($values) ? $values->getRevisionId() : $values[$this->revisionKey][LanguageInterface::LANGCODE_DEFAULT];
}
$query->condition($this->revisionKey, $revision_ids);
$query->condition($this->revisionKey, $revision_ids, 'IN');
}
$data = $query->execute();
......@@ -878,24 +878,24 @@ protected function doDelete($entities) {
$ids = array_keys($entities);
$this->database->delete($this->entityType->getBaseTable())
->condition($this->idKey, $ids)
->condition($this->idKey, $ids, 'IN')
->execute();
if ($this->revisionTable) {
$this->database->delete($this->revisionTable)
->condition($this->idKey, $ids)
->condition($this->idKey, $ids, 'IN')
->execute();
}
if ($this->dataTable) {
$this->database->delete($this->dataTable)
->condition($this->idKey, $ids)
->condition($this->idKey, $ids, 'IN')
->execute();
}
if ($this->revisionDataTable) {
$this->database->delete($this->revisionDataTable)
->condition($this->idKey, $ids)
->condition($this->idKey, $ids, 'IN')
->execute();
}
......
......@@ -75,7 +75,7 @@ public function has($key) {
public function getMultiple(array $keys) {
$values = array();
try {
$result = $this->connection->query('SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE name IN (:keys) AND collection = :collection', array(':keys' => $keys, ':collection' => $this->collection))->fetchAllAssoc('name');
$result = $this->connection->query('SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE name IN ( :keys[] ) AND collection = :collection', array(':keys[]' => $keys, ':collection' => $this->collection))->fetchAllAssoc('name');
foreach ($keys as $key) {
if (isset($result[$key])) {
$values[$key] = $this->serializer->decode($result[$key]->value);
......@@ -152,7 +152,7 @@ public function deleteMultiple(array $keys) {
// Delete in chunks when a large array is passed.
while ($keys) {
$this->connection->delete($this->table)
->condition('name', array_splice($keys, 0, 1000))
->condition('name', array_splice($keys, 0, 1000), 'IN')
->condition('collection', $this->collection)
->execute();
}
......
......@@ -51,10 +51,10 @@ public function has($key) {
*/
public function getMultiple(array $keys) {
$values = $this->connection->query(
'SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE expire > :now AND name IN (:keys) AND collection = :collection',
'SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE expire > :now AND name IN ( :keys[] ) AND collection = :collection',
array(
':now' => REQUEST_TIME,
':keys' => $keys,
':keys[]' => $keys,
':collection' => $this->collection,
))->fetchAllKeyed();
return array_map(array($this->serializer, 'decode'), $values);
......
......@@ -114,7 +114,7 @@ public function checkNodeAccess(array $tree) {
$nids = array_keys($node_links);
$query = $this->queryFactory->get('node');
$query->condition('nid', $nids);
$query->condition('nid', $nids, 'IN');
// Allows admins to view all nodes, by both disabling node_access
// query rewrite as well as not checking for the node status. The
......
......@@ -118,7 +118,7 @@ public function delete($conditions) {
*/
public function preloadPathAlias($preloaded, $langcode) {
$args = array(
':system' => $preloaded,
':system[]' => $preloaded,
':langcode' => $langcode,
':langcode_undetermined' => LanguageInterface::LANGCODE_NOT_SPECIFIED,
);
......@@ -132,13 +132,13 @@ public function preloadPathAlias($preloaded, $langcode) {
if ($langcode == LanguageInterface::LANGCODE_NOT_SPECIFIED) {
// Prevent PDO from complaining about a token the query doesn't use.
unset($args[':langcode']);
$result = $this->connection->query('SELECT source, alias FROM {url_alias} WHERE source IN (:system) AND langcode = :langcode_undetermined ORDER BY pid ASC', $args);
$result = $this->connection->query('SELECT source, alias FROM {url_alias} WHERE source IN ( :system[] ) AND langcode = :langcode_undetermined ORDER BY pid ASC', $args);
}
elseif ($langcode < LanguageInterface::LANGCODE_NOT_SPECIFIED) {
$result = $this->connection->query('SELECT source, alias FROM {url_alias} WHERE source IN (:system) AND langcode IN (:langcode, :langcode_undetermined) ORDER BY langcode ASC, pid ASC', $args);
$result = $this->connection->query('SELECT source, alias FROM {url_alias} WHERE source IN ( :system[] ) AND langcode IN (:langcode, :langcode_undetermined) ORDER BY langcode ASC, pid ASC', $args);
}
else {
$result = $this->connection->query('SELECT source, alias FROM {url_alias} WHERE source IN (:system) AND langcode IN (:langcode, :langcode_undetermined) ORDER BY langcode DESC, pid ASC', $args);
$result = $this->connection->query('SELECT source, alias FROM {url_alias} WHERE source IN ( :system[] ) AND langcode IN (:langcode, :langcode_undetermined) ORDER BY langcode DESC, pid ASC', $args);
}
return $result->fetchAllKeyed();
......
......@@ -178,7 +178,7 @@ public function getRoutesByNames($names) {
$routes_to_load = array_diff($names, array_keys($this->routes));
if ($routes_to_load) {
$result = $this->connection->query('SELECT name, route FROM {' . $this->connection->escapeTable($this->tableName) . '} WHERE name IN (:names)', array(':names' => $routes_to_load));
$result = $this->connection->query('SELECT name, route FROM {' . $this->connection->escapeTable($this->tableName) . '} WHERE name IN ( :names[] )', array(':names[]' => $routes_to_load));
$routes = $result->fetchAllKeyed();
foreach ($routes as $name => $route) {
......@@ -289,8 +289,8 @@ protected function getRoutesByPath($path) {
return $collection;
}
$routes = $this->connection->query("SELECT name, route FROM {" . $this->connection->escapeTable($this->tableName) . "} WHERE pattern_outline IN (:patterns) ORDER BY fit DESC, name ASC", array(
':patterns' => $ancestors,
$routes = $this->connection->query("SELECT name, route FROM {" . $this->connection->escapeTable($this->tableName) . "} WHERE pattern_outline IN ( :patterns[] ) ORDER BY fit DESC, name ASC", array(
':patterns[]' => $ancestors,
))
->fetchAllKeyed();
......
......@@ -214,7 +214,7 @@ public function save(array $form, FormStateInterface $form_state) {
*/
public function validateForm(array &$form, FormStateInterface $form_state) {
if ($this->entity->isNew()) {
$exists = $this->blockContentStorage->loadByProperties(array('info' => $form_state->getValue('info')));
$exists = $this->blockContentStorage->loadByProperties(array('info' => $form_state->getValue(['info', 0, 'value'])));
if (!empty($exists)) {
$form_state->setErrorByName('info', $this->t('A block with description %name already exists.', array(
'%name' => $form_state->getValue(array('info', 0, 'value')),
......
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