#25213 closed defect (bug) (fixed)
set_site_transient() incorrectly passes prefixed transient names to hooks when not using an object cache
Reported by: | dd32 | Owned by: | dd32 |
---|---|---|---|
Milestone: | 3.7 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | General | Keywords: | has-patch |
Focuses: | Cc: |
Description (last modified by )
When using the hooks within set_site_transient(), developers are forced to cover two different cases, Object caching, and no object caching.
This is because the value of the $transient
is overwritten in the no-object cache case, ending up with two different action names fired:
For those not using an object cache: set_site_transient__site_transient_update_plugins
For those using an object cache: set_site_transient_update_plugins
.
The setted_site_transient
hook also receives a prefixed value in the event of no object caching, ie, update_plugins
for an object cache, _site_transient_update_plugins
for no object cache.
This appears to stretch back to [13148]
Attached is a patch for review, with some back-compat actions commented out for now
Attachments (1)
Change History (10)
#3
@
11 years ago
I think we can also keep in mind this is the set_site_transient() function, not set_transient(). As it is, when we moved certain transients from set_transient() to set_site_transient(), we changed the hooks being used for numerous transients. I pointed this out somewhere (no idea if it was Trac, IRC, or a mailing list) but we decided to not take any action. I never saw a single complaint.
And as for this bug: No one has reported this previously. It's an obscure hook in an obscure level (site_transient, versus transient) of the transients API. If they wanted to use it, they immediately would have realized something was very, very wrong. And the "correct" way to use it would have been to also account for an external object cache.
I have a feeling that firing only the correct hook here would be OK. My instinct is to pull the band-aid off, do a make/core post on it (3.7-compat tag), and move on.
#4
@
11 years ago
I have a feeling that firing only the correct hook here would be OK. My instinct is to pull the band-aid off, do a make/core post on it (3.7-compat tag), and move on.
I think we're in agreement here really, my previous comment was being way too cautious on compatibility issues, and i'd also say these actions are not commonly used, otherwise we'd have heard about it already.
#5
@
11 years ago
- Owner set to dd32
- Resolution set to fixed
- Status changed from new to closed
In 25350:
#6
follow-up:
↓ 7
@
11 years ago
Worth a grep of the plugin repo for set_site_transient_
? With the trailing underscore there it should just flag up any plugins that are using that hook.
#7
in reply to:
↑ 6
@
11 years ago
Replying to johnbillion:
Worth a grep of the plugin repo for
set_site_transient_
? With the trailing underscore there it should just flag up any plugins that are using that hook.
Sure, done. Nearly every result found for set_site_transient_
is for the pre_set_site_transient_
filter (almost 70 instances). Two exceptions:
- pento's automatic-updater plugin, which attached a callback to both versions of the action.
- dd32's wordpress-background-update plugin, which didn't account for the "proper" action. DUDE.
#8
follow-up:
↓ 9
@
11 years ago
dd32's wordpress-background-update plugin, which didn't account for the "proper" action. DUDE.
I had forgotten that plugin ever existed..
But I knew I'd seen this stupidity before! - I guess I didn't realise that the action was so messed up (or firing 2 different actions) and just copy-pasted it.
#9
in reply to:
↑ 8
@
11 years ago
Replying to dd32:
dd32's wordpress-background-update plugin, which didn't account for the "proper" action. DUDE.
I had forgotten that plugin ever existed..
But I knew I'd seen this stupidity before! - I guess I didn't realise that the action was so messed up (or firing 2 different actions) and just copy-pasted it.
Yeah, yeah — caught red-handed. :-)
Current thoughts on the back-compat:
So really, we have back compat issues either way, Either we fire only the proper action, or we fire both.
Firing both seems like it'll be the lesser of 2 evils in this case, most existing code should still work, but may cause duplicate processing for some plugin code until fixed.