[MediaWiki-commits] [Gerrit] Read full memcached response before manipulating data - change (mediawiki/core)

2013-03-07 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Read full memcached response before manipulating data
..


Read full memcached response before manipulating data

Memcached response when fetching data typically looks like this:
VALUE the stored value for whatever key you requested
END

What the code used to do is read the first line (the VALUE) and re-
assemble the data being fetched there (like unserializing serialized
data). After that, it will read the next line (END).

The value could be a serialized object, which could have a __wakeup.
This __wakeup could have code which in turn executes Memcached-
related stuff. The problem is that, while that object is being
unserialized already, it's wakeup code is attempting to read new
stuff from Memcached, but we have yet to read the END of the data
we're attempting to unserialize (when we'll read a new value from
Memcached, the first thing we'd get is the END we have not yet read..)

The correct way to go about this would be to first read the full
Memcached response, and only unserialize the read data after that.
This is exactly what this patchset does.

Change-Id: I902809c6dde657091c8161a09df823170bd41f7a
---
M includes/objectcache/MemcachedClient.php
M tests/phpunit/includes/objectcache/BagOStuffTest.php
2 files changed, 62 insertions(+), 18 deletions(-)

Approvals:
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/objectcache/MemcachedClient.php 
b/includes/objectcache/MemcachedClient.php
index 2342d63..54f6c59 100644
--- a/includes/objectcache/MemcachedClient.php
+++ b/includes/objectcache/MemcachedClient.php
@@ -908,34 +908,47 @@
 * @access private
 */
function _load_items( $sock, $ret, $casToken = null ) {
+   $results = array();
+
while ( 1 ) {
$decl = $this-_fgets( $sock );
if( $decl === false ) {
return false;
-   } elseif ( $decl == END ) {
-   return true;
} elseif ( preg_match( '/^VALUE (\S+) (\d+) (\d+) 
(\d+)$/', $decl, $match ) ) {
-   list( $rkey, $flags, $len, $casToken ) = array( 
$match[1], $match[2], $match[3], $match[4] );
-   $data = $this-_fread( $sock, $len + 2 );
-   if ( $data === false ) {
+
+   $results[] = array(
+   $match[1], // rkey
+   $match[2], // flags
+   $match[3], // len
+   $match[4], // casToken
+   $this-_fread( $sock, $match[3] + 2 ), 
// data
+   );
+   } elseif ( $decl == END ) {
+   if ( count( $results ) == 0 ) {
return false;
}
-   if ( substr( $data, -2 ) !== \r\n ) {
-   $this-_handle_error( $sock,
-   'line ending missing from data 
block from $1' );
-   return false;
-   }
-   $data = substr( $data, 0, -2 );
-   $ret[$rkey] = $data;
 
-   if ( $this-_have_zlib  $flags  
self::COMPRESSED ) {
-   $ret[$rkey] = gzuncompress( $ret[$rkey] 
);
+   foreach ( $results as $vars ) {
+   list( $rkey, $flags, $len, $casToken, 
$data ) = $vars;
+
+   if ( $data === false || substr( $data, 
-2 ) !== \r\n ) {
+   $this-_handle_error( $sock,
+   'line ending missing 
from data block from $1' );
+   return false;
+   }
+   $data = substr( $data, 0, -2 );
+   $ret[$rkey] = $data;
+
+   if ( $this-_have_zlib  $flags  
self::COMPRESSED ) {
+   $ret[$rkey] = gzuncompress( 
$ret[$rkey] );
+   }
+
+   if ( $flags  self::SERIALIZED ) {
+   $ret[$rkey] = unserialize( 
$ret[$rkey] );
+   }
}
 
-   if ( $flags  self::SERIALIZED ) {
-  

[MediaWiki-commits] [Gerrit] Read full memcached response before manipulating data - change (mediawiki/core[master])

2013-02-04 Thread Matthias Mullie (Code Review)
Matthias Mullie has uploaded a new change for review.

Change subject: Read full memcached response before manipulating data
..

Read full memcached response before manipulating data

Memcached response when fetching data typically looks like this:
VALUE the stored value for whatever key you requested
END

What the code used to do is read the first line (the VALUE) and re-
assemble the data is fetches there (like unserializing serialized
data). After that, it will read the next line (END).

The value could be a serialized object, which could have a __wakeup.
This __wakeup could have code which in turn executes Memcached-
related stuff. The problem is that, while that object is being
unserialized already, it's wakeup code is attempting to read new
stuff from Memcached, but we have yet to read the END of the data
we're attempting to unserialize (when we'll read a new value from
Memcached, the first thing we'd get is the END we have not yet read..)

The correct way to go about this would be to first read the full
Memcached response, and only unserialize the read data after that.
This is exactly what this patchset does.

Change-Id: I902809c6dde657091c8161a09df823170bd41f7a
---
M includes/objectcache/MemcachedClient.php
1 file changed, 4 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/19/47419/1
--
To view, visit https://gerrit.wikimedia.org/r/47419
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I902809c6dde657091c8161a09df823170bd41f7a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie mmul...@wikimedia.org

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits