Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
94 changes: 94 additions & 0 deletions src/wp-includes/option.php
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,50 @@ function wp_filter_default_autoload_value_via_option_size( $autoload, $option, $
return $autoload;
}

/**
* Determines whether a transient name is a non-empty string.
*
* Names consisting solely of whitespace are considered empty.
*
* @since 7.3.0
* @access private
*
* @param mixed $transient Transient name to validate.
* @return bool True if the transient name is a non-empty string, false otherwise.
*/
function _wp_is_valid_transient_name( $transient ) {
return is_string( $transient ) && '' !== trim( $transient );
}

/**
* Determines whether a transient name fits within a maximum length.
*
* Transients are stored in `option_name`, a VARCHAR(191) column. Site transients
* share that limit, since they are stored there on single site installations.
* The longest key written is the timeout, so the name must leave room for the
* `_transient_timeout_` (19 characters) or `_site_transient_timeout_` (24 characters)
* prefix: 191 - 19 = 172 and 191 - 24 = 167.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is some complicated exposition, and yet I wonder if it helps or harms comprehension.

for one, it’s overly-coupled to the later code. the comment mentions the option prefixes, but those don’t appear in this function. that’s a distraction that will lead someone hunting for an explanation of why.

additionally, this highlights an abstraction mismatch in this function, which I think shouldn’t exist. I’ll share more in the next comment.

*
* @since 7.3.0
* @access private
*
* @param string $transient Transient name.
* @param int $max_length Maximum number of characters: 172 for transients,
* 167 for site transients.
* @return bool True if the transient name is within the limit, false otherwise.
*/
function _wp_is_valid_transient_name_length( $transient, $max_length ) {
/*
* VARCHAR lengths count characters, not bytes. strlen() can only overcount a
* multibyte string, so the slower mb_strlen() is only needed when it exceeds the limit.
*/
if ( strlen( $transient ) <= $max_length ) {
return true;
}

return mb_strlen( $transient, 'UTF-8' ) <= $max_length;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function doesn’t do much despite adding a lot of documentation and some indirection. it’s a thin wrapper around mb_strlen() and it doesn’t apply any semantic constraints because the max length is provided by the caller.

if transients are stored in options, then what we want is is_valid_key() for options, which can then apply only those constraints related to options, and then we never have to talk about _transient_ or _transient_timeout_ because those will be handled automatically.

}

