From 90ca759f1527338d0745e8439973da01ff528c12 Mon Sep 17 00:00:00 2001 From: addshore Date: Fri, 12 Jan 2018 11:52:31 +0000 Subject: [PATCH] RevisionStore, fix loadSlotContent with no $blobFlags This includes tests that were previously created in: I6dcfc0497bfce6605fa5517c9f91faf7131f4334 Bug: T184749 Change-Id: Ieb02ac593fc6b42af1692d03d9d578a76417eb54 --- includes/Storage/RevisionStore.php | 35 +++-- .../includes/Storage/RevisionStoreDbTest.php | 130 +++++++++++++++++- 2 files changed, 150 insertions(+), 15 deletions(-) diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php index 6c8cac9423..b0c193b31e 100644 --- a/includes/Storage/RevisionStore.php +++ b/includes/Storage/RevisionStore.php @@ -689,7 +689,7 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup $content = null; $blobData = null; - $blobFlags = ''; + $blobFlags = null; if ( is_object( $row ) ) { // archive row @@ -706,7 +706,11 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup if ( isset( $row->old_text ) ) { // this happens when the text-table gets joined directly, in the pre-1.30 schema $blobData = isset( $row->old_text ) ? strval( $row->old_text ) : null; - $blobFlags = isset( $row->old_flags ) ? strval( $row->old_flags ) : ''; + // Check against selects that might have not included old_flags + if ( !property_exists( $row, 'old_flags' ) ) { + throw new InvalidArgumentException( 'old_flags was not set in $row' ); + } + $blobFlags = ( $row->old_flags === null ) ? '' : $row->old_flags; } $mainSlotRow->slot_revision = intval( $row->rev_id ); @@ -735,7 +739,9 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup $mainSlotRow->format_name = isset( $row['content_format'] ) ? strval( $row['content_format'] ) : null; $blobData = isset( $row['text'] ) ? rtrim( strval( $row['text'] ) ) : null; - $blobFlags = isset( $row['flags'] ) ? trim( strval( $row['flags'] ) ) : ''; + // XXX: If the flags field is not set then $blobFlags should be null so that no + // decoding will happen. An empty string will result in default decodings. + $blobFlags = isset( $row['flags'] ) ? trim( strval( $row['flags'] ) ) : null; // if we have a Content object, override mText and mContentModel if ( !empty( $row['content'] ) ) { @@ -797,7 +803,8 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup * * @param SlotRecord $slot The SlotRecord to load content for * @param string|null $blobData The content blob, in the form indicated by $blobFlags - * @param string $blobFlags Flags indicating how $blobData needs to be processed + * @param string|null $blobFlags Flags indicating how $blobData needs to be processed. + * null if no processing should happen. * @param string|null $blobFormat MIME type indicating how $dataBlob is encoded * @param int $queryFlags * @@ -807,23 +814,27 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup private function loadSlotContent( SlotRecord $slot, $blobData = null, - $blobFlags = '', + $blobFlags = null, $blobFormat = null, $queryFlags = 0 ) { if ( $blobData !== null ) { Assert::parameterType( 'string', $blobData, '$blobData' ); - Assert::parameterType( 'string', $blobFlags, '$blobFlags' ); + Assert::parameterType( 'string|null', $blobFlags, '$blobFlags' ); $cacheKey = $slot->hasAddress() ? $slot->getAddress() : null; - $data = $this->blobStore->expandBlob( $blobData, $blobFlags, $cacheKey ); - - if ( $data === false ) { - throw new RevisionAccessException( - "Failed to expand blob data using flags $blobFlags (key: $cacheKey)" - ); + if ( $blobFlags === null ) { + $data = $blobData; + } else { + $data = $this->blobStore->expandBlob( $blobData, $blobFlags, $cacheKey ); + if ( $data === false ) { + throw new RevisionAccessException( + "Failed to expand blob data using flags $blobFlags (key: $cacheKey)" + ); + } } + } else { $address = $slot->getAddress(); try { diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php index 15f173a2c9..e33de20b2e 100644 --- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php +++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php @@ -6,8 +6,10 @@ use CommentStoreComment; use Exception; use HashBagOStuff; use InvalidArgumentException; +use Language; use MediaWiki\Linker\LinkTarget; use MediaWiki\MediaWikiServices; +use MediaWiki\Storage\BlobStoreFactory; use MediaWiki\Storage\IncompleteRevisionException; use MediaWiki\Storage\MutableRevisionRecord; use MediaWiki\Storage\RevisionRecord; @@ -611,9 +613,10 @@ class RevisionStoreDbTest extends MediaWikiTestCase { */ public function testNewRevisionFromRow_anonEdit() { $page = WikiPage::factory( Title::newFromText( 'UTPage' ) ); + $text = __METHOD__ . 'a-ä'; /** @var Revision $rev */ $rev = $page->doEditContent( - new WikitextContent( __METHOD__. 'a' ), + new WikitextContent( $text ), __METHOD__. 'a' )->value['revision']; @@ -624,6 +627,32 @@ class RevisionStoreDbTest extends MediaWikiTestCase { $page->getTitle() ); $this->assertRevisionRecordMatchesRevision( $rev, $record ); + $this->assertSame( $text, $rev->getContent()->serialize() ); + } + + /** + * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromRow + * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromRow_1_29 + */ + public function testNewRevisionFromRow_anonEdit_legacyEncoding() { + $this->setMwGlobals( 'wgLegacyEncoding', 'windows-1252' ); + $this->overrideMwServices(); + $page = WikiPage::factory( Title::newFromText( 'UTPage' ) ); + $text = __METHOD__ . 'a-ä'; + /** @var Revision $rev */ + $rev = $page->doEditContent( + new WikitextContent( $text ), + __METHOD__. 'a' + )->value['revision']; + + $store = MediaWikiServices::getInstance()->getRevisionStore(); + $record = $store->newRevisionFromRow( + $this->revisionToRow( $rev ), + [], + $page->getTitle() + ); + $this->assertRevisionRecordMatchesRevision( $rev, $record ); + $this->assertSame( $text, $rev->getContent()->serialize() ); } /** @@ -632,9 +661,10 @@ class RevisionStoreDbTest extends MediaWikiTestCase { */ public function testNewRevisionFromRow_userEdit() { $page = WikiPage::factory( Title::newFromText( 'UTPage' ) ); + $text = __METHOD__ . 'b-ä'; /** @var Revision $rev */ $rev = $page->doEditContent( - new WikitextContent( __METHOD__. 'b' ), + new WikitextContent( $text ), __METHOD__ . 'b', 0, false, @@ -648,6 +678,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase { $page->getTitle() ); $this->assertRevisionRecordMatchesRevision( $rev, $record ); + $this->assertSame( $text, $rev->getContent()->serialize() ); } /** @@ -656,9 +687,41 @@ class RevisionStoreDbTest extends MediaWikiTestCase { public function testNewRevisionFromArchiveRow() { $store = MediaWikiServices::getInstance()->getRevisionStore(); $title = Title::newFromText( __METHOD__ ); + $text = __METHOD__ . '-bä'; + $page = WikiPage::factory( $title ); + /** @var Revision $orig */ + $orig = $page->doEditContent( new WikitextContent( $text ), __METHOD__ ) + ->value['revision']; + $page->doDeleteArticle( __METHOD__ ); + + $db = wfGetDB( DB_MASTER ); + $arQuery = $store->getArchiveQueryInfo(); + $res = $db->select( + $arQuery['tables'], $arQuery['fields'], [ 'ar_rev_id' => $orig->getId() ], + __METHOD__, [], $arQuery['joins'] + ); + $this->assertTrue( is_object( $res ), 'query failed' ); + + $row = $res->fetchObject(); + $res->free(); + $record = $store->newRevisionFromArchiveRow( $row ); + + $this->assertRevisionRecordMatchesRevision( $orig, $record ); + $this->assertSame( $text, $record->getContent( 'main' )->serialize() ); + } + + /** + * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromArchiveRow + */ + public function testNewRevisionFromArchiveRow_legacyEncoding() { + $this->setMwGlobals( 'wgLegacyEncoding', 'windows-1252' ); + $this->overrideMwServices(); + $store = MediaWikiServices::getInstance()->getRevisionStore(); + $title = Title::newFromText( __METHOD__ ); + $text = __METHOD__ . '-bä'; $page = WikiPage::factory( $title ); /** @var Revision $orig */ - $orig = $page->doEditContent( new WikitextContent( __METHOD__ ), __METHOD__ ) + $orig = $page->doEditContent( new WikitextContent( $text ), __METHOD__ ) ->value['revision']; $page->doDeleteArticle( __METHOD__ ); @@ -675,6 +738,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase { $record = $store->newRevisionFromArchiveRow( $row ); $this->assertRevisionRecordMatchesRevision( $orig, $record ); + $this->assertSame( $text, $record->getContent( 'main' )->serialize() ); } /** @@ -1026,6 +1090,39 @@ class RevisionStoreDbTest extends MediaWikiTestCase { 'content' => new WikitextContent( 'Some Content' ), ] ]; + yield 'Basic array, serialized text' => [ + [ + 'id' => 2, + 'page' => 1, + 'timestamp' => '20171017114835', + 'user_text' => '111.0.1.2', + 'user' => 0, + 'minor_edit' => false, + 'deleted' => 0, + 'len' => 46, + 'parent_id' => 1, + 'sha1' => 'rdqbbzs3pkhihgbs8qf2q9jsvheag5z', + 'comment' => 'Goat Comment!', + 'text' => ( new WikitextContent( 'Söme Content' ) )->serialize(), + ] + ]; + yield 'Basic array, serialized text, utf-8 flags' => [ + [ + 'id' => 2, + 'page' => 1, + 'timestamp' => '20171017114835', + 'user_text' => '111.0.1.2', + 'user' => 0, + 'minor_edit' => false, + 'deleted' => 0, + 'len' => 46, + 'parent_id' => 1, + 'sha1' => 'rdqbbzs3pkhihgbs8qf2q9jsvheag5z', + 'comment' => 'Goat Comment!', + 'text' => ( new WikitextContent( 'Söme Content' ) )->serialize(), + 'flags' => 'utf-8', + ] + ]; yield 'Basic array, with title' => [ [ 'title' => Title::newFromText( 'SomeText' ), @@ -1092,6 +1189,8 @@ class RevisionStoreDbTest extends MediaWikiTestCase { $this->assertTrue( $result->getSlot( 'main' )->getContent()->equals( $array['content'] ) ); + } elseif ( isset( $array['text'] ) ) { + $this->assertSame( $array['text'], $result->getSlot( 'main' )->getContent()->serialize() ); } else { $this->assertSame( $array['content_format'], @@ -1101,4 +1200,29 @@ class RevisionStoreDbTest extends MediaWikiTestCase { } } + /** + * @dataProvider provideNewMutableRevisionFromArray + * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray + */ + public function testNewMutableRevisionFromArray_legacyEncoding( array $array ) { + $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] ); + $blobStore = new SqlBlobStore( wfGetLB(), $cache ); + $blobStore->setLegacyEncoding( 'windows-1252', Language::factory( 'en' ) ); + + $factory = $this->getMockBuilder( BlobStoreFactory::class ) + ->setMethods( [ 'newBlobStore', 'newSqlBlobStore' ] ) + ->disableOriginalConstructor() + ->getMock(); + $factory->expects( $this->any() ) + ->method( 'newBlobStore' ) + ->willReturn( $blobStore ); + $factory->expects( $this->any() ) + ->method( 'newSqlBlobStore' ) + ->willReturn( $blobStore ); + + $this->setService( 'BlobStoreFactory', $factory ); + + $this->testNewMutableRevisionFromArray( $array ); + } + } -- 2.20.1