Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#27000 closed defect (bug) (fixed)

taxonomy bug with microtime

Reported by: _jameslee Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.8.1
Component: Taxonomy Keywords: good-first-bug has-patch
Focuses: Cc:


in the ~/wp_includes/taxonomy->get_terms() function
the $last_changed if it was not set, it gets set to what ever the microtime() is at that moment. previous versions of this file it was time(), unless you call microtime with the bool of true, you will get a space in between the "msec sec". now this is fine and dandy, but if you are running memcached as your caching device, the space in the cache key causes all sort of weird things to happen.

here's my diff for this file.

Index: taxonomy.php
--- taxonomy.php	(revision 26930)
+++ taxonomy.php	(working copy)
@@ -1313,7 +1313,7 @@
 	$key = md5( serialize( compact(array_keys($defaults)) ) . serialize( $taxonomies ) . $filter_key );
 	$last_changed = wp_cache_get( 'last_changed', 'terms' );
 	if ( ! $last_changed ) {
-		$last_changed = microtime();
+		$last_changed = microtime(true);
 		wp_cache_set( 'last_changed', $last_changed, 'terms' );
 	$cache_key = "get_terms:$key:$last_changed";

Can someone please also update the docblock as well? it has had changes since version 2.3

Attachments (1)

27000.patch (2.9 KB) - added by drozdz 8 years ago.

Download all attachments as: .zip

Change History (10)

#1 @nacin
8 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 3.9

Hi _jameslee, this looks good, thanks! We use microtime() in a few other spots and should validate all of those are good as well.

#2 @_jameslee
8 years ago

class-phpass.php:52:		$this->random_state = microtime() . uniqid(rand(), TRUE); // removed getmypid() for compatibility reasons
class-phpass.php:68:				    md5(microtime() . $this->random_state);
class-snoopy.php:1215:				$this->_mime_boundary = "Snoopy".md5(uniqid(microtime()));
comment.php:283:			$last_changed = microtime();
comment.php:1563:	wp_cache_set( 'last_changed', microtime(), 'comment' );
comment.php:2363:	wp_cache_set( 'last_changed', microtime(), 'comment' );
general-template.php:988:		$last_changed = microtime();
pluggable.php:1612:		$rnd_value = md5( uniqid(microtime() . mt_rand(), true ) . $seed );
post.php:3752:		$last_changed = microtime();
post.php:4766:	wp_cache_set( 'last_changed', microtime(), 'posts' );
taxonomy.php:1316:		$last_changed = microtime();
taxonomy.php:2805:	wp_cache_set( 'last_changed', microtime(), 'terms' );

here is the list of all microtime usage in the wp-includes directory that do not have the bool value set.

8 years ago

#3 @drozdz
8 years ago

  • Keywords has-patch added; needs-patch removed

Changed this only in cache context. I'm pretty sure there is no point in changing this also in places, where microtime is called only to generate md5, unique ids and random values.

Version 0, edited 8 years ago by drozdz (next)

#4 @nacin
8 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 27115:

Use a float for last_changed microtime cache values.

microtime() by default returns a string with a space, which isn't allowed for keys in some cache backends.

props _jameslee, drozdz.
fixes #27000. see #23448.

#5 @westi
8 years ago

Nice find @_jameslee - I'm curious which memcache object cache back end you are using because this one http://wordpress.org/plugins/memcached/ has stripped whitespace from key names since 2006 thereby hiding these bugs from us :)

#6 @_jameslee
8 years ago

we have a very old version of object-cache.php version 0.42 basically an old WPMU version. we will upgrading this very soon. Thanks!

#7 @SergeyBiryukov
8 years ago

[27115] introduced a race condition in taxonomy unit tests, see ticket:14485:95.

#8 @nacin
8 years ago

In 27300:

Revert [27115] and let cache backends handle the stripping of spaces in cache keys as necessary.

microtime() returns greater precision than microtime(true).

see #27000, #23448, #26903, #14485.

This ticket was mentioned in IRC in #wordpress-ui by hanni. View the logs.

8 years ago

Note: See TracTickets for help on using tickets.