[wp-trac] [WordPress Trac] #10300: Optimization in wp_get_sidebars_widgets() corrupts the widgets

WordPress Trac wp-trac at lists.automattic.com
Mon Jun 29 18:44:55 GMT 2009


#10300: Optimization in wp_get_sidebars_widgets() corrupts the widgets
-------------------------------+--------------------------------------------
 Reporter:  Denis-de-Bernardy  |       Owner:  azaozz
     Type:  defect (bug)       |      Status:  new   
 Priority:  normal             |   Milestone:  2.8.1 
Component:  Widgets            |     Version:  2.8   
 Severity:  blocker            |    Keywords:        
-------------------------------+--------------------------------------------
 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:

  1. call wp_get_sidebars_widgets() (e.g. due to dynamic_sidebar())
  2. check for $_wp_sidebars_widgets, which isn't set yet
  3. set $_wp_sidebars_widgets using get_option()
  4. if array_version is not set, run upgrade procedure
  5. unset array_version, so as to return a clean sidebars_widgets array
  6. call wp_get_sidebars_widgets() again for whatever reason (e.g. you've
 two sidebars)
  7. check for $_wp_sidebars_widgets, which is now set
  8. array_version is no longer around, since it was unset, so run the
 upgrade procedure
  9. 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.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/10300>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list