WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#31503 closed defect (bug) (fixed)

code flaw in function wp_clean_update_cache

Reported by: juggledad Owned by: 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 6 years ago.
31503.2.diff (607 bytes) - added by TobiasBg 6 years ago.
Updated patch

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
6 years ago

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

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

@TobiasBg
6 years ago

#2 follow-up: @TobiasBg
6 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
6 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.


6 years ago

#5 @ocean90
6 years ago

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

#6 in reply to: ↑ 2 ; follow-up: @nacin
6 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
6 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
6 years ago

Updated patch

#8 @ocean90
6 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.