From: daniel <daniel.kinzler@wikimedia.de>
Date: Tue, 21 Aug 2012 10:48:29 +0000 (+0200)
Subject: Revision::getContent must return clone if mutable
X-Git-Tag: 1.31.0-rc.0~22097^2^2~54^2
X-Git-Url: https://git.cyclocoop.org/%242?a=commitdiff_plain;h=266b66c899094f5fd12a8456da6a26363832dfb3;p=lhc%2Fweb%2Fwiklou.git

Revision::getContent must return clone if mutable

Revision::getContent must return a cloned instance of the Content object
if the Content object is mutable to avoid confusion.

Content::copy is used to achieve this, which is specified to return $this
for immutable Content.

Change-Id: Iace17b6ae8aa85a3500624441b69bc067c1ade00
---

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 9c18cedba1..6b3b27aa9e 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -293,6 +293,7 @@ $wgAutoloadLocalClasses = array(
 	'AbstractContent' => 'includes/Content.php',
 	'ContentHandler' => 'includes/ContentHandler.php',
 	'CssContent' => 'includes/Content.php',
+	'TextContentHandler' => 'includes/ContentHandler.php',
 	'CssContentHandler' => 'includes/ContentHandler.php',
 	'JavaScriptContent' => 'includes/Content.php',
 	'JavaScriptContentHandler' => 'includes/ContentHandler.php',
diff --git a/includes/Revision.php b/includes/Revision.php
index 30ffbea35a..d5d1e0bb9c 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -970,7 +970,7 @@ class Revision implements IDBAccessObject {
 			$this->mContent = is_null( $this->mText ) ? null : $handler->unserializeContent( $this->mText, $format );
 		}
 
-		return $this->mContent;
+		return $this->mContent->copy(); // NOTE: copy() will return $this for immutable content objects
 	}
 
 	/**
diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php
index f9a400cdd3..5a983555c0 100644
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -31,6 +31,7 @@ class RevisionTest extends MediaWikiTestCase {
 
 		$wgNamespaceContentModels[ 12312 ] = "testing";
 		$wgContentHandlers[ "testing" ] = 'DummyContentHandlerForTesting';
+		$wgContentHandlers[ "RevisionTestModifyableContent" ] = 'RevisionTestModifyableContentHandler';
 
 		MWNamespace::getCanonicalNamespaces( true ); # reset namespace cache
 		$wgContLang->resetNamespaces(); # reset namespace cache
@@ -346,6 +347,87 @@ class RevisionTest extends MediaWikiTestCase {
 		$this->assertEquals( CONTENT_MODEL_JAVASCRIPT, $rev->getContentModel() );
 	}
 
+	/**
+	 * Tests whether $rev->getContent() returns a clone when needed.
+	 *
+	 * @group Database
+	 */
+	function testGetContentClone( ) {
+		$content = new RevisionTestModifyableContent( "foo" );
+
+		$rev = new Revision(
+			array(
+				'id'         => 42,
+				'page'       => 23,
+				'title'      => Title::newFromText( "testGetContentClone_dummy" ),
+
+				'content'    => $content,
+				'length'     => $content->getSize(),
+				'comment'    => "testing",
+				'minor_edit' => false,
+			)
+		);
+
+		$content = $rev->getContent( Revision::RAW );
+		$content->setText( "bar" );
+
+		$content2 = $rev->getContent( Revision::RAW );
+		$this->assertNotSame( $content, $content2, "expected a clone" ); // content is mutable, expect clone
+		$this->assertEquals( "foo", $content2->getText() ); // clone should contain the original text
+
+		$content2->setText( "bla bla" );
+		$this->assertEquals( "bar", $content->getText() ); // clones should be independent
+	}
+
+
+	/**
+	 * Tests whether $rev->getContent() returns the same object repeatedly if appropriate.
+	 *
+	 * @group Database
+	 */
+	function testGetContentUncloned() {
+		$rev = $this->newTestRevision( "hello", "testGetContentUncloned_dummy", CONTENT_MODEL_WIKITEXT );
+		$content = $rev->getContent( Revision::RAW );
+		$content2 = $rev->getContent( Revision::RAW );
+
+		// for immutable content like wikitext, this should be the same object
+		$this->assertSame( $content, $content2 );
+	}
+
+}
+
+class RevisionTestModifyableContent extends TextContent {
+	public function __construct( $text ) {
+		parent::__construct( $text, "RevisionTestModifyableContent" );
+	}
+
+	public function copy( ) {
+		return new RevisionTestModifyableContent( $this->mText );
+	}
+
+	public function getText() {
+		return $this->mText;
+	}
+
+	public function setText( $text ) {
+		$this->mText = $text;
+	}
+
 }
 
+class RevisionTestModifyableContentHandler extends TextContentHandler {
+
+	public function __construct( ) {
+		parent::__construct( "RevisionTestModifyableContent", array( CONTENT_FORMAT_TEXT ) );
+	}
 
+	public function unserializeContent( $text, $format = null ) {
+		$this->checkFormat( $format );
+
+		return new RevisionTestModifyableContent( $text );
+	}
+
+	public function makeEmptyContent() {
+		return new RevisionTestModifyableContent( '' );
+	}
+}