From 26d87a26fee1b6e66e221c4452a9f1d23cc003b6 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 22 Feb 2018 19:23:19 -0800
Subject: [PATCH] rdbms: make DBMasterPos implement Serializable

ChronologyProtector uses these classes to briefly store positions
and everytime the fields change then errors can happen when old
values are unserialized and used. Use a simple two-element map
format for serialized positions. The fields are recomputed back
from the data map.

Values from before this change will issue the warning
"Erroneous data format for unserializing". To avoid that, bump
the ChronologyProtector key version. Future field changes will
not require this.

This change should be deployed on all wikis at once.

Bug: T187942
Change-Id: I71bbbc9b9d4c7e02ac02f1d8750b70bda08d4db1
---
 includes/libs/rdbms/ChronologyProtector.php   |  2 +-
 .../rdbms/database/position/DBMasterPos.php   |  4 +++-
 .../database/position/MySQLMasterPos.php      | 24 ++++++++++++++++++-
 .../rdbms/database/DatabaseMysqlBaseTest.php  | 15 ++++++++++++
 4 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/includes/libs/rdbms/ChronologyProtector.php b/includes/libs/rdbms/ChronologyProtector.php
index 88e276f122..e11528659d 100644
--- a/includes/libs/rdbms/ChronologyProtector.php
+++ b/includes/libs/rdbms/ChronologyProtector.php
@@ -75,7 +75,7 @@ class ChronologyProtector implements LoggerAwareInterface {
 	public function __construct( BagOStuff $store, array $client, $posIndex = null ) {
 		$this->store = $store;
 		$this->clientId = md5( $client['ip'] . "\n" . $client['agent'] );
-		$this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 'v1' );
+		$this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 'v2' );
 		$this->waitForPosIndex = $posIndex;
 		$this->logger = new NullLogger();
 	}
diff --git a/includes/libs/rdbms/database/position/DBMasterPos.php b/includes/libs/rdbms/database/position/DBMasterPos.php
index 2f79ea9a20..28d2a1b804 100644
--- a/includes/libs/rdbms/database/position/DBMasterPos.php
+++ b/includes/libs/rdbms/database/position/DBMasterPos.php
@@ -2,12 +2,14 @@
 
 namespace Wikimedia\Rdbms;
 
+use Serializable;
+
 /**
  * An object representing a master or replica DB position in a replicated setup.
  *
  * The implementation details of this opaque type are up to the database subclass.
  */
-interface DBMasterPos {
+interface DBMasterPos extends Serializable {
 	/**
 	 * @return float UNIX timestamp
 	 * @since 1.25
diff --git a/includes/libs/rdbms/database/position/MySQLMasterPos.php b/includes/libs/rdbms/database/position/MySQLMasterPos.php
index 2ee9068446..cdcb79cde6 100644
--- a/includes/libs/rdbms/database/position/MySQLMasterPos.php
+++ b/includes/libs/rdbms/database/position/MySQLMasterPos.php
@@ -3,6 +3,7 @@
 namespace Wikimedia\Rdbms;
 
 use InvalidArgumentException;
+use UnexpectedValueException;
 
 /**
  * DBMasterPos class for MySQL/MariaDB
@@ -27,6 +28,14 @@ class MySQLMasterPos implements DBMasterPos {
 	 * @param float $asOfTime UNIX timestamp
 	 */
 	public function __construct( $position, $asOfTime ) {
+		$this->init( $position, $asOfTime );
+	}
+
+	/**
+	 * @param string $position
+	 * @param float $asOfTime
+	 */
+	protected function init( $position, $asOfTime ) {
 		$m = [];
 		if ( preg_match( '!^(.+)\.(\d+)/(\d+)$!', $position, $m ) ) {
 			$this->binlog = $m[1]; // ideally something like host name
@@ -34,7 +43,7 @@ class MySQLMasterPos implements DBMasterPos {
 		} else {
 			$gtids = array_filter( array_map( 'trim', explode( ',', $position ) ) );
 			foreach ( $gtids as $gtid ) {
-				if ( !$this->parseGTID( $gtid ) ) {
+				if ( !self::parseGTID( $gtid ) ) {
 					throw new InvalidArgumentException( "Invalid GTID '$gtid'." );
 				}
 				$this->gtids[] = $gtid;
@@ -192,4 +201,17 @@ class MySQLMasterPos implements DBMasterPos {
 			? [ 'binlog' => $this->binlog, 'pos' => $this->pos ]
 			: false;
 	}
+
+	public function serialize() {
+		return serialize( [ 'position' => $this->__toString(), 'asOfTime' => $this->asOfTime ] );
+	}
+
+	public function unserialize( $serialized ) {
+		$data = unserialize( $serialized );
+		if ( !is_array( $data ) ) {
+			throw new UnexpectedValueException( __METHOD__ . ": cannot unserialize position" );
+		}
+
+		$this->init( $data['position'], $data['asOfTime'] );
+	}
 }
diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
index b9f57b568b..5fcca1a410 100644
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
@@ -495,4 +495,19 @@ class DatabaseMysqlBaseTest extends PHPUnit\Framework\TestCase {
 
 		$db->clearFlag( Database::DBO_IGNORE );
 	}
+
+	/**
+	 * @covers Wikimedia\Rdbms\MySQLMasterPos
+	 */
+	public function testSerialize() {
+		$pos = new MySQLMasterPos( '3E11FA47-71CA-11E1-9E33-C80AA9429562:99', 53636363 );
+		$roundtripPos = unserialize( serialize( $pos ) );
+
+		$this->assertEquals( $pos, $roundtripPos );
+
+		$pos = new MySQLMasterPos( '255-11-23', 53636363 );
+		$roundtripPos = unserialize( serialize( $pos ) );
+
+		$this->assertEquals( $pos, $roundtripPos );
+	}
 }
-- 
2.20.1