/**
* Deletes a transient.
*
Expand Down Expand Up @@ -1426,12 +1470,18 @@ function delete_transient( $transient ) {
* then the return value will be false.
*
* @since 2.8.0
* @since 7.3.0 Returns false if the transient name is not a non-empty string.
*
* @param string $transient Transient name. Expected to not be SQL-escaped.
* @return mixed Value of transient.
*/
function get_transient( $transient ) {

if ( ! _wp_is_valid_transient_name( $transient ) ) {
_doing_it_wrong( __FUNCTION__, __( 'Transient names must be non-empty strings.' ), '7.3.0' );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a read operation may not warrant _doing_it_wrong(), especially if we could ever attempt reads on user-supplied key names.

in any case, the error message contradicts the code.

return false;
}

/**
* Filters the value of an existing transient before it is retrieved.
*
Expand Down Expand Up @@ -1500,6 +1550,7 @@ function get_transient( $transient ) {
* then it will be serialized before it is set.
*
* @since 2.8.0
* @since 7.3.0 Returns false if the transient name is invalid.
*
* @param string $transient Transient name. Expected to not be SQL-escaped.
* Must be 172 characters or fewer in length.
Expand All @@ -1510,6 +1561,24 @@ function get_transient( $transient ) {
*/
function set_transient( $transient, $value, $expiration = 0 ) {

if ( ! _wp_is_valid_transient_name( $transient ) ) {
_doing_it_wrong( __FUNCTION__, __( 'Transient names must be non-empty strings.' ), '7.3.0' );
return false;
}

if ( ! _wp_is_valid_transient_name_length( $transient, 172 ) ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ( '' === trim( $transient ) || ! wp_is_valid_option_name( "_transient_{$transient}" ) )

this is where the specifics about _transient_ can disappear. we can still have the function, but it would delegate to the option code.

function wp_parse_transient_name( $name ) {
	$trimmed = trim( $name );

	return (
		'' !== $trimmed &&
		! str_starts_with( $trimmed, 'timeout_' ) &&
		wp_is_valid_option_name( "_transient_timeout_{$trimmed}" ) &&
		wp_is_valid_option_name( "_transient_{$trimmed}" )
	)
		? $trimmed
		: null;
}

_doing_it_wrong(
__FUNCTION__,
sprintf(
/* translators: %d: Maximum number of characters in a transient name. */
__( 'Transient names must be %d characters or fewer.' ),
172
),
'7.3.0'
);
return false;
}

$expiration = (int) $expiration;

/**
Expand Down Expand Up @@ -2559,6 +2628,7 @@ function delete_site_transient( $transient ) {
* then the return value will be false.
*
* @since 2.9.0
* @since 7.3.0 Returns false if the transient name is not a non-empty string.
*
* @see get_transient()
*
Expand All @@ -2567,6 +2637,11 @@ function delete_site_transient( $transient ) {
*/
function get_site_transient( $transient ) {

if ( ! _wp_is_valid_transient_name( $transient ) ) {
_doing_it_wrong( __FUNCTION__, __( 'Transient names must be non-empty strings.' ), '7.3.0' );
return false;
}

/**
* Filters the value of an existing site transient before it is retrieved.
*
Expand Down Expand Up @@ -2633,6 +2708,7 @@ function get_site_transient( $transient ) {
* then it will be serialized before it is set.
*
* @since 2.9.0
* @since 7.3.0 Returns false if the transient name is invalid.
*
* @see set_transient()
*
Expand All @@ -2644,6 +2720,24 @@ function get_site_transient( $transient ) {
*/
function set_site_transient( $transient, $value, $expiration = 0 ) {

if ( ! _wp_is_valid_transient_name( $transient ) ) {
_doing_it_wrong( __FUNCTION__, __( 'Transient names must be non-empty strings.' ), '7.3.0' );
return false;
}

if ( ! _wp_is_valid_transient_name_length( $transient, 167 ) ) {
_doing_it_wrong(
__FUNCTION__,
sprintf(
/* translators: %d: Maximum number of characters in a transient name. */
__( 'Transient names must be %d characters or fewer.' ),
167
),
'7.3.0'
);
return false;
}

/**
* Filters the value of a specific site transient before it is set.
*
Expand Down
132 changes: 132 additions & 0 deletions tests/phpunit/tests/option/siteTransient.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,136 @@ 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 set_site_transient() rejects names that are not non-empty strings.
*
* @ticket 58903
*
* @dataProvider data_invalid_site_transient_names
* @expectedIncorrectUsage set_site_transient
*
* @covers ::set_site_transient
* @covers ::_wp_is_valid_transient_name
*
* @param mixed $transient Transient name.
*/
public function test_set_site_transient_should_reject_invalid_name( $transient ) {
$this->assertFalse( set_site_transient( $transient, 'value' ), 'set_site_transient() should return false.' );
$this->assertFalse( get_site_option( '_site_transient_' ), 'No option should be added for an empty transient name.' );
}

/**
* Tests that get_site_transient() rejects names that are not non-empty strings.
*
* @ticket 58903
*
* @dataProvider data_invalid_site_transient_names
* @expectedIncorrectUsage get_site_transient
*
* @covers ::get_site_transient
* @covers ::_wp_is_valid_transient_name
*
* @param mixed $transient Transient name.
*/
public function test_get_site_transient_should_reject_invalid_name( $transient ) {
$this->assertFalse( get_site_transient( $transient ) );
}

/**
* Data provider.
*
* @return array[]
*/
public function data_invalid_site_transient_names() {
return array(
'empty string' => array( '' ),
'whitespace only' => array( ' ' ),
'null' => array( null ),
'boolean false' => array( false ),
'boolean true' => array( true ),
'integer zero' => array( 0 ),
'integer one' => array( 1 ),
'empty array' => array( array() ),
'object' => array( new stdClass() ),
);
}

/**
* Tests that set_site_transient() accepts a name at the 167 character limit.
*
* @ticket 58903
*
* @dataProvider data_site_transient_names_at_length_limit
*
* @covers ::set_site_transient
* @covers ::get_site_transient
* @covers ::delete_site_transient
* @covers ::_wp_is_valid_transient_name_length
*
* @param string $transient Transient name.
*/
public function test_set_site_transient_should_accept_name_at_length_limit( $transient ) {
$this->assertTrue( set_site_transient( $transient, 'value', HOUR_IN_SECONDS ), 'set_site_transient() should return true.' );
$this->assertNotFalse( get_site_option( '_site_transient_timeout_' . $transient ), 'The timeout option should be stored.' );
$this->assertSame( 'value', get_site_transient( $transient ), 'get_site_transient() should return the stored value.' );
$this->assertTrue( delete_site_transient( $transient ), 'delete_site_transient() should return true.' );
$this->assertFalse( get_site_transient( $transient ), 'get_site_transient() should return false after deletion.' );
}

/**
* Data provider.
*
* @return array[]
*/
public function data_site_transient_names_at_length_limit() {
return array(
'167 single-byte characters' => array( str_repeat( 'a', 167 ) ),
'167 multibyte characters' => array( str_repeat( 'é', 167 ) ),
);
}

/**
* Tests that set_site_transient() rejects a name over the 167 character limit.
*
* @ticket 58903
*
* @dataProvider data_site_transient_names_over_length_limit
* @expectedIncorrectUsage set_site_transient
*
* @covers ::set_site_transient
* @covers ::_wp_is_valid_transient_name_length
*
* @param string $transient Transient name.
*/
public function test_set_site_transient_should_reject_name_over_length_limit( $transient ) {
$this->assertFalse( set_site_transient( $transient, 'value' ), 'set_site_transient() should return false.' );
$this->assertFalse( get_site_option( '_site_transient_' . $transient ), 'No option should be added.' );
}

/**
* Data provider.
*
* @return array[]
*/
public function data_site_transient_names_over_length_limit() {
return array(
'168 single-byte characters' => array( str_repeat( 'a', 168 ) ),
'168 multibyte characters' => array( str_repeat( 'é', 168 ) ),
);
}

/**
* Tests that delete_site_transient() can still remove a site transient stored with an empty name.
*
* @ticket 58903
*
* @covers ::delete_site_transient
*/
public function test_delete_site_transient_should_delete_site_transient_with_empty_name() {
add_site_option( '_site_transient_', 'value' );

$this->assertTrue( delete_site_transient( '' ), 'delete_site_transient() should return true.' );
$this->assertFalse( get_site_option( '_site_transient_' ), 'The option should be deleted.' );
}
}
Loading
Loading