From 90163adf71db6b2ac7fcbd0cecb4f7c8e4e0d5ee Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aaron@users.mediawiki.org>
Date: Tue, 18 Mar 2008 19:37:03 +0000
Subject: [PATCH] *Return false when a user cannot see a field of a revision,
 rather than "", which could actually be valid *Remove now unneeded perm
 checks in diffEng *Fix unsafe cache leak for diffs (bug 9432)

---
 includes/DifferenceEngine.php | 32 +++++++++++++++++++-------------
 includes/Revision.php         |  8 ++++----
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/includes/DifferenceEngine.php b/includes/DifferenceEngine.php
index 48a8226ed0..1f45dba778 100644
--- a/includes/DifferenceEngine.php
+++ b/includes/DifferenceEngine.php
@@ -424,7 +424,13 @@ CONTROL;
 		global $wgMemc;
 		$fname = 'DifferenceEngine::getDiffBody';
 		wfProfileIn( $fname );
-		
+		// Should part of this diff be hidden?
+		$deleted = false;
+		if ( $this->mOldRev && $this->mOldRev->isDeleted(Revision::DELETED_TEXT) ) {
+			$deleted = true;
+		} else if ( $this->mNewRev && $this->mNewRev->isDeleted(Revision::DELETED_TEXT) ) {
+			$deleted = true;
+		}
 		// Cacheable?
 		$key = false;
 		if ( $this->mOldid && $this->mNewid ) {
@@ -433,11 +439,17 @@ CONTROL;
 			if ( !$this->mRefreshCache ) {
 				$difftext = $wgMemc->get( $key );
 				if ( $difftext ) {
-					wfIncrStats( 'diff_cache_hit' );
-					$difftext = $this->localiseLineNumbers( $difftext );
-					$difftext .= "\n<!-- diff cache key $key -->\n";
-					wfProfileOut( $fname );
-					return $difftext;
+					// If this diff should be hidden, kill the cache!
+					if( $deleted ) {
+						$wgMemc->delete( $key );
+						$difftext = false;
+					} else {
+						wfIncrStats( 'diff_cache_hit' );
+						$difftext = $this->localiseLineNumbers( $difftext );
+						$difftext .= "\n<!-- diff cache key $key -->\n";
+						wfProfileOut( $fname );
+						return $difftext;
+					}
 				}
 			} // don't try to load but save the result
 		}
@@ -446,19 +458,13 @@ CONTROL;
 		if ( !$this->loadText() ) {
 			wfProfileOut( $fname );
 			return false;
-		} else if ( $this->mOldRev && !$this->mOldRev->userCan(Revision::DELETED_TEXT) ) {
-		  	return '';
-		} else if ( $this->mNewRev && !$this->mNewRev->userCan(Revision::DELETED_TEXT) ) {
-		  	return '';
 		}
 
 		$difftext = $this->generateDiffBody( $this->mOldtext, $this->mNewtext );
 		
 		// Save to cache for 7 days
 		// Only do this for public revs, otherwise an admin can view the diff and a non-admin can nab it!
-		if ( $this->mOldRev && $this->mOldRev->isDeleted(Revision::DELETED_TEXT) ) {
-			wfIncrStats( 'diff_uncacheable' );
-		} else if ( $this->mNewRev && $this->mNewRev->isDeleted(Revision::DELETED_TEXT) ) {
+		if ( $deleted ) {
 			wfIncrStats( 'diff_uncacheable' );
 		} else if ( $key !== false && $difftext !== false ) {
 			wfIncrStats( 'diff_cache_miss' );
diff --git a/includes/Revision.php b/includes/Revision.php
index 05a4a68ade..4d872559d5 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -421,7 +421,7 @@ class Revision {
 	 */
 	function getUserText() {
 		if( $this->isDeleted( self::DELETED_USER ) ) {
-			return "";
+			return false;
 		} else {
 			return $this->mUserText;
 		}
@@ -441,7 +441,7 @@ class Revision {
 	 */
 	function getComment() {
 		if( $this->isDeleted( self::DELETED_COMMENT ) ) {
-			return "";
+			return false;
 		} else {
 			return $this->mComment;
 		}
@@ -476,7 +476,7 @@ class Revision {
 	 */
 	function getText() {
 		if( $this->isDeleted( self::DELETED_TEXT ) ) {
-			return "";
+			return false;
 		} else {
 			return $this->getRawText();
 		}
@@ -500,7 +500,7 @@ class Revision {
 	 */
 	function revText() {
 		if( !$this->userCan( self::DELETED_TEXT ) ) {
-			return "";
+			return false;
 		} else {
 			return $this->getRawText();
 		}
-- 
2.20.1