Skip to content

Commit

Permalink
Merge pull request #1677 from codeigniter4/escapeinserts
Browse files Browse the repository at this point in the history
Implement Don't Escape feature for db engine
  • Loading branch information
lonnieezell authored Jan 23, 2019
2 parents b510f7d + 113ed76 commit df38172
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 72 deletions.
43 changes: 28 additions & 15 deletions system/Database/BaseBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ public function __construct($tableName, ConnectionInterface &$db, array $options

$this->db = $db;

// turn off automatic escape flags
$this->db->setEscapeFlags(false);

$this->from($tableName);

if (! empty($options))
Expand Down Expand Up @@ -664,7 +667,7 @@ protected function whereHaving($qb_key, $key, $value = null, $type = 'AND ', $es
$op = $this->getOperator($k);
$k = trim(str_replace($op, '', $k));

$bind = $this->setBind($k, $v);
$bind = $this->setBind($k, $v, $escape);

if (empty($op))
{
Expand Down Expand Up @@ -814,7 +817,7 @@ protected function _whereIn($key = null, $values = null, $not = false, $type = '
$not = ($not) ? ' NOT' : '';

$where_in = array_values($values);
$ok = $this->setBind($ok, $where_in);
$ok = $this->setBind($ok, $where_in, $escape);

$prefix = empty($this->QBWhere) ? $this->groupGetType('') : $this->groupGetType($type);

Expand Down Expand Up @@ -955,19 +958,19 @@ protected function _like($field, $match = '', $type = 'AND ', $side = 'both', $n

if ($side === 'none')
{
$bind = $this->setBind($k, $v);
$bind = $this->setBind($k, $v, $escape);
}
elseif ($side === 'before')
{
$bind = $this->setBind($k, "%$v");
$bind = $this->setBind($k, "%$v", $escape);
}
elseif ($side === 'after')
{
$bind = $this->setBind($k, "$v%");
$bind = $this->setBind($k, "$v%", $escape);
}
else
{
$bind = $this->setBind($k, "%$v%");
$bind = $this->setBind($k, "%$v%", $escape);
}

$like_statement = $this->_like_statement($prefix, $k, $not, $bind, $insensitiveSearch);
Expand Down Expand Up @@ -1345,7 +1348,7 @@ public function set($key, $value = '', $escape = null)
{
if ($escape)
{
$bind = $this->setBind($k, $v);
$bind = $this->setBind($k, $v, $escape);
$this->QBSet[$this->db->protectIdentifiers($k, false, $escape)] = ":$bind:";
}
else
Expand Down Expand Up @@ -1415,7 +1418,7 @@ public function getCompiledSelect($reset = true)
protected function compileFinalQuery(string $sql): string
{
$query = new Query($this->db);
$query->setQuery($sql, $this->binds);
$query->setQuery($sql, $this->binds, false);

if (! empty($this->db->swapPre) && ! empty($this->db->DBPrefix))
{
Expand Down Expand Up @@ -1718,7 +1721,7 @@ public function setInsertBatch($key, $value = '', $escape = null)
$clean = [];
foreach ($row as $k => $value)
{
$clean[] = ':' . $this->setBind($k, $value) . ':';
$clean[] = ':' . $this->setBind($k, $value, $escape) . ':';
}

$row = $clean;
Expand Down Expand Up @@ -2225,7 +2228,7 @@ public function setUpdateBatch($key, $index = '', $escape = null)
$index_set = true;
}

$bind = $this->setBind($k2, $v2);
$bind = $this->setBind($k2, $v2, $escape);

$clean[$this->db->protectIdentifiers($k2, false, $escape)] = ":$bind:";
}
Expand Down Expand Up @@ -2940,17 +2943,24 @@ protected function getOperator($str)

/**
* Stores a bind value after ensuring that it's unique.
* While it might be nicer to have named keys for our binds array
* with PHP 7+ we get a huge memory/performance gain with indexed
* arrays instead, so lets take advantage of that here.
*
* @param string $key
* @param null $value
* @param string $key
* @param null $value
* @param boolean $escape
*
* @return string
*/
protected function setBind(string $key, $value = null)
protected function setBind(string $key, $value = null, bool $escape = true)
{
if (! array_key_exists($key, $this->binds))
{
$this->binds[$key] = $value;
$this->binds[$key] = [
$value,
$escape,
];

return $key;
}
Expand All @@ -2962,7 +2972,10 @@ protected function setBind(string $key, $value = null)
++$count;
}

$this->binds[$key . $count] = $value;
$this->binds[$key . $count] = [
$value,
$escape,
];

return $key . $count;
}
Expand Down
35 changes: 30 additions & 5 deletions system/Database/BaseConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,16 @@ abstract class BaseConnection implements ConnectionInterface
*/
protected $aliasedTables = [];

/**
* Should the bindings be collected with a default escape value?
* The Builder will automatically set this to false, so that any
* $db->query('...', [binds]); Will result in bindings being
* automatically escaped.
*
* @var boolean
*/
protected $setEscapeFlags = true;

//--------------------------------------------------------------------

/**
Expand Down Expand Up @@ -577,6 +587,20 @@ public function addTableAlias(string $table)
return $this;
}

/**
* Should query() automatically attach escape flags to each bound var?
*
* @param boolean $setFlags
*
* @return $this
*/
public function setEscapeFlags(bool $setFlags)
{
$this->setEscapeFlags = $setFlags;

return $this;
}

/**
* Executes the query against the database.
*
Expand All @@ -596,9 +620,11 @@ abstract protected function execute($sql);
* Should automatically handle different connections for read/write
* queries if needed.
*
* @param string $sql
* @param array ...$binds
* @param string $queryClass
* @param string $sql
* @param array ...$binds
* @param string $queryClass
* @param boolean $setEscape
*
* @return BaseResult|Query|false
*/
public function query(string $sql, $binds = null, $queryClass = 'CodeIgniter\\Database\\Query')
Expand All @@ -609,13 +635,12 @@ public function query(string $sql, $binds = null, $queryClass = 'CodeIgniter\\Da
}

$resultClass = str_replace('Connection', 'Result', get_class($this));

/**
* @var Query $query
*/
$query = new $queryClass($this);

$query->setQuery($sql, $binds);
$query->setQuery($sql, $binds, $this->setEscapeFlags);

if (! empty($this->swapPre) && ! empty($this->DBPrefix))
{
Expand Down
28 changes: 22 additions & 6 deletions system/Database/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,32 @@ public function __construct(&$db)
/**
* Sets the raw query string to use for this statement.
*
* @param string $sql
* @param array $binds
* @param string $sql
* @param array $binds
* @param boolean $setEscape
*
* @return mixed
*/
public function setQuery(string $sql, $binds = null)
public function setQuery(string $sql, $binds = null, bool $setEscape = true)
{
$this->originalQueryString = $sql;

if (! is_null($binds))
{
if (! is_array($binds))
{
$binds = [$binds];
}

if ($setEscape)
{
array_walk($binds, function (&$item) {
$item = [
$item,
true,
];
});
}
$this->binds = $binds;
}

Expand Down Expand Up @@ -407,12 +422,13 @@ protected function matchNamedBinds(string $sql, array $binds)

foreach ($binds as $placeholder => $value)
{
$escapedValue = $this->db->escape($value);
// $value[1] contains the boolean whether should be escaped or not
$escapedValue = $value[1] ? $this->db->escape($value[0]) : $value[0];

// In order to correctly handle backlashes in saved strings
// we will need to preg_quote, so remove the wrapping escape characters
// otherwise it will get escaped.
if (is_array($value))
if (is_array($value[0]))
{
$escapedValue = '(' . implode(',', $escapedValue) . ')';
}
Expand Down Expand Up @@ -460,7 +476,7 @@ protected function matchSimpleBinds(string $sql, array $binds, int $bindCount, i
do
{
$c --;
$escapedValue = $this->db->escape($binds[$c]);
$escapedValue = $binds[$c][1] ? $this->db->escape($binds[$c][0]) : $binds[$c[0]];
if (is_array($escapedValue))
{
$escapedValue = '(' . implode(',', $escapedValue) . ')';
Expand Down
2 changes: 1 addition & 1 deletion tests/_support/Database/MockConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function query(string $sql, $binds = null, $queryClass = 'CodeIgniter\\Da

$query = new $queryClass($this);

$query->setQuery($sql, $binds);
$query->setQuery($sql, $binds, $this->setEscapeFlags);

if (! empty($this->swapPre) && ! empty($this->DBPrefix))
{
Expand Down
7 changes: 6 additions & 1 deletion tests/system/Database/Builder/DeleteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ public function testDelete()
$answer = $builder->delete(['id' => 1], null, true, true);

$expectedSQL = 'DELETE FROM "jobs" WHERE "id" = :id:';
$expectedBinds = ['id' => 1];
$expectedBinds = [
'id' => [
1,
true,
],
];

$this->assertEquals($expectedSQL, str_replace("\n", ' ', $answer));
$this->assertEquals($expectedBinds, $builder->getBinds());
Expand Down
13 changes: 11 additions & 2 deletions tests/system/Database/Builder/InsertTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ public function testSimpleInsert()
$builder->insert($insertData, true, true);

$expectedSQL = 'INSERT INTO "jobs" ("id", "name") VALUES (1, \'Grocery Sales\')';
$expectedBinds = $insertData;
$expectedBinds = [
'id' => [
1,
true,
],
'name' => [
'Grocery Sales',
true,
],
];

$this->assertEquals($expectedSQL, str_replace("\n", ' ', $builder->getCompiledInsert()));
$this->assertEquals($expectedBinds, $builder->getBinds());
Expand Down Expand Up @@ -97,7 +106,7 @@ public function testInsertBatchThrowsExceptionOnNoData()

//--------------------------------------------------------------------

public function testInsertBatchThrowsExceptionOnEmptData()
public function testInsertBatchThrowsExceptionOnEmptyData()
{
$builder = $this->db->table('jobs');

Expand Down
Loading

0 comments on commit df38172

Please sign in to comment.