Repository navigation
Options: Validate transient names (#58903) - #14865
abhishekfdd wants to merge 1 commit into
Conversation
`set_transient()` and `set_site_transient()` now return false and trigger `_doing_it_wrong()` when the name is not a non-empty string, or is longer than 172 (or 167 for site transients) characters, which would not fit in the `option_name` column alongside the timeout prefix. `get_transient()` and `get_site_transient()` reject non-string and empty names. The delete functions are left permissive so that existing invalid rows can still be cleaned up. Introduces the private `_wp_is_valid_transient_name()` and `_wp_is_valid_transient_name_length()` helpers. See #58903.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
set_transient()andset_site_transient()accept names that cannot be stored correctly: an empty name writes a bare_transient_row, non-strings are silently cast, and names longer than the column allows are truncated or fail at the database layer. This PR rejects those names up front.This picks up the stale #4911 by @jeremyescott and incorporates the review feedback left there and on the ticket.
Changes
_wp_is_valid_transient_name( $transient )(private): returns true only for a string that is not empty or whitespace-only. No casting, so ints, bools,null, arrays and objects are invalid. The name itself is never trimmed or otherwise changed, so existing keys keep working._wp_is_valid_transient_name_length( $transient, $max_length )(private): the limit is 172 characters for transients and 167 for site transients. That isoption_nameVARCHAR(191)minus the_transient_timeout_(19) and_site_transient_timeout_(24) prefixes, per comment:21. Site transients get the same limit because they live inwp_optionson single site, even thoughwp_sitemeta.meta_keyisVARCHAR(255). The column counts characters, not bytes, so it checksstrlen()first and only falls back tomb_strlen()when the byte length is over the limit.set_transient()/set_site_transient(): returnfalsewith_doing_it_wrong()for invalid names, before any filters, cache, or option writes. Empty/non-string names and names that are too long get separate messages.get_transient()/get_site_transient(): returnfalsewith_doing_it_wrong()for non-string or empty names. The length limit is not enforced here.delete_transient()/delete_site_transient(): unchanged (see open questions).Why not validate in
add_option()orWP_Object_Cache?On the ticket, dmsnell suggested moving this down into
add_option()andWP_Object_Cache::is_valid_key(), and having the transient functions reject whatever those reject. I went with validating in the transient functions instead:add_option()never sees an empty transient name. It receives the prefixed_transient_, which is a valid option name, so the bug gets past it.wp_cache_set(). Third-party object cache drop-ins don't shareWP_Object_Cache::is_valid_key(), so core can't rely on it.Open questions for reviewers
_transient_/_site_transient_rows from this bug can still remove them (covered by a test). Should they reject non-strings, or emit a notice for empty names?_doing_it_wrong()in getters. Getters are called far more often than setters, often with dynamically built names. Is a notice appropriate there, or should they just returnfalsequietly?Testing
New tests in
tests/phpunit/tests/option/transient.phpandsiteTransient.phpcover'',' ',null,false,true,0,1,array()and an object for set and get. They also check that a name exactly at the limit (172 / 167) round-trips through set/get/delete with the timeout row written, that a name one over the limit fails, that a multibyte name at the character limit succeeds (so characters are counted, not bytes), and that delete still works on an empty name.--filter "Tests_Option_Transient|Tests_Option_SiteTransient": OK (58 tests, 185 assertions) on single site and multisite.--group option: no failures on single site (492 tests) or multisite (527 tests). The one warning, inTests_Option_wpUserSettings, also occurs on trunk.Both test classes skip under an external object cache, and validation runs before the ext-object-cache branch, so this code path does not depend on the cache.
Trac ticket: https://core.trac.wordpress.org/ticket/58903
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Implementing the code and tests to a design I specified, and drafting this description; I reviewed and directed changes to the diff and test results.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.