Skip to content

wpdb: Escape column identifiers in CRUD methods #15802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
salvatorecorsi wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

wpdb: Escape column identifiers in CRUD methods #15802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/wp-includes/class-wpdb.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2623,7 +2623,7 @@ public function _insert_replace_helper( $table, $data, $format = null, $type = '
}

$data = $this->process_fields( $table, $data, $format );
if ( false === $data ) {
if ( false === $data || array() === $data ) {
return false;
}

Expand All @@ -2639,8 +2639,9 @@ public function _insert_replace_helper( $table, $data, $format = null, $type = '
$values[] = $value['value'];
}

$fields = '`' . implode( '`, `', array_keys( $data ) ) . '`';
$fields = implode( ', ', array_fill( 0, count( $data ), '%i' ) );
$formats = implode( ', ', $formats );
$values = array_merge( array_keys( $data ), $values );

$sql = "$type INTO `$table` ($fields) VALUES ($formats)";

Expand Down Expand Up @@ -2727,21 +2728,23 @@ public function update( $table, $data, $where, $format = null, $where_format = n
$conditions = array();
$values = array();
foreach ( $data as $field => $value ) {
$values[] = $field;
if ( is_null( $value['value'] ) ) {
$fields[] = "`$field` = NULL";
$fields[] = '%i = NULL';
continue;
}

$fields[] = "`$field` = " . $value['format'];
$fields[] = '%i = ' . $value['format'];
$values[] = $value['value'];
}
foreach ( $where as $field => $value ) {
$values[] = $field;
if ( is_null( $value['value'] ) ) {
$conditions[] = "`$field` IS NULL";
$conditions[] = '%i IS NULL';
continue;
}

$conditions[] = "`$field` = " . $value['format'];
$conditions[] = '%i = ' . $value['format'];
$values[] = $value['value'];
}

Expand Down Expand Up @@ -2807,12 +2810,13 @@ public function delete( $table, $where, $where_format = null ) {
$conditions = array();
$values = array();
foreach ( $where as $field => $value ) {
$values[] = $field;
if ( is_null( $value['value'] ) ) {
$conditions[] = "`$field` IS NULL";
$conditions[] = '%i IS NULL';
continue;
}

$conditions[] = "`$field` = " . $value['format'];
$conditions[] = '%i = ' . $value['format'];
$values[] = $value['value'];
}

Expand Down
171 changes: 171 additions & 0 deletions tests/phpunit/tests/db.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,177 @@ public function test_replace() {
$this->assertSame( 'Walter Replace Sobchak', $row->display_name );
}

/**
* Tests that CRUD methods accept raw column names that require escaping.
*
* @ticket 60505
* @dataProvider data_crud_column_identifiers
*
* @param string $method CRUD method.
* @param string $column Column name.
*/
public function test_crud_methods_escape_column_identifiers( $method, $column ) {
global $wpdb;

$table = $this->create_table_with_column_identifier( $column );
$suppress = $wpdb->suppress_errors( true );

try {
if ( in_array( $method, array( 'update', 'delete' ), true ) ) {
$wpdb->query( $wpdb->prepare( 'INSERT INTO %i (%i, ordinary) VALUES (%s, %d)', $table, $column, 'original', 17 ) );
}

$data = array(
$column => 'changed',
'ordinary' => 23,
'fraction' => 0.25,
'nullable' => null,
);
$where = array(
$column => 'original',
'ordinary' => 17,
);
$format = array( '%s', '%d', '%f', '%s' );

if ( 'update' === $method ) {
$result = $wpdb->update( $table, $data, $where, $format, array( '%s', '%d' ) );
} elseif ( 'delete' === $method ) {
$result = $wpdb->delete( $table, $where, array( '%s', '%d' ) );
} else {
$result = $wpdb->$method( $table, $data, $format );
}

$this->assertSame( 1, $result, 'The CRUD operation should affect one row.' );
$this->assertEmpty( $wpdb->last_error, 'The raw column name should not cause a database error.' );

if ( 'delete' === $method ) {
$this->assertSame( '0', $wpdb->get_var( $wpdb->prepare( 'SELECT COUNT(*) FROM %i', $table ) ) );
$this->assertSame( 0, $wpdb->delete( $table, $where, array( '%s', '%d' ) ), 'Deleting a missing row should still return zero.' );
} else {
$row = $wpdb->get_row( $wpdb->prepare( 'SELECT * FROM %i', $table ), ARRAY_A );
$this->assertSame( 'changed', $row[ $column ] );
$this->assertSame( '23', $row['ordinary'], 'Value formats should retain their order.' );
$this->assertSame( '0.25', $row['fraction'] );
$this->assertNull( $row['nullable'], 'A null value should ignore its format.' );

if ( 'update' === $method ) {
$this->assertSame( 0, $wpdb->update( $table, $data, array( $column => 'changed' ), $format ), 'An unchanged row should still return zero.' );
}
}
} finally {
$wpdb->query( $wpdb->prepare( 'DROP TEMPORARY TABLE IF EXISTS %i', $table ) );
$wpdb->suppress_errors( $suppress );
}
}

/**
* Tests escaping identifiers when null data and conditions omit value placeholders.
*
* @ticket 60505
* @dataProvider data_crud_null_column_identifiers
*
* @param string $method CRUD method.
* @param string $column Column name.
*/
public function test_crud_methods_escape_null_column_identifiers( $method, $column ) {
global $wpdb;

$table = $this->create_table_with_column_identifier( $column );
$suppress = $wpdb->suppress_errors( true );

try {
$wpdb->query( $wpdb->prepare( 'INSERT INTO %i (%i, ordinary) VALUES (NULL, %d)', $table, $column, 17 ) );

$where = array(
$column => null,
'ordinary' => 17,
);
if ( 'update' === $method ) {
$data = array(
$column => null,
'ordinary' => 23,
);
$result = $wpdb->update( $table, $data, $where, array( '%s', '%d' ), array( '%s', '%d' ) );
} else {
$result = $wpdb->delete( $table, $where, array( '%s', '%d' ) );
}

$this->assertSame( 1, $result, 'Null values should not displace identifier or value arguments.' );
$this->assertEmpty( $wpdb->last_error );

if ( 'update' === $method ) {
$row = $wpdb->get_row( $wpdb->prepare( 'SELECT * FROM %i', $table ), ARRAY_A );
$this->assertNull( $row[ $column ] );
$this->assertSame( '23', $row['ordinary'] );
} else {
$this->assertSame( '0', $wpdb->get_var( $wpdb->prepare( 'SELECT COUNT(*) FROM %i', $table ) ) );
}
} finally {
$wpdb->query( $wpdb->prepare( 'DROP TEMPORARY TABLE IF EXISTS %i', $table ) );
$wpdb->suppress_errors( $suppress );
}
}

/**
* Creates a temporary table with a raw column identifier.
*
* @param string $column Column name.
* @return string Table name.
*/
private function create_table_with_column_identifier( $column ) {
global $wpdb;

$table = $wpdb->prefix . 'test_column_identifier_' . md5( $column );
$this->assertNotFalse(
$wpdb->query( $wpdb->prepare( 'CREATE TEMPORARY TABLE %i (id bigint NOT NULL AUTO_INCREMENT PRIMARY KEY, %i varchar(100) NULL, ordinary int, fraction double, nullable varchar(100) NULL)', $table, $column ) ),
'The test table should accept the column identifier.'
);

return $table;
}

/**
* Provides CRUD methods and raw column identifiers.
*
* @return array[]
*/
public static function data_crud_column_identifiers() {
$data = array();
foreach ( array( 'insert', 'replace', 'update', 'delete' ) as $method ) {
foreach ( array( 'plain', 'back`tick', 'percent%s' ) as $column ) {
$data[ $method . ': ' . $column ] = array( $method, $column );
}
}

return $data;
}

/**
* Provides CRUD methods with null data and conditions.
*
* @return array[]
*/
public static function data_crud_null_column_identifiers() {
return array_filter(
self::data_crud_column_identifiers(),
static function ( $test_case ) {
return in_array( $test_case[0], array( 'update', 'delete' ), true );
}
);
}

/**
* Tests that an empty column list does not turn into a default-row insert.
*
* @ticket 60505
*/
public function test_insert_and_replace_reject_empty_column_lists() {
global $wpdb;

$this->assertFalse( $wpdb->insert( $wpdb->users, array() ) );
$this->assertFalse( $wpdb->replace( $wpdb->users, array() ) );
}

/**
* wpdb::update() requires a WHERE condition.
*
Expand Down
Loading