Skip to content

#21989 - Avoid sanitizing new option values twice #13535

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
skikken wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

#21989 - Avoid sanitizing new option values twice #13535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b460b38
fix(Options): avoid sanitizing new option values twice
skikken Sep 15, 2026
1553f5f
test(Options): cover single sanitization of new option values
skikken Sep 15, 2026
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
54 changes: 53 additions & 1 deletion src/wp-includes/option.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,44 @@ function wp_load_core_site_options( $network_id = null ) {
wp_prime_network_option_caches( $network_id, $core_options );
}

/**
* Marks or consumes the internal flag that tracks a pre-sanitized option value.
*
* When update_option() finds that an option does not exist yet, it sanitizes the value
* before delegating to add_option(). This function carries that fact across the two calls
* so the value is not passed through sanitize_option() a second time.
*
* The flag lives in a function-scoped static rather than in a global, so it cannot be read
* or written through `$GLOBALS`. Consuming the flag always clears it, so a value set for
* one delegation cannot survive into an unrelated add_option() call.
*
* This is an internal function and is not part of the public API.
*
* @since 7.2.0
* @access private
*
* @param bool|null $set Optional. True to mark the value passed to the next add_option()
* call as already sanitized. Default null, which consumes and clears
* the flag.
* @return bool The flag value in effect for this call. When `$set` is passed, this is the
* value now in effect; otherwise it is the previous value, which has just been
* cleared.
*/
function wp_sanitize_option_pre_sanitized( $set = null ) {
static $pre_sanitized = false;

if ( null !== $set ) {
$pre_sanitized = (bool) $set;

return $pre_sanitized;
}

$consumed = $pre_sanitized;
$pre_sanitized = false;

return $consumed;
}

/**
* Updates the value of an option that was already added.
*
Expand Down Expand Up @@ -926,6 +964,10 @@ 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 ) {
// The value has already been sanitized above. Signal that so
// add_option() does not sanitize it a second time.
// See https://core.trac.wordpress.org/ticket/21989.
wp_sanitize_option_pre_sanitized( true );
return add_option( $option, $value, '', $autoload );
}

Expand Down Expand Up @@ -1069,6 +1111,14 @@ function update_option( $option, $value, $autoload = null ) {
function add_option( $option, $value = '', $deprecated = '', $autoload = null ) {
global $wpdb;

/*
* Consume the flag set by update_option() when it delegates here, so a value that was
* already sanitized is not sanitized a second time. Consuming happens before any of the
* early returns below, so the flag cannot leak into a later add_option() call.
* See https://core.trac.wordpress.org/ticket/21989.
*/
$pre_sanitized = wp_sanitize_option_pre_sanitized();

if ( ! empty( $deprecated ) ) {
_deprecated_argument( __FUNCTION__, '2.3.0' );
}
Expand Down Expand Up @@ -1110,7 +1160,9 @@ function add_option( $option, $value = '', $deprecated = '', $autoload = null )
$value = clone $value;
}

$value = sanitize_option( $option, $value );
if ( ! $pre_sanitized ) {
$value = sanitize_option( $option, $value );
}

/*
* Make sure the option doesn't already exist.
Expand Down
182 changes: 182 additions & 0 deletions tests/phpunit/tests/option/updateOption.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,188 @@ public function test_update_option_array_with_object() {
$this->assertSame( $num_queries_pre_update, get_num_queries() );
}

/**
* @ticket 21989
*
* @covers ::update_option
* @covers ::add_filter
* @covers ::remove_filter
*/
public function test_update_option_sanitized_callback_runs_once_when_creating_new_option() {
$option_name = 'test_21989_non_idempotent_create';
$sanitize_count = 0;

$callback = function ( $value ) use ( &$sanitize_count ) {
++$sanitize_count;
return $value . '_y';
};

add_filter( "sanitize_option_{$option_name}", $callback );

// Ensure the option does not exist.
delete_option( $option_name );
$this->assertFalse( get_option( $option_name ) );

$result = update_option( $option_name, 'x' );
remove_filter( "sanitize_option_{$option_name}", $callback );

$this->assertTrue( $result );
$this->assertSame( 1, $sanitize_count, 'Sanitize callback must run exactly once.' );
$this->assertSame( 'x_y', get_option( $option_name ), 'Stored value must reflect a single sanitization pass.' );

// Clean up.
delete_option( $option_name );
}

