Skip to content

Options, Meta APIs: Do not store a transient value without its timeout. #14340

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
dhrupo wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Options, Meta APIs: Do not store a transient value without its timeout. #14340

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
30 changes: 24 additions & 6 deletions src/wp-includes/option.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1501,6 +1501,8 @@ function get_transient( $transient ) {
*
* @since 2.8.0
*
* @global wpdb $wpdb WordPress database abstraction object.
*
* @param string $transient Transient name. Expected to not be SQL-escaped.
* Must be 172 characters or fewer in length.
* @param mixed $value Transient value. Must be serializable if non-scalar.
Expand All @@ -1509,6 +1511,7 @@ function get_transient( $transient ) {
* @return bool True if the value was set, false otherwise.
*/
function set_transient( $transient, $value, $expiration = 0 ) {
global $wpdb;

$expiration = (int) $expiration;

Expand Down Expand Up @@ -1549,11 +1552,18 @@ function set_transient( $transient, $value, $expiration = 0 ) {

if ( false === get_option( $transient_option ) ) {
$autoload = true;
$result = true;
if ( $expiration ) {
$autoload = false;
add_option( $transient_timeout, time() + $expiration, '', false );
// add_option() also fails when the timeout row already exists (orphaned or added concurrently);
// only a database error leaves the value without a timeout, so it would never expire.
$result = add_option( $transient_timeout, time() + $expiration, '', false )
|| '' === $wpdb->last_error
|| false !== get_option( $transient_timeout );
}
if ( $result ) {
$result = add_option( $transient_option, $value, '', $autoload );
}
$result = add_option( $transient_option, $value, '', $autoload );
} else {
/*
* If expiration is requested, but the transient has no timeout option,
Expand All @@ -1564,8 +1574,8 @@ function set_transient( $transient, $value, $expiration = 0 ) {
if ( $expiration ) {
if ( false === get_option( $transient_timeout ) ) {
delete_option( $transient_option );
add_option( $transient_timeout, time() + $expiration, '', false );
$result = add_option( $transient_option, $value, '', false );
$result = ( add_option( $transient_timeout, time() + $expiration, '', false ) || '' === $wpdb->last_error )
&& add_option( $transient_option, $value, '', false );
$update = false;
} else {
update_option( $transient_timeout, time() + $expiration );
Expand Down Expand Up @@ -2636,13 +2646,16 @@ function get_site_transient( $transient ) {
*
* @see set_transient()
*
* @global wpdb $wpdb WordPress database abstraction object.
*
* @param string $transient Transient name. Expected to not be SQL-escaped. Must be
* 167 characters or fewer in length.
* @param mixed $value Transient value. Expected to not be SQL-escaped.
* @param int $expiration Optional. Time until expiration in seconds. Default 0 (no expiration).
* @return bool True if the value was set, false otherwise.
*/
function set_site_transient( $transient, $value, $expiration = 0 ) {
global $wpdb;

/**
* Filters the value of a specific site transient before it is set.
Expand Down Expand Up @@ -2680,10 +2693,15 @@ function set_site_transient( $transient, $value, $expiration = 0 ) {
wp_prime_site_option_caches( array( $option, $transient_timeout ) );

if ( false === get_site_option( $option ) ) {
$result = true;
if ( $expiration ) {
add_site_option( $transient_timeout, time() + $expiration );
$result = add_site_option( $transient_timeout, time() + $expiration )
|| '' === $wpdb->last_error
|| false !== get_site_option( $transient_timeout );
}
if ( $result ) {
$result = add_site_option( $option, $value );
}
$result = add_site_option( $option, $value );
} else {
if ( $expiration ) {
update_site_option( $transient_timeout, time() + $expiration );
Expand Down
101 changes: 101 additions & 0 deletions tests/phpunit/tests/option/siteTransient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
*/
class Tests_Option_SiteTransient extends WP_UnitTestCase {

/**
* Previous `$wpdb->suppress_errors()` state, restored after a test that forces a database error.
*
* @var bool|null
*/
private $suppress_errors = null;

public function set_up() {
parent::set_up();

Expand All @@ -13,6 +20,15 @@ public function set_up() {
}
}

public function tear_down() {
if ( null !== $this->suppress_errors ) {
$GLOBALS['wpdb']->suppress_errors( $this->suppress_errors );
$this->suppress_errors = null;
}

parent::tear_down();
}

/**
* @covers ::get_site_transient
* @covers ::set_site_transient
Expand Down Expand Up @@ -158,4 +174,89 @@ public function test_site_transients_not_stored_in_options_table_on_ms() {

$this->assertNull( $option, 'Querying option table should not return transient on multisite.' );
}

/**
* Tests that a new site transient is not stored when its timeout cannot be saved.
*
* @ticket 52345
*
* @covers ::set_site_transient
*/
public function test_set_site_transient_should_not_store_value_when_timeout_cannot_be_added() {
$key = 'site_transient_timeout_fails';
$timeout = "_site_transient_timeout_{$key}";
$action = new MockAction();
add_action( "set_site_transient_{$key}", array( $action, 'action' ) );
$this->fail_insert_of_site_option( $timeout );

$this->assertFalse( set_site_transient( $key, 'value', 60 ), 'set_site_transient() should report the failure.' );
$this->assertFalse( get_site_transient( $key ), 'The site transient should not be readable without a timeout.' );
$this->assertFalse( get_site_option( "_site_transient_{$key}" ), 'The value should not be stored.' );
$this->assertSame( 0, $action->get_call_count(), 'The set_site_transient_{$transient} action should not fire.' );
}

/**
* Tests that a site transient can still be set when an orphaned timeout is left behind.
*
* @ticket 52345
*
* @covers ::set_site_transient
*/
public function test_set_site_transient_should_store_value_when_orphaned_timeout_exists() {
$key = 'site_transient_orphaned_timeout';
set_site_transient( $key, 'old', 60 );
delete_site_option( "_site_transient_{$key}" );

$this->assertTrue( set_site_transient( $key, 'new', 60 ), 'set_site_transient() should succeed.' );
$this->assertSame( 'new', get_site_transient( $key ), 'The new value should be returned.' );
}

/**
* Tests that a new site transient is stored when another request adds the same timeout first.
*
* @ticket 52345
*
* @covers ::set_site_transient
*/
public function test_set_site_transient_should_store_value_when_timeout_was_added_concurrently() {
global $wpdb;

$key = 'site_transient_timeout_race';
$timeout = "_site_transient_timeout_{$key}";
$filter = static function ( $query ) use ( $timeout, $wpdb, &$filter ) {
if ( 0 === stripos( ltrim( $query ), 'INSERT' ) && str_contains( $query, "'{$timeout}'" ) ) {
remove_filter( 'query', $filter );
$wpdb->query( $query ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Replays core's already prepared INSERT.
}
return $query;
};
add_filter( 'query', $filter );

$this->assertTrue( set_site_transient( $key, 'value', 60 ), 'set_site_transient() should succeed.' );
$this->assertSame( 'value', get_site_transient( $key ), 'The value should be returned.' );
}

/**
* Makes the INSERT query for the given site option fail with a database error.
*
* @param string $option_name Site option name whose INSERT should fail.
*/
private function fail_insert_of_site_option( $option_name ) {
global $wpdb;

$this->suppress_errors = $wpdb->suppress_errors();
add_filter(
'query',
static function ( $query ) use ( $option_name, $wpdb ) {
if ( 0 === stripos( ltrim( $query ), 'INSERT' ) && str_contains( $query, "'{$option_name}'" ) ) {
return str_replace(
array( "`{$wpdb->options}`", "`{$wpdb->sitemeta}`" ),
array( "`{$wpdb->options}_missing`", "`{$wpdb->sitemeta}_missing`" ),
$query
);
}
return $query;
}
);
}
}
152 changes: 152 additions & 0 deletions tests/phpunit/tests/option/transient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
*/
class Tests_Option_Transient extends WP_UnitTestCase {

/**
* Previous `$wpdb->suppress_errors()` state, restored after a test that forces a database error.
*
* @var bool|null
*/
private $suppress_errors = null;

public function set_up() {
parent::set_up();

Expand All @@ -13,6 +20,15 @@ public function set_up() {
}
}

public function tear_down() {
if ( null !== $this->suppress_errors ) {
$GLOBALS['wpdb']->suppress_errors( $this->suppress_errors );
$this->suppress_errors = null;
}

parent::tear_down();
}

/**
* @covers ::get_transient
* @covers ::set_transient
Expand Down Expand Up @@ -266,4 +282,140 @@ public function test_nonexistent_key_old_timeout() {
);
$this->assertSame( $expected, $a->get_events() );
}

/**
* Tests that a new transient is not stored when its timeout cannot be saved.
*
* @ticket 52345
*
* @covers ::set_transient
*/
public function test_set_transient_should_not_store_value_when_timeout_cannot_be_added() {
$key = 'transient_timeout_fails';
$action = new MockAction();
add_action( "set_transient_{$key}", array( $action, 'action' ) );
$this->fail_insert_of_option( "_transient_timeout_{$key}" );

$this->assertFalse( set_transient( $key, 'value', 60 ), 'set_transient() should report the failure.' );
$this->assertFalse( get_transient( $key ), 'The transient should not be readable without a timeout.' );
$this->assertFalse( get_option( "_transient_{$key}" ), 'The value row should not be stored.' );
$this->assertSame( 0, $action->get_call_count(), 'The set_transient_{$transient} action should not fire.' );
}

/**
* Tests that adding an expiry to a transient without a timeout does not leave a non-expiring value.
*
* @ticket 52345
*
* @covers ::set_transient
*/
public function test_set_transient_should_not_store_value_when_timeout_cannot_be_added_to_existing_transient() {
$key = 'transient_timeout_fails_existing';
set_transient( $key, 'old' );
$this->fail_insert_of_option( "_transient_timeout_{$key}" );

$this->assertFalse( set_transient( $key, 'new', 60 ), 'set_transient() should report the failure.' );
$this->assertFalse( get_transient( $key ), 'The transient should not be readable without a timeout.' );
$this->assertFalse( get_option( "_transient_{$key}" ), 'The value row should not be stored.' );
}

/**
* Tests that setting the same transient twice with the same expiry keeps the latest value.
*
* @ticket 52345
*
* @covers ::set_transient
*/
public function test_set_transient_should_keep_latest_value_when_set_twice_with_same_expiry() {
$key = 'transient_set_twice';

$this->assertTrue( set_transient( $key, 'first', 60 ), 'The first value should be stored.' );
$this->assertTrue( set_transient( $key, 'second', 60 ), 'The second value should be stored.' );
$this->assertSame( 'second', get_transient( $key ), 'The latest value should be returned.' );
}

/**
* Tests that a transient can still be set when an orphaned timeout row is left behind.
*
* @ticket 52345
*
* @covers ::set_transient
*/
public function test_set_transient_should_store_value_when_orphaned_timeout_exists() {
$key = 'transient_orphaned_timeout';
set_transient( $key, 'old', 60 );
delete_option( "_transient_{$key}" );

$this->assertTrue( set_transient( $key, 'new', 60 ), 'set_transient() should succeed.' );
$this->assertSame( 'new', get_transient( $key ), 'The new value should be returned.' );
}

/**
* Tests that a new transient is stored when another request adds the same timeout row first.
*
* @ticket 52345
*
* @covers ::set_transient
*/
public function test_set_transient_should_store_value_when_timeout_was_added_concurrently() {
$key = 'transient_timeout_race';
$this->insert_option_concurrently( "_transient_timeout_{$key}" );

$this->assertTrue( set_transient( $key, 'value', 60 ), 'set_transient() should succeed.' );
$this->assertSame( 'value', get_transient( $key ), 'The value should be returned.' );
}

/**
* Tests that adding an expiry to an existing transient succeeds when another request adds the same timeout first.
*
* @ticket 52345
*
* @covers ::set_transient
*/
public function test_set_transient_should_store_value_when_timeout_was_added_concurrently_to_existing_transient() {
$key = 'transient_timeout_race_existing';
set_transient( $key, 'old' );
$this->insert_option_concurrently( "_transient_timeout_{$key}" );

$this->assertTrue( set_transient( $key, 'new', 60 ), 'set_transient() should succeed.' );
$this->assertSame( 'new', get_transient( $key ), 'The new value should be returned.' );
}

/**
* Makes the INSERT query for the given option fail with a database error.
*
* @param string $option_name Option name whose INSERT should fail.
*/
private function fail_insert_of_option( $option_name ) {
global $wpdb;

$this->suppress_errors = $wpdb->suppress_errors();
add_filter(
'query',
static function ( $query ) use ( $option_name, $wpdb ) {
if ( 0 === stripos( ltrim( $query ), 'INSERT' ) && str_contains( $query, "'{$option_name}'" ) ) {
return str_replace( "`{$wpdb->options}`", "`{$wpdb->options}_missing`", $query );
}
return $query;
}
);
}

/**
* Runs the INSERT query for the given option once before WordPress does, as a parallel request would.
*
* @param string $option_name Option name whose row is inserted concurrently.
*/
private function insert_option_concurrently( $option_name ) {
global $wpdb;

$filter = static function ( $query ) use ( $option_name, $wpdb, &$filter ) {
if ( 0 === stripos( ltrim( $query ), 'INSERT' ) && str_contains( $query, "'{$option_name}'" ) ) {
remove_filter( 'query', $filter );
$wpdb->query( $query ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Replays core's already prepared INSERT.
}
return $query;
};
add_filter( 'query', $filter );
}
}
Loading