When encountering bad blobs, log the text row id.
authordaniel <daniel.kinzler@wikimedia.de>
Tue, 19 Jun 2018 11:37:35 +0000 (13:37 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Tue, 19 Jun 2018 11:59:01 +0000 (13:59 +0200)
Change-Id: Ie7c932f229854fd3582d507fa7cc154ffdd21f55

includes/Revision.php
includes/Storage/SqlBlobStore.php
tests/phpunit/includes/RevisionTest.php

index 548ef8d..c6889ab 100644 (file)
@@ -1038,7 +1038,18 @@ class Revision implements IDBAccessObject {
                        ? SqlBlobStore::makeAddressFromTextId( $row->old_id )
                        : null;
 
-               return self::getBlobStore( $wiki )->expandBlob( $text, $flags, $cacheKey );
+               $revisionText = self::getBlobStore( $wiki )->expandBlob( $text, $flags, $cacheKey );
+
+               if ( $revisionText === false ) {
+                       if ( isset( $row->old_id ) ) {
+                               wfLogWarning( __METHOD__ . ": Bad data in text row {$row->old_id}! " );
+                       } else {
+                               wfLogWarning( __METHOD__ . ": Bad data in text row! " );
+                       }
+                       return false;
+               }
+
+               return $revisionText;
        }
 
        /**
index 72de2c9..f097f67 100644 (file)
@@ -349,7 +349,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                $blob = $this->expandBlob( $row->old_text, $row->old_flags, $blobAddress );
 
                if ( $blob === false ) {
-                       wfWarn( __METHOD__ . ": Bad data in text row $textId." );
+                       wfLogWarning( __METHOD__ . ": Bad data in text row $textId." );
                        return false;
                }
 
@@ -486,7 +486,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                        $blob = gzinflate( $blob );
 
                        if ( $blob === false ) {
-                               wfLogWarning( __METHOD__ . ': gzinflate() failed' );
+                               wfWarn( __METHOD__ . ': gzinflate() failed' );
                                return false;
                        }
                }
index ab067a4..f50dc80 100644 (file)
@@ -439,6 +439,31 @@ class RevisionTest extends MediaWikiTestCase {
                $this->testGetRevisionText( $expected, $rowData );
        }
 
+       public function provideGetRevisionTextWithZlibExtension_badData() {
+               yield 'Generic gzip test' => [
+                       'This is a small goat of revision text.',
+                       [
+                               'old_flags' => 'gzip',
+                               'old_text' => 'DEAD BEEF',
+                       ],
+               ];
+       }
+
+       /**
+        * @covers Revision::getRevisionText
+        * @dataProvider provideGetRevisionTextWithZlibExtension_badData
+        */
+       public function testGetRevisionWithZlibExtension_badData( $expected, $rowData ) {
+               $this->checkPHPExtension( 'zlib' );
+               Wikimedia\suppressWarnings();
+               $this->assertFalse(
+                       Revision::getRevisionText(
+                               (object)$rowData
+                       )
+               );
+               Wikimedia\suppressWarnings( true );
+       }
+
        private function getWANObjectCache() {
                return new WANObjectCache( [ 'cache' => new HashBagOStuff() ] );
        }
@@ -802,6 +827,7 @@ class RevisionTest extends MediaWikiTestCase {
        public function testGetRevisionText_external_returnsFalseWhenNotEnoughUrlParts(
                $text
        ) {
+               Wikimedia\suppressWarnings();
                $this->assertFalse(
                        Revision::getRevisionText(
                                (object)[
@@ -810,6 +836,7 @@ class RevisionTest extends MediaWikiTestCase {
                                ]
                        )
                );
+               Wikimedia\suppressWarnings( true );
        }
 
        /**