Optimization in wp_get_sidebars_widgets() corrupts the widgets
|Reported by:||Denis-de-Bernardy||Owned by:||azaozz|
Found this while testing upgrade scripts. It's not simple to reproduce in a live case, either. (You need to switch from a multi-sidebar theme to another multi-sidebar theme with different sidebars, possibly at the same time you're upgrading from WP 2.7 to WP 2.8.1-beta.)
Basically, the workflow in the non-admin area goes down to this:
- call wp_get_sidebars_widgets() (e.g. due to dynamic_sidebar())
- check for $_wp_sidebars_widgets, which isn't set yet
- set $_wp_sidebars_widgets using get_option()
- if array_version is not set, run upgrade procedure
- unset array_version, so as to return a clean sidebars_widgets array
- call wp_get_sidebars_widgets() again for whatever reason (e.g. you've two sidebars)
- check for $_wp_sidebars_widgets, which is now set
- array_version is no longer around, since it was unset, so run the upgrade procedure
- Corrupt empty sidebars, and turn the site into a train wreck (in my case, ext_sidebar became top_sidebar instead of the sidebar-2 that I was trying to auto-convert)
I investigated a couple of potential patches.
Setting array_version to 3 in wp_convert_widget_settings is plain wrong.
Merely removing the ref in wp_get_sidebars_widgets() works but it potentially removes the work that wp_convert_widget_settings() is doing in the background.
Checking for $_wp_sidebars_widgets before setting a default array_version seemed wrong: on a very old site, array_version is not set indeed (I've yet to test how it behaves, but figured it was better to just fix the workflow instead).
The last option was to fix the workflow. The attached patch does just that.
Change History (39)
- Resolution fixed deleted
- Status changed from closed to reopened