From 564257dc504f42cbe6cfd6e7335672bc893dc3ff Mon Sep 17 00:00:00 2001
From: daniel <daniel.kinzler@wikimedia.de>
Date: Fri, 12 Jan 2018 16:03:47 +0100
Subject: [PATCH] Blob can't be false

Change-Id: Ic06fcfaac71128c0ff7e9079685eac18206f2004
---
 includes/Revision.php                               |  5 +++++
 includes/Storage/SqlBlobStore.php                   | 12 +++++-------
 tests/phpunit/includes/Storage/SqlBlobStoreTest.php | 12 +++++++++++-
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/includes/Revision.php b/includes/Revision.php
index b9a03f5fc3..495e484acb 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -1063,6 +1063,11 @@ class Revision implements IDBAccessObject {
 	 * @return string|bool Decompressed text, or false on failure
 	 */
 	public static function decompressRevisionText( $text, $flags ) {
+		if ( $text === false ) {
+			// Text failed to be fetched; nothing to do
+			return false;
+		}
+
 		return self::getBlobStore()->decompressData( $text, $flags );
 	}
 
diff --git a/includes/Storage/SqlBlobStore.php b/includes/Storage/SqlBlobStore.php
index 72de2c961a..79dce49a98 100644
--- a/includes/Storage/SqlBlobStore.php
+++ b/includes/Storage/SqlBlobStore.php
@@ -399,13 +399,13 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
 						// No negative caching per BlobStore::getBlob()
 						$blob = ExternalStore::fetchFromURL( $url, [ 'wiki' => $this->wikiId ] );
 
-						return $this->decompressData( $blob, $flags );
+						return $blob === false ? false : $this->decompressData( $blob, $flags );
 					},
 					[ 'pcGroup' => self::TEXT_CACHE_GROUP, 'pcTTL' => WANObjectCache::TTL_PROC_LONG ]
 				);
 			} else {
 				$blob = ExternalStore::fetchFromURL( $url, [ 'wiki' => $this->wikiId ] );
-				return $this->decompressData( $blob, $flags );
+				return $blob === false ? false : $this->decompressData( $blob, $flags );
 			}
 		} else {
 			return $this->decompressData( $raw, $flags );
@@ -461,7 +461,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
 	 * @note direct use is deprecated, use getBlob() or SlotRecord::getContent() instead.
 	 * @todo make this private, there should be no need to use this method outside this class.
 	 *
-	 * @param mixed $blob Reference to a text
+	 * @param string $blob Blob in compressed/encoded form.
 	 * @param array $blobFlags Compression flags, such as 'gzip'.
 	 *   Note that not including 'utf-8' in $blobFlags will cause the data to be decoded
 	 *   according to the legacy encoding specified via setLegacyEncoding.
@@ -469,10 +469,8 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
 	 * @return string|bool Decompressed text, or false on failure
 	 */
 	public function decompressData( $blob, array $blobFlags ) {
-		if ( $blob === false ) {
-			// Text failed to be fetched; nothing to do
-			return false;
-		}
+		// Revision::decompressRevisionText accepted false here, so defend against that
+		Assert::parameterType( 'string', $blob, '$blob' );
 
 		if ( in_array( 'error', $blobFlags ) ) {
 			// Error row, return false
diff --git a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
index 07f1f823f0..b5f7322e1a 100644
--- a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
+++ b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php
@@ -86,9 +86,9 @@ class SqlBlobStoreTest extends MediaWikiTestCase {
 	}
 
 	public function provideDecompress() {
-		yield '(no legacy encoding), false in false out' => [ false, false, [], false ];
 		yield '(no legacy encoding), empty in empty out' => [ false, '', [], '' ];
 		yield '(no legacy encoding), empty in empty out' => [ false, 'A', [], 'A' ];
+		yield '(no legacy encoding), error flag -> false' => [ false, 'X', [ 'error' ], false ];
 		yield '(no legacy encoding), string in with gzip flag returns string' => [
 			// gzip string below generated with gzdeflate( 'AAAABBAAA' )
 			false, "sttttr\002\022\000", [ 'gzip' ], 'AAAABBAAA',
@@ -166,6 +166,16 @@ class SqlBlobStoreTest extends MediaWikiTestCase {
 		);
 	}
 
+	/**
+	 * @covers \MediaWiki\Storage\SqlBlobStore::decompressData
+	 */
+	public function testDecompressData_InvalidArgumentException() {
+		$store = $this->getBlobStore();
+
+		$this->setExpectedException( InvalidArgumentException::class );
+		$store->decompressData( false, [] );
+	}
+
 	/**
 	 * @covers \MediaWiki\Storage\SqlBlobStore::compressData
 	 */
-- 
2.20.1