/**
* @ticket 21989
*
* @covers ::add_option
* @covers ::add_filter
* @covers ::remove_filter
*/
public function test_add_option_sanitized_callback_runs_once_for_direct_call() {
$option_name = 'test_21989_direct_add';
$sanitize_count = 0;

$callback = function ( $value ) use ( &$sanitize_count ) {
++$sanitize_count;
return $value . '_y';
};

add_filter( "sanitize_option_{$option_name}", $callback );

// Ensure the option does not exist.
delete_option( $option_name );
$this->assertFalse( get_option( $option_name ) );

$result = add_option( $option_name, 'x' );
remove_filter( "sanitize_option_{$option_name}", $callback );

$this->assertTrue( $result );
$this->assertSame( 1, $sanitize_count, 'Direct add_option() must sanitize exactly once.' );
$this->assertSame( 'x_y', get_option( $option_name ) );

// Clean up.
delete_option( $option_name );
}

/**
* @ticket 21989
*
* @covers ::update_option
* @covers ::add_filter
* @covers ::remove_filter
*/
public function test_update_option_sanitized_callback_runs_once_for_existing_option() {
$option_name = 'test_21989_existing_update';
$sanitize_count = 0;

$callback = function ( $value ) use ( &$sanitize_count ) {
++$sanitize_count;
return $value . '_y';
};

add_filter( "sanitize_option_{$option_name}", $callback );

// Create the option first.
delete_option( $option_name );
add_option( $option_name, 'initial' );
$this->assertSame( 'initial', get_option( $option_name ) );

// Reset counter after the add_option() call.
$sanitize_count = 0;

$result = update_option( $option_name, 'x' );
remove_filter( "sanitize_option_{$option_name}", $callback );

$this->assertTrue( $result );
$this->assertSame( 1, $sanitize_count, 'Updating an existing option must sanitize exactly once.' );
$this->assertSame( 'x_y', get_option( $option_name ) );

// Clean up.
delete_option( $option_name );
}

/**
* @ticket 21989
*
* @covers ::update_option
* @covers ::add_filter
* @covers ::remove_filter
*/
public function test_pre_update_option_filter_fires_on_new_option_path() {
$option_name = 'test_21989_pre_update_fires';
$filter_fired = false;
$captured_value = null;
$captured_old_value = null;

$callback = function ( $value, $old_value, $option ) use ( &$filter_fired, &$captured_value, &$captured_old_value, $option_name ) {
$filter_fired = true;
$captured_value = $value;
$captured_old_value = $old_value;
$this->assertSame( $option_name, $option );
return $value;
};

add_filter( "pre_update_option_{$option_name}", $callback, 10, 3 );

// Ensure the option does not exist.
delete_option( $option_name );
$this->assertFalse( get_option( $option_name ) );

$result = update_option( $option_name, 'bar' );
remove_filter( "pre_update_option_{$option_name}", $callback, 10, 3 );

$this->assertTrue( $result );
$this->assertTrue( $filter_fired, 'pre_update_option_{$option} must fire on the new-option path.' );
$this->assertSame( 'bar', $captured_value );
$this->assertFalse( $captured_old_value, 'Old value must be false for a non-existent option.' );

// Clean up.
delete_option( $option_name );
}

/**
* @ticket 21989
*
* @covers ::add_option
* @covers ::update_option
* @covers ::add_filter
* @covers ::remove_filter
*/
public function test_sanitize_flag_does_not_leak_to_subsequent_add_option() {
$updated_option = 'test_21989_leak_source';
$added_option = 'test_21989_leak_target';
$sanitize_count = 0;

$callback = function ( $value ) use ( &$sanitize_count ) {
++$sanitize_count;
return $value . '_y';
};

add_filter( "sanitize_option_{$added_option}", $callback );

// Make sure neither option exists before the calls under test.
delete_option( $updated_option );
delete_option( $added_option );

$this->assertTrue( update_option( $updated_option, 'x' ) );

// The flag must have been consumed by that update_option() call, so this
// direct add_option() of an unrelated option still sanitizes exactly once.
$this->assertTrue( add_option( $added_option, 'x' ) );

remove_filter( "sanitize_option_{$added_option}", $callback );

$this->assertSame( 1, $sanitize_count, 'Sanitize callback must run exactly once for the later add_option() call.' );
$this->assertSame( 'x_y', get_option( $added_option ) );

// Clean up.
delete_option( $updated_option );
delete_option( $added_option );
}

/**
* `add_filter()` callback for test_should_respect_default_option_filter_when_option_does_not_yet_exist_in_database().
*/
Expand Down
Loading