*Return false when a user cannot see a field of a revision, rather than "", which...

*Remove now unneeded perm checks in diffEng
*Fix unsafe cache leak for diffs (bug 9432)

diff --git a/includes/DifferenceEngine.php b/includes/DifferenceEngine.php
index 48a8226..1f45dba 100644 (file)
--- 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 05a4a68..4d87255 100644 (file)
--- 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();
                }