Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#31503 closed defect (bug) (fixed)

code flaw in function wp_clean_update_cache

Reported by: juggledad's profile juggledad Owned by: ocean90's profile ocean90
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.1.1
Component: General Keywords: has-patch commit
Focuses: Cc:

Description (last modified by SergeyBiryukov)

I may be mistaken, but there seems to be a flaw in wp_clean_update_cache. here is teh existing code:

/**
 * Clear existing update caches for plugins, themes, and core.
 *
 * @since 4.1.0
 */
function wp_clean_update_cache() {
	if ( function_exists( 'wp_clean_plugins_cache' ) ) {
		wp_clean_plugins_cache();
	} else {
		delete_site_transient( 'update_plugins' );
	}
	wp_clean_plugins_cache();
	wp_clean_themes_cache();
	delete_site_transient( 'update_core' );
}

If the function 'wp_clean_plugins_cache' exists, then it is called twice, once in the if...else and once outside the if...else.
If the function wp_clean_plugins_cache does not exist then delete_site_transient( 'update_plugins' ); will run (the else) but then you will get a fatal error 'call to an undefined function.

Since no one is getting the fatal error, then the else must never be running and why have two calls to teh same function?

Attachments (2)

31503.diff (840 bytes) - added by TobiasBg 10 years ago.
31503.2.diff (607 bytes) - added by TobiasBg 10 years ago.
Updated patch

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
10 years ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 4.2

Introduced in [30856] ([30870] for the 4.1 branch).

@TobiasBg
10 years ago

#2 follow-up: @TobiasBg
10 years ago

I can't think of a scenario right now, where WP_INC/theme.php (for wp_clean_themes_cache()) is loaded, but plugin.php (for wp_clean_plugins_cache()) isn't. Thus, we should probably add the same check for wp_clean_themes_cache() as well, see 31503.diff.

#3 @DrewAPicture
10 years ago

  • Keywords has-patch commit added

31503.diff still applies. Moving for commit consideration.

This ticket was mentioned in Slack in #core by drew. View the logs.


10 years ago

#5 @ocean90
10 years ago

  • Owner set to ocean90
  • Status changed from new to reviewing

#6 in reply to: ↑ 2 ; follow-up: @nacin
10 years ago

Replying to TobiasBg:

I can't think of a scenario right now, where WP_INC/theme.php (for wp_clean_themes_cache()) is loaded, but plugin.php (for wp_clean_plugins_cache()) isn't. Thus, we should probably add the same check for wp_clean_themes_cache() as well, see 31503.diff.

wp_clean_plugins_cache() is in *wp-admin/includes*, which is not always loaded, while wp-includes/theme.php is always loaded. Just removing the one line is appropriate.

#7 in reply to: ↑ 6 @TobiasBg
10 years ago

Replying to nacin:

wp_clean_plugins_cache() is in *wp-admin/includes*, which is not always loaded, while wp-includes/theme.php is always loaded. Just removing the one line is appropriate.

Ah, indeed. Had somehow confused it with the WP_INC/plugin.php. Thanks for clearing this up!

@TobiasBg
10 years ago

Updated patch

#8 @ocean90
10 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 31825:

Don't try to call wp_clean_plugins_cache(); twice in wp_clean_update_cache().

props TobiasBg.
fixes #31503.

Note: See TracTickets for help on using tickets.