mirror of
https://github.com/gosticks/wordpress-develop.git
synced 2026-06-28 14:20:15 +00:00
Database: Add %i placeholder support to $wpdb->prepare to escape table and column names, take 2.
[53575] during the 6.1 cycle was reverted in [54734] to address issues around multiple `%` placeholders not being properly quoted as reported in #56933. Since then, this issue has been resolved and the underlying code improved significantly. Additionally, the unit tests have been expanded and the inline docs have been improved as well. This change reintroduces `%i` placeholder support in `$wpdb->prepare()` to give extenders the ability to safely escape table and column names in database queries. Follow-up to [53575] and [54734]. Props craigfrancis, jrf, xknown, costdev, ironprogrammer, SergeyBiryukov. Fixes #52506. git-svn-id: https://develop.svn.wordpress.org/trunk@55151 602fd350-edb4-49c9-b593-d223f7449a82
This commit is contained in:
David Baumwald
@@ -494,9 +494,11 @@ class Tests_DB extends WP_UnitTestCase {
|
||||
$this->assertTrue( $wpdb->has_cap( 'collation' ) );
|
||||
$this->assertTrue( $wpdb->has_cap( 'group_concat' ) );
|
||||
$this->assertTrue( $wpdb->has_cap( 'subqueries' ) );
|
||||
$this->assertTrue( $wpdb->has_cap( 'identifier_placeholders' ) );
|
||||
$this->assertTrue( $wpdb->has_cap( 'COLLATION' ) );
|
||||
$this->assertTrue( $wpdb->has_cap( 'GROUP_CONCAT' ) );
|
||||
$this->assertTrue( $wpdb->has_cap( 'SUBQUERIES' ) );
|
||||
$this->assertTrue( $wpdb->has_cap( 'IDENTIFIER_PLACEHOLDERS' ) );
|
||||
$this->assertSame(
|
||||
version_compare( $wpdb->db_version(), '5.0.7', '>=' ),
|
||||
$wpdb->has_cap( 'set_charset' )
|
||||
@@ -1515,7 +1517,7 @@ class Tests_DB extends WP_UnitTestCase {
|
||||
public function test_prepare_with_placeholders_and_individual_args( $sql, $values, $incorrect_usage, $expected ) {
|
||||
global $wpdb;
|
||||
|
||||
if ( $incorrect_usage ) {
|
||||
if ( is_string( $incorrect_usage ) || true === $incorrect_usage ) {
|
||||
$this->setExpectedIncorrectUsage( 'wpdb::prepare' );
|
||||
}
|
||||
|
||||
@@ -1525,7 +1527,11 @@ class Tests_DB extends WP_UnitTestCase {
|
||||
|
||||
// phpcs:ignore WordPress.DB.PreparedSQL
|
||||
$sql = $wpdb->prepare( $sql, ...$values );
|
||||
$this->assertSame( $expected, $sql );
|
||||
$this->assertSame( $expected, $sql, 'The expected SQL does not match' );
|
||||
|
||||
if ( is_string( $incorrect_usage ) && array_key_exists( 'wpdb::prepare', $this->caught_doing_it_wrong ) ) {
|
||||
$this->assertStringContainsString( $incorrect_usage, $this->caught_doing_it_wrong['wpdb::prepare'], 'The "_doing_it_wrong" message does not match' );
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1534,7 +1540,7 @@ class Tests_DB extends WP_UnitTestCase {
|
||||
public function test_prepare_with_placeholders_and_array_args( $sql, $values, $incorrect_usage, $expected ) {
|
||||
global $wpdb;
|
||||
|
||||
if ( $incorrect_usage ) {
|
||||
if ( is_string( $incorrect_usage ) || true === $incorrect_usage ) {
|
||||
$this->setExpectedIncorrectUsage( 'wpdb::prepare' );
|
||||
}
|
||||
|
||||
@@ -1544,7 +1550,11 @@ class Tests_DB extends WP_UnitTestCase {
|
||||
|
||||
// phpcs:ignore WordPress.DB.PreparedSQL
|
||||
$sql = $wpdb->prepare( $sql, $values );
|
||||
$this->assertSame( $expected, $sql );
|
||||
$this->assertSame( $expected, $sql, 'The expected SQL does not match' );
|
||||
|
||||
if ( is_string( $incorrect_usage ) && array_key_exists( 'wpdb::prepare', $this->caught_doing_it_wrong ) ) {
|
||||
$this->assertStringContainsString( $incorrect_usage, $this->caught_doing_it_wrong['wpdb::prepare'], 'The "_doing_it_wrong" message does not match' );
|
||||
}
|
||||
}
|
||||
|
||||
public function data_prepare_with_placeholders() {
|
||||
@@ -1703,6 +1713,42 @@ class Tests_DB extends WP_UnitTestCase {
|
||||
true,
|
||||
"'{$placeholder_escape}'{$placeholder_escape}s",
|
||||
),
|
||||
|
||||
/*
|
||||
* @ticket 56933.
|
||||
* When preparing a '%%%s%%', test that the inserted value
|
||||
* is not wrapped in single quotes between the 2 "%".
|
||||
*/
|
||||
array(
|
||||
'%%s %d',
|
||||
1,
|
||||
false,
|
||||
"{$placeholder_escape}s 1",
|
||||
),
|
||||
array(
|
||||
'%%%s',
|
||||
'hello',
|
||||
false,
|
||||
"{$placeholder_escape}hello",
|
||||
),
|
||||
array(
|
||||
'%%%%s',
|
||||
'hello',
|
||||
false,
|
||||
"{$placeholder_escape}{$placeholder_escape}s",
|
||||
),
|
||||
array(
|
||||
'%%%%%s',
|
||||
'hello',
|
||||
false,
|
||||
"{$placeholder_escape}{$placeholder_escape}hello",
|
||||
),
|
||||
array(
|
||||
'%%%s%%',
|
||||
'hello',
|
||||
false,
|
||||
"{$placeholder_escape}hello{$placeholder_escape}",
|
||||
),
|
||||
array(
|
||||
"'%'%%s%s",
|
||||
'hello',
|
||||
@@ -1715,23 +1761,359 @@ class Tests_DB extends WP_UnitTestCase {
|
||||
false,
|
||||
"'{$placeholder_escape}'{$placeholder_escape}s 'hello'",
|
||||
),
|
||||
/*
|
||||
* @ticket 56933.
|
||||
* When preparing a '%%%s%%', test that the inserted value
|
||||
* is not wrapped in single quotes between the 2 hex values.
|
||||
*/
|
||||
array(
|
||||
'%%%s%%',
|
||||
'hello',
|
||||
false,
|
||||
"{$placeholder_escape}hello{$placeholder_escape}",
|
||||
),
|
||||
array(
|
||||
"'%-'#5s' '%'#-+-5s'",
|
||||
array( 'hello', 'foo' ),
|
||||
false,
|
||||
"'hello' 'foo##'",
|
||||
),
|
||||
|
||||
/*
|
||||
* Before WP 6.2 the "force floats to be locale-unaware" RegEx didn't
|
||||
* convert "%%%f" to "%%%F" (note the uppercase F).
|
||||
* This was because it didn't check to see if the leading "%" was escaped.
|
||||
* And because the "Escape any unescaped percents" RegEx used "[sdF]" in its
|
||||
* negative lookahead assertion, when there was an odd number of "%", it added
|
||||
* an extra "%", to give the fully escaped "%%%%f" (not a placeholder).
|
||||
*/
|
||||
array(
|
||||
'%f OR id = %d',
|
||||
array( 3, 5 ),
|
||||
false,
|
||||
'3.000000 OR id = 5',
|
||||
),
|
||||
array(
|
||||
'%%f OR id = %d',
|
||||
array( 5 ),
|
||||
false,
|
||||
"{$placeholder_escape}f OR id = 5",
|
||||
),
|
||||
array(
|
||||
'%%%f OR id = %d',
|
||||
array( 5 ),
|
||||
false,
|
||||
"{$placeholder_escape}{$placeholder_escape}f OR id = 5",
|
||||
),
|
||||
array(
|
||||
'%%%%f OR id = %d',
|
||||
array( 5 ),
|
||||
false,
|
||||
"{$placeholder_escape}{$placeholder_escape}f OR id = 5",
|
||||
),
|
||||
array(
|
||||
"WHERE id = %d AND content LIKE '%.4f'",
|
||||
array( 1, 2 ),
|
||||
false,
|
||||
"WHERE id = 1 AND content LIKE '2.0000'",
|
||||
),
|
||||
array(
|
||||
"WHERE id = %d AND content LIKE '%%.4f'",
|
||||
array( 1 ),
|
||||
false,
|
||||
"WHERE id = 1 AND content LIKE '{$placeholder_escape}.4f'",
|
||||
),
|
||||
array(
|
||||
"WHERE id = %d AND content LIKE '%%%.4f'",
|
||||
array( 1 ),
|
||||
false,
|
||||
"WHERE id = 1 AND content LIKE '{$placeholder_escape}{$placeholder_escape}.4f'",
|
||||
),
|
||||
array(
|
||||
"WHERE id = %d AND content LIKE '%%%%.4f'",
|
||||
array( 1 ),
|
||||
false,
|
||||
"WHERE id = 1 AND content LIKE '{$placeholder_escape}{$placeholder_escape}.4f'",
|
||||
),
|
||||
array(
|
||||
"WHERE id = %d AND content LIKE '%%%%%.4f'",
|
||||
array( 1 ),
|
||||
false,
|
||||
"WHERE id = 1 AND content LIKE '{$placeholder_escape}{$placeholder_escape}{$placeholder_escape}.4f'",
|
||||
),
|
||||
array(
|
||||
'%.4f',
|
||||
array( 1 ),
|
||||
false,
|
||||
'1.0000',
|
||||
),
|
||||
array(
|
||||
'%.4f OR id = %d',
|
||||
array( 1, 5 ),
|
||||
false,
|
||||
'1.0000 OR id = 5',
|
||||
),
|
||||
array(
|
||||
'%%.4f OR id = %d',
|
||||
array( 5 ),
|
||||
false,
|
||||
"{$placeholder_escape}.4f OR id = 5",
|
||||
),
|
||||
array(
|
||||
'%%%.4f OR id = %d',
|
||||
array( 5 ),
|
||||
false,
|
||||
"{$placeholder_escape}{$placeholder_escape}.4f OR id = 5",
|
||||
),
|
||||
array(
|
||||
'%%%%.4f OR id = %d',
|
||||
array( 5 ),
|
||||
false,
|
||||
"{$placeholder_escape}{$placeholder_escape}.4f OR id = 5",
|
||||
),
|
||||
array(
|
||||
'%%%%%.4f OR id = %d',
|
||||
array( 5 ),
|
||||
false,
|
||||
"{$placeholder_escape}{$placeholder_escape}{$placeholder_escape}.4f OR id = 5",
|
||||
),
|
||||
|
||||
/*
|
||||
* @ticket 52506.
|
||||
* Adding an escape method for Identifiers (e.g. table/field names).
|
||||
*/
|
||||
array(
|
||||
'SELECT * FROM %i WHERE %i = %d;',
|
||||
array( 'my_table', 'my_field', 321 ),
|
||||
false,
|
||||
'SELECT * FROM `my_table` WHERE `my_field` = 321;',
|
||||
),
|
||||
array(
|
||||
'WHERE %i = %d;',
|
||||
array( 'evil_`_field', 321 ),
|
||||
false,
|
||||
'WHERE `evil_``_field` = 321;', // To quote the identifier itself, then you need to double the character, e.g. `a``b`.
|
||||
),
|
||||
array(
|
||||
'WHERE %i = %d;',
|
||||
array( 'evil_````````_field', 321 ),
|
||||
false,
|
||||
'WHERE `evil_````````````````_field` = 321;',
|
||||
),
|
||||
array(
|
||||
'WHERE %i = %d;',
|
||||
array( '``evil_field``', 321 ),
|
||||
false,
|
||||
'WHERE `````evil_field````` = 321;',
|
||||
),
|
||||
array(
|
||||
'WHERE %i = %d;',
|
||||
array( 'evil\'field', 321 ),
|
||||
false,
|
||||
'WHERE `evil\'field` = 321;',
|
||||
),
|
||||
array(
|
||||
'WHERE %i = %d;',
|
||||
array( 'evil_\``_field', 321 ),
|
||||
false,
|
||||
'WHERE `evil_\````_field` = 321;',
|
||||
),
|
||||
array(
|
||||
'WHERE %i = %d;',
|
||||
array( 'evil_%s_field', 321 ),
|
||||
false,
|
||||
"WHERE `evil_{$placeholder_escape}s_field` = 321;",
|
||||
),
|
||||
array(
|
||||
'WHERE %i = %d;',
|
||||
array( 'value`', 321 ),
|
||||
false,
|
||||
'WHERE `value``` = 321;',
|
||||
),
|
||||
array(
|
||||
'WHERE `%i = %d;',
|
||||
array( ' AND evil_value', 321 ),
|
||||
false,
|
||||
'WHERE `` AND evil_value` = 321;', // Won't run (SQL parse error: "Unclosed quote").
|
||||
),
|
||||
array(
|
||||
'WHERE %i` = %d;',
|
||||
array( 'evil_value -- ', 321 ),
|
||||
false,
|
||||
'WHERE `evil_value -- `` = 321;', // Won't run (SQL parse error: "Unclosed quote").
|
||||
),
|
||||
array(
|
||||
'WHERE `%i`` = %d;',
|
||||
array( ' AND true -- ', 321 ),
|
||||
false,
|
||||
'WHERE `` AND true -- ``` = 321;', // Won't run (Unknown column '').
|
||||
),
|
||||
array(
|
||||
'WHERE ``%i` = %d;',
|
||||
array( ' AND true -- ', 321 ),
|
||||
false,
|
||||
'WHERE ``` AND true -- `` = 321;', // Won't run (SQL parse error: "Unclosed quote").
|
||||
),
|
||||
array(
|
||||
'WHERE %2$i = %1$d;',
|
||||
array( '1', 'two' ),
|
||||
false,
|
||||
'WHERE `two` = 1;',
|
||||
),
|
||||
array(
|
||||
'WHERE \'%i\' = 1 AND "%i" = 2 AND `%i` = 3 AND ``%i`` = 4 AND %15i = 5',
|
||||
array( 'my_field1', 'my_field2', 'my_field3', 'my_field4', 'my_field5' ),
|
||||
false,
|
||||
'WHERE \'`my_field1`\' = 1 AND "`my_field2`" = 2 AND ``my_field3`` = 3 AND ```my_field4``` = 4 AND ` my_field5` = 5', // Does not remove any existing quotes, always adds it's own (safer).
|
||||
),
|
||||
array(
|
||||
'WHERE id = %d AND %i LIKE %2$s LIMIT 1',
|
||||
array( 123, 'field -- ', false ),
|
||||
'Arguments cannot be prepared as both an Identifier and Value. Found the following conflicts: %i and %2$s',
|
||||
null, // Should be rejected, otherwise the `%1$s` could use Identifier escaping, e.g. 'WHERE `field -- ` LIKE field -- LIMIT 1' (thanks @vortfu).
|
||||
),
|
||||
array(
|
||||
'WHERE %i LIKE %s LIMIT 1',
|
||||
array( "field' -- ", "field' -- " ),
|
||||
false,
|
||||
"WHERE `field' -- ` LIKE 'field\' -- ' LIMIT 1", // In contrast to the above, Identifier vs String escaping is used.
|
||||
),
|
||||
array(
|
||||
'WHERE %2$i IN ( %s , %s ) LIMIT 1',
|
||||
array( 'a', 'b' ),
|
||||
'Arguments cannot be prepared as both an Identifier and Value. Found the following conflicts: %2$i and %s',
|
||||
null,
|
||||
),
|
||||
array(
|
||||
'WHERE %1$i = %1$s',
|
||||
array( 'a', 'b' ),
|
||||
'Arguments cannot be prepared as both an Identifier and Value. Found the following conflicts: %1$i and %1$s',
|
||||
null,
|
||||
),
|
||||
array(
|
||||
'WHERE %1$i = %1$s OR %2$i = %2$s',
|
||||
array( 'a', 'b' ),
|
||||
'Arguments cannot be prepared as both an Identifier and Value. Found the following conflicts: %1$i and %1$s, %2$i and %2$s',
|
||||
null,
|
||||
),
|
||||
array(
|
||||
'WHERE %1$i = %1$s OR %2$i = %1$s',
|
||||
array( 'a', 'b' ),
|
||||
'Arguments cannot be prepared as both an Identifier and Value. Found the following conflicts: %1$i and %1$s and %1$s',
|
||||
null,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The wpdb->allow_unsafe_unquoted_parameters is true (for now), purely for backwards compatibility reasons.
|
||||
*
|
||||
* @ticket 52506
|
||||
*
|
||||
* @dataProvider data_prepare_should_respect_the_allow_unsafe_unquoted_parameters_property
|
||||
*
|
||||
* @covers wpdb::prepare
|
||||
*
|
||||
* @param bool $allow Whether to allow unsafe unquoted parameters.
|
||||
* @param string $sql The SQL to prepare.
|
||||
* @param array $values The values for prepare.
|
||||
* @param string $expected The expected prepared parameters.
|
||||
*/
|
||||
public function test_prepare_should_respect_the_allow_unsafe_unquoted_parameters_property( $allow, $sql, $values, $expected ) {
|
||||
global $wpdb;
|
||||
|
||||
$default = $wpdb->allow_unsafe_unquoted_parameters;
|
||||
|
||||
$property = new ReflectionProperty( $wpdb, 'allow_unsafe_unquoted_parameters' );
|
||||
$property->setAccessible( true );
|
||||
$property->setValue( $wpdb, $allow );
|
||||
|
||||
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
|
||||
$actual = $wpdb->prepare( $sql, $values );
|
||||
|
||||
// Reset.
|
||||
$property->setValue( $wpdb, $default );
|
||||
$property->setAccessible( false );
|
||||
|
||||
$this->assertSame( $expected, $actual );
|
||||
}
|
||||
|
||||
/**
|
||||
* Data provider for test_prepare_should_respect_the_allow_unsafe_unquoted_parameters_property().
|
||||
*
|
||||
* @return array[]
|
||||
*/
|
||||
public function data_prepare_should_respect_the_allow_unsafe_unquoted_parameters_property() {
|
||||
global $wpdb;
|
||||
|
||||
$placeholder_escape = $wpdb->placeholder_escape();
|
||||
|
||||
return array(
|
||||
|
||||
'numbered-true-1' => array(
|
||||
'allow' => true,
|
||||
'sql' => 'WHERE (%i = %s) OR (%3$i = %4$s)',
|
||||
'values' => array( 'field_a', 'string_a', 'field_b', 'string_b' ),
|
||||
'expected' => 'WHERE (`field_a` = \'string_a\') OR (`field_b` = string_b)',
|
||||
),
|
||||
'numbered-false-1' => array(
|
||||
'allow' => false,
|
||||
'sql' => 'WHERE (%i = %s) OR (%3$i = %4$s)',
|
||||
'values' => array( 'field_a', 'string_a', 'field_b', 'string_b' ),
|
||||
'expected' => 'WHERE (`field_a` = \'string_a\') OR (`field_b` = \'string_b\')',
|
||||
),
|
||||
'numbered-true-2' => array(
|
||||
'allow' => true,
|
||||
'sql' => 'WHERE (%i = %s) OR (%3$i = %4$s)',
|
||||
'values' => array( 'field_a', 'string_a', 'field_b', '0 OR EvilSQL' ),
|
||||
'expected' => 'WHERE (`field_a` = \'string_a\') OR (`field_b` = 0 OR EvilSQL)',
|
||||
),
|
||||
'numbered-false-2' => array(
|
||||
'allow' => false,
|
||||
'sql' => 'WHERE (%i = %s) OR (%3$i = %4$s)',
|
||||
'values' => array( 'field_a', 'string_a', 'field_b', '0 OR EvilSQL' ),
|
||||
'expected' => 'WHERE (`field_a` = \'string_a\') OR (`field_b` = \'0 OR EvilSQL\')',
|
||||
),
|
||||
|
||||
'format-true-1' => array(
|
||||
'allow' => true,
|
||||
'sql' => 'WHERE (%10i = %10s)',
|
||||
'values' => array( 'field_a', 'string_a' ),
|
||||
'expected' => 'WHERE (` field_a` = string_a)',
|
||||
),
|
||||
'format-false-1' => array(
|
||||
'allow' => false,
|
||||
'sql' => 'WHERE (%10i = %10s)',
|
||||
'values' => array( 'field_a', 'string_a' ),
|
||||
'expected' => 'WHERE (` field_a` = \' string_a\')',
|
||||
),
|
||||
'format-true-2' => array(
|
||||
'allow' => true,
|
||||
'sql' => 'WHERE (%10i = %10s)',
|
||||
'values' => array( 'field_a', '0 OR EvilSQL' ),
|
||||
'expected' => 'WHERE (` field_a` = 0 OR EvilSQL)',
|
||||
),
|
||||
'format-false-2' => array(
|
||||
'allow' => false,
|
||||
'sql' => 'WHERE (%10i = %10s)',
|
||||
'values' => array( 'field_a', '0 OR EvilSQL' ),
|
||||
'expected' => 'WHERE (` field_a` = \'0 OR EvilSQL\')',
|
||||
),
|
||||
|
||||
'escaped-true-1' => array(
|
||||
'allow' => true,
|
||||
'sql' => 'SELECT 9%%%s',
|
||||
'values' => array( '7' ),
|
||||
'expected' => "SELECT 9{$placeholder_escape}7", // SELECT 9%7.
|
||||
),
|
||||
'escaped-false-1' => array(
|
||||
'allow' => false,
|
||||
'sql' => 'SELECT 9%%%s',
|
||||
'values' => array( '7' ),
|
||||
'expected' => "SELECT 9{$placeholder_escape}'7'", // SELECT 9%'7'.
|
||||
),
|
||||
'escaped-true-2' => array(
|
||||
'allow' => true,
|
||||
'sql' => 'SELECT 9%%%s',
|
||||
'values' => array( '7 OR EvilSQL' ),
|
||||
'expected' => "SELECT 9{$placeholder_escape}7 OR EvilSQL", // SELECT 9%7 OR EvilSQL.
|
||||
),
|
||||
'escaped-false-2' => array(
|
||||
'allow' => false,
|
||||
'sql' => 'SELECT 9%%%s',
|
||||
'values' => array( '7 OR EvilSQL' ),
|
||||
'expected' => "SELECT 9{$placeholder_escape}'7 OR EvilSQL'", // SELECT 9%'7 OR EvilSQL'.
|
||||
),
|
||||
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user