From 004ebc67b5ab6af85da5ca890756ed91acb98021 Mon Sep 17 00:00:00 2001 From: Roy Orbitson Date: Thu, 22 Feb 2024 12:59:48 +1030 Subject: [PATCH 1/2] Prevent double-processing new option values --- src/wp-includes/option.php | 39 ++++++++++++++++++--- tests/phpunit/tests/option/updateOption.php | 38 ++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php index 58217ce3177a4..8086e4b7530ff 100644 --- a/src/wp-includes/option.php +++ b/src/wp-includes/option.php @@ -927,7 +927,7 @@ function update_option( $option, $value, $autoload = null ) { /** This filter is documented in wp-includes/option.php */ if ( apply_filters( "default_option_{$option}", false, $option, false ) === $old_value ) { - return add_option( $option, $value, '', $autoload ); + return _add_option( $option, $value, $autoload ); } $serialized_value = maybe_serialize( $value ); @@ -1049,8 +1049,6 @@ function update_option( $option, $value, $autoload = null ) { * @since 6.6.0 The $autoload parameter's default value was changed to null. * @since 6.7.0 The autoload values 'yes' and 'no' are deprecated. * - * @global wpdb $wpdb WordPress database abstraction object. - * * @param string $option Name of the option to add. Expected to not be SQL-escaped. * @param mixed $value Optional. Option value. Must be serializable if non-scalar. * Expected to not be SQL-escaped. @@ -1068,7 +1066,6 @@ function update_option( $option, $value, $autoload = null ) { * @return bool True if the option was added, false otherwise. */ function add_option( $option, $value = '', $deprecated = '', $autoload = null ) { - global $wpdb; if ( ! empty( $deprecated ) ) { _deprecated_argument( __FUNCTION__, '2.3.0' ); @@ -1113,6 +1110,40 @@ function add_option( $option, $value = '', $deprecated = '', $autoload = null ) $value = sanitize_option( $option, $value ); + return _add_option( $option, $value, $autoload ); +} + +/** + * Adds a new option. + * + * Warning: This is an internal function solely to prevent double-processing the + * value when update_option() detects the option does not yet exist. You should + * use add_option() instead. Checks to ensure you aren't adding a protected + * WordPress option should already have been performed. Do not use those which + * are protected. The value is expected to be filtered and sanitized. + * + * @since X.X.X + * @access private + * + * @global wpdb $wpdb WordPress database abstraction object. + * + * @param string $option Name of the option to add. Expected to not be SQL-escaped. + * @param mixed $value Optional. Option value. Must be serializable if non-scalar. + * Expected to not be SQL-escaped. + * @param bool|null $autoload Optional. Whether to load the option when WordPress starts up. + * Accepts a boolean, or `null` to leave the decision up to default heuristics in WordPress. + * For backward compatibility 'yes' and 'no' are also accepted. + * Autoloading too many options can lead to performance problems, especially if the + * options are not frequently used. For options which are accessed across several places + * in the frontend, it is recommended to autoload them, by using 'yes'|true. + * For options which are accessed only on few specific URLs, it is recommended + * to not autoload them, by using false. + * Default is null, which means WordPress will determine the autoload value. + * @return bool True if the option was added, false otherwise. + */ +function _add_option( $option, $value = '', $autoload = null ) { + global $wpdb; + /* * Make sure the option doesn't already exist. * We can check the 'notoptions' cache before we ask for a DB query. diff --git a/tests/phpunit/tests/option/updateOption.php b/tests/phpunit/tests/option/updateOption.php index c33f91ef73ebb..6acc97eba658f 100644 --- a/tests/phpunit/tests/option/updateOption.php +++ b/tests/phpunit/tests/option/updateOption.php @@ -219,10 +219,48 @@ public function test_update_option_array_with_object() { $this->assertSame( $num_queries_pre_update, get_num_queries() ); } + /** + * @ticket 21989 + * + * @covers ::add_option + * @covers ::add_filter + * @covers ::update_option + * @covers ::remove_filter + * @covers ::get_option + */ + public function test_stored_sanitized_value_from_update_of_nonexistent_option_should_be_same_as_that_from_add_option() { + $before = 'x'; + $sanitized = $this->__append_y( $before ); + + // Add the comparison option, it did not exist before this. + add_filter( 'sanitize_option_doesnotexist_filtered_add', array( $this, '__append_y' ) ); + add_option( 'doesnotexist_filtered_add', $before ); + remove_filter( 'sanitize_option_doesnotexist_filtered_add', array( $this, '__append_y' ) ); + + // Add the option, it did not exist before this. + add_filter( 'sanitize_option_doesnotexist_filtered_update', array( $this, '__append_y' ) ); + $added = update_option( 'doesnotexist_filtered_update', $before ); + remove_filter( 'sanitize_option_doesnotexist_filtered_update', array( $this, '__append_y' ) ); + + $after = get_option( 'doesnotexist_filtered_update' ); + + // Check all values match. + $this->assertTrue( $added ); + $this->assertSame( get_option( 'doesnotexist_filtered_add' ), $after ); + $this->assertSame( $sanitized, $after ); + } + /** * `add_filter()` callback for test_should_respect_default_option_filter_when_option_does_not_yet_exist_in_database(). */ public function __return_foo() { return 'foo'; } + + /** + * `add_filter()` callback for test_stored_sanitized_value_from_update_of_nonexistent_option_should_be_same_as_that_from_add_option(). + */ + public function __append_y( $value ) { + return $value . '_y'; + } } From 866de40e12fa5d8bd83a3f248ad0c326f58d2705 Mon Sep 17 00:00:00 2001 From: Roy Orbitson Date: Thu, 1 Oct 2026 12:26:12 +0930 Subject: [PATCH 2/2] Apply suggestion from @westonruter Co-authored-by: Weston Ruter --- src/wp-includes/option.php | 8 ++++---- tests/phpunit/tests/option/updateOption.php | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php index 69b4ef2fca1e0..58f8a6e203e0a 100644 --- a/src/wp-includes/option.php +++ b/src/wp-includes/option.php @@ -926,7 +926,7 @@ function update_option( $option, $value, $autoload = null ) { /** This filter is documented in wp-includes/option.php */ if ( apply_filters( "default_option_{$option}", false, $option, false ) === $old_value ) { - return _add_option( $option, $value, $autoload ); + return _wp_add_option( $option, $value, $autoload ); } $serialized_value = maybe_serialize( $value ); @@ -1065,7 +1065,6 @@ function update_option( $option, $value, $autoload = null ) { * @return bool True if the option was added, false otherwise. */ function add_option( $option, $value = '', $deprecated = '', $autoload = null ) { - if ( ! empty( $deprecated ) ) { _deprecated_argument( __FUNCTION__, '2.3.0' ); } @@ -1109,7 +1108,7 @@ function add_option( $option, $value = '', $deprecated = '', $autoload = null ) $value = sanitize_option( $option, $value ); - return _add_option( $option, $value, $autoload ); + return _wp_add_option( $option, $value, $autoload ); } /** @@ -1122,6 +1121,7 @@ function add_option( $option, $value = '', $deprecated = '', $autoload = null ) * are protected. The value is expected to be filtered and sanitized. * * @since X.X.X + * @internal * @access private * * @global wpdb $wpdb WordPress database abstraction object. @@ -1140,7 +1140,7 @@ function add_option( $option, $value = '', $deprecated = '', $autoload = null ) * Default is null, which means WordPress will determine the autoload value. * @return bool True if the option was added, false otherwise. */ -function _add_option( $option, $value = '', $autoload = null ) { +function _wp_add_option( $option, $value, $autoload ) { global $wpdb; /* diff --git a/tests/phpunit/tests/option/updateOption.php b/tests/phpunit/tests/option/updateOption.php index 6acc97eba658f..2864730f79896 100644 --- a/tests/phpunit/tests/option/updateOption.php +++ b/tests/phpunit/tests/option/updateOption.php @@ -229,18 +229,19 @@ public function test_update_option_array_with_object() { * @covers ::get_option */ public function test_stored_sanitized_value_from_update_of_nonexistent_option_should_be_same_as_that_from_add_option() { - $before = 'x'; - $sanitized = $this->__append_y( $before ); + $before = 'cats'; + $sanitized = $this->__sanitize_modify( $before ); + $sanitize_expected = 'cats and dogs'; // Add the comparison option, it did not exist before this. - add_filter( 'sanitize_option_doesnotexist_filtered_add', array( $this, '__append_y' ) ); + add_filter( 'sanitize_option_doesnotexist_filtered_add', array( $this, '__sanitize_modify' ) ); add_option( 'doesnotexist_filtered_add', $before ); - remove_filter( 'sanitize_option_doesnotexist_filtered_add', array( $this, '__append_y' ) ); + remove_filter( 'sanitize_option_doesnotexist_filtered_add', array( $this, '__sanitize_modify' ) ); // Add the option, it did not exist before this. - add_filter( 'sanitize_option_doesnotexist_filtered_update', array( $this, '__append_y' ) ); + add_filter( 'sanitize_option_doesnotexist_filtered_update', array( $this, '__sanitize_modify' ) ); $added = update_option( 'doesnotexist_filtered_update', $before ); - remove_filter( 'sanitize_option_doesnotexist_filtered_update', array( $this, '__append_y' ) ); + remove_filter( 'sanitize_option_doesnotexist_filtered_update', array( $this, '__sanitize_modify' ) ); $after = get_option( 'doesnotexist_filtered_update' ); @@ -248,6 +249,7 @@ public function test_stored_sanitized_value_from_update_of_nonexistent_option_sh $this->assertTrue( $added ); $this->assertSame( get_option( 'doesnotexist_filtered_add' ), $after ); $this->assertSame( $sanitized, $after ); + $this->assertSame( $sanitize_expected, $after ); } /** @@ -260,7 +262,7 @@ public function __return_foo() { /** * `add_filter()` callback for test_stored_sanitized_value_from_update_of_nonexistent_option_should_be_same_as_that_from_add_option(). */ - public function __append_y( $value ) { - return $value . '_y'; + public function __sanitize_modify( $value ) { + return $value . ' and dogs'; } }