Beware that getContent() may return null

Before the introduction of the content handler, missing content was
signified by getText() returning null instead of a string. null will
work much like an empty string in most contexts, so in many places,
it was not checked explcitely whether the conent was null.

Now, when getContent() returns null, this often caused a fatal error,
because the code would access whatever getContent() returned as an object,
without checking whether it was null (because no such check was performed
previously, when the content was represented as a string).

This check introduces explicite checks for getContent() returning null
in the most essential core classes.

Change-Id: I551a90b0b67b8edc7570ca5d252ecc1de903f097

diff --git a/includes/Article.php b/includes/Article.php
index 0eb0c68..8403bc6 100644 (file)
--- a/includes/Article.php
+++ b/includes/Article.php
@@ -688,7 +688,7 @@ class Article extends Page {
                                                $outputDone = true;
                                        } else {
                                                $content = $this->getContentObject();
-                                               $rt = $content->getRedirectChain();
+                                               $rt = $content ? $content->getRedirectChain() : null;
                                                if ( $rt ) {
                                                        wfDebug( __METHOD__ . ": showing redirect=no page\n" );
                                                        # Viewing a redirect page (e.g. with parameter redirect=no)
@@ -842,10 +842,14 @@ class Article extends Page {
                                'clearyourcache' );
                }
 
-               // Give hooks a chance to customise the output
-               if ( ContentHandler::runLegacyHooks( 'ShowRawCssJs', array( $this->fetchContentObject(), $this->getTitle(), $outputPage ) ) ) {
-                       $po = $this->mContentObject->getParserOutput( $this->getTitle() );
-                       $outputPage->addHTML( $po->getText() );
+               $this->fetchContentObject();
+
+               if ( $this->mContentObject ) {
+                       // Give hooks a chance to customise the output
+                       if ( ContentHandler::runLegacyHooks( 'ShowRawCssJs', array( $this->mContentObject, $this->getTitle(), $outputPage ) ) ) {
+                               $po = $this->mContentObject->getParserOutput( $this->getTitle() );
+                               $outputPage->addHTML( $po->getText() );
+                       }
                }
        }
 

diff --git a/includes/Revision.php b/includes/Revision.php
index 3de8303..df02b16 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -999,7 +999,8 @@ class Revision implements IDBAccessObject {
                                $this->mText = $this->loadText();
                        }
 
-                       $this->mContent = is_null( $this->mText ) ? null : $handler->unserializeContent( $this->mText, $format );
+                       $this->mContent = ( $this->mText === null || $this->mText === false ) ? null
+                               : $handler->unserializeContent( $this->mText, $format );
                }
 
                return $this->mContent->copy(); // NOTE: copy() will return $this for immutable content objects
@@ -1377,7 +1378,7 @@ class Revision implements IDBAccessObject {
 
                $content = $this->getContent( Revision::RAW );
 
-               if ( !$content->isValid() ) {
+               if ( !$content || !$content->isValid() ) {
                        $t = $title->getPrefixedDBkey();
 
                        throw new MWException( "Content of $t is not valid! Content model is $model" );
@@ -1397,7 +1398,7 @@ class Revision implements IDBAccessObject {
         * Lazy-load the revision's text.
         * Currently hardcoded to the 'text' table storage engine.
         *
-        * @return String
+        * @return String|boolean the revision text, or false on failure
         */
        protected function loadText() {
                wfProfileIn( __METHOD__ );

diff --git a/includes/Title.php b/includes/Title.php
index c1e2ccf..0f02dc7 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -3981,7 +3981,7 @@ class Title {
                $content = $rev->getContent();
                # Does the redirect point to the source?
                # Or is it a broken self-redirect, usually caused by namespace collisions?
-               $redirTitle = $content->getRedirectTarget();
+               $redirTitle = $content ? $content->getRedirectTarget() : null;
 
                if ( $redirTitle ) {
                        if ( $redirTitle->getPrefixedDBkey() != $this->getPrefixedDBkey() &&

diff --git a/includes/WikiPage.php b/includes/WikiPage.php
index df3086a..a4bc6ee 100644 (file)
--- a/includes/WikiPage.php
+++ b/includes/WikiPage.php
@@ -1237,8 +1237,8 @@ class WikiPage extends Page implements IDBAccessObject {
                wfProfileIn( __METHOD__ );
 
                $content = $revision->getContent();
-               $len = $content->getSize();
-               $rt = $content->getUltimateRedirectTarget();
+               $len = $content ? $content->getSize() : 0;
+               $rt = $content ? $content->getUltimateRedirectTarget() : null;
 
                $conditions = array( 'page_id' => $this->getId() );
 
@@ -1469,11 +1469,6 @@ class WikiPage extends Page implements IDBAccessObject {
                        // Bug 30711: always use current version when adding a new section
                        if ( is_null( $edittime ) || $section == 'new' ) {
                                $oldContent = $this->getContent();
-                               if ( ! $oldContent ) {
-                                       wfDebug( __METHOD__ . ": no page text\n" );
-                                       wfProfileOut( __METHOD__ );
-                                       return null;
-                               }
                        } else {
                                $dbw = wfGetDB( DB_MASTER );
                                $rev = Revision::loadFromTimestamp( $dbw, $this->mTitle, $edittime );
@@ -1488,6 +1483,13 @@ class WikiPage extends Page implements IDBAccessObject {
                                $oldContent = $rev->getContent();
                        }
 
+                       if ( ! $oldContent ) {
+                               wfDebug( __METHOD__ . ": no page text\n" );
+                               wfProfileOut( __METHOD__ );
+                               return null;
+                       }
+
+                       //FIXME: $oldContent might be null?
                        $newContent = $oldContent->replaceSection( $section, $sectionContent, $sectionTitle );
                }
 
@@ -1852,6 +1854,10 @@ class WikiPage extends Page implements IDBAccessObject {
                        # Bug 37225: use accessor to get the text as Revision may trim it
                        $content = $revision->getContent(); // sanity; get normalized version
 
+                       if ( $content ) {
+                               $newsize = $content->getSize();
+                       }
+
                        # Update the page record with revision data
                        $this->updateRevisionOn( $dbw, $revision, 0 );
 
@@ -1864,7 +1870,7 @@ class WikiPage extends Page implements IDBAccessObject {
                                        $this->mTitle->getUserPermissionsErrors( 'autopatrol', $user ) );
                                # Add RC row to the DB
                                $rc = RecentChange::notifyNew( $now, $this->mTitle, $isminor, $user, $summary, $bot,
-                                       '', $content->getSize(), $revisionId, $patrolled );
+                                       '', $newsize, $revisionId, $patrolled );
 
                                # Log auto-patrolled edits
                                if ( $patrolled ) {
@@ -1956,7 +1962,7 @@ class WikiPage extends Page implements IDBAccessObject {
         * @since 1.21
         */
        public function prepareContentForEdit( Content $content, $revid = null, User $user = null, $serialization_format = null ) {
-               global $wgParser, $wgContLang, $wgUser;
+               global $wgContLang, $wgUser;
                $user = is_null( $user ) ? $wgUser : $user;
                //XXX: check $user->getId() here???
 
@@ -1977,23 +1983,21 @@ class WikiPage extends Page implements IDBAccessObject {
                $edit = (object)array();
                $edit->revid = $revid;
 
-               $edit->pstContent = $content->preSaveTransform( $this->mTitle, $user, $popts );
-               $edit->pst = $edit->pstContent->serialize( $serialization_format ); #XXX: do we need this??
-               $edit->format = $serialization_format;
+               $edit->pstContent = $content ? $content->preSaveTransform( $this->mTitle, $user, $popts ) : null;
 
+               $edit->format = $serialization_format;
                $edit->popts = $this->makeParserOptions( 'canonical' );
-
-               $edit->output = $edit->pstContent->getParserOutput( $this->mTitle, $revid, $edit->popts );
+               $edit->output = $edit->pstContent ? $edit->pstContent->getParserOutput( $this->mTitle, $revid, $edit->popts ) : null;
 
                $edit->newContent = $content;
                $edit->oldContent = $this->getContent( Revision::RAW );
 
                #NOTE: B/C for hooks! don't use these fields!
-               $edit->newText = ContentHandler::getContentText( $edit->newContent );
+               $edit->newText = $edit->newContent ? ContentHandler::getContentText( $edit->newContent ) : '';
                $edit->oldText = $edit->oldContent ? ContentHandler::getContentText( $edit->oldContent ) : '';
+               $edit->pst = $edit->pstContent ? $edit->pstContent->serialize( $serialization_format ) : '';
 
                $this->mPreparedEdit = $edit;
-
                return $edit;
        }
 
@@ -2038,8 +2042,10 @@ class WikiPage extends Page implements IDBAccessObject {
                }
 
                # Update the links tables and other secondary data
-               $updates = $content->getSecondaryDataUpdates( $this->getTitle(), null, true, $editInfo->output );
-               DataUpdate::runUpdates( $updates );
+               if ( $content ) {
+                       $updates = $content->getSecondaryDataUpdates( $this->getTitle(), null, true, $editInfo->output );
+                       DataUpdate::runUpdates( $updates );
+               }
 
                wfRunHooks( 'ArticleEditUpdates', array( &$this, &$editInfo, $options['changed'] ) );
 
@@ -2111,7 +2117,7 @@ class WikiPage extends Page implements IDBAccessObject {
 
                if ( $this->mTitle->getNamespace() == NS_MEDIAWIKI ) {
                        #XXX: could skip pseudo-messages like js/css here, based on content model.
-                       $msgtext = $content->getWikitextForTransclusion();
+                       $msgtext = $content ? $content->getWikitextForTransclusion() : null;
                        if ( $msgtext === false || $msgtext === null ) $msgtext = '';
 
                        MessageCache::singleton()->replace( $shortTitle, $msgtext );

diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php
index f3dc5a3..0295f77 100644 (file)
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -522,8 +522,10 @@ class DifferenceEngine extends ContextSource {
                                if ( ContentHandler::runLegacyHooks( 'ShowRawCssJs', array( $this->mNewContent, $this->mNewPage, $out ) ) ) {
                                        // NOTE: deprecated hook, B/C only
                                        // use the content object's own rendering
-                                       $po = $this->mNewRev->getContent()->getParserOutput( $this->mNewRev->getTitle(), $this->mNewRev->getId() );
-                                       $out->addHTML( $po->getText() );
+                                       $cnt = $this->mNewRev->getContent();
+                                       $po = $cnt ? $cnt->getParserOutput( $this->mNewRev->getTitle(), $this->mNewRev->getId() ) : null;
+                                       $txt = $po ? $po->getText() : '';
+                                       $out->addHTML( $txt );
                                }
                        } elseif( !wfRunHooks( 'ArticleContentViewCustom', array( $this->mNewContent, $this->mNewPage, $out ) ) ) {
                                // Handled by extension
@@ -545,7 +547,7 @@ class DifferenceEngine extends ContextSource {
                                $parserOutput = $this->getParserOutput( $wikiPage, $this->mNewRev );
 
                                # Also try to load it as a redirect
-                               $rt = $this->mNewContent->getRedirectTarget();
+                               $rt = $this->mNewContent ? $this->mNewContent->getRedirectTarget() : null;
 
                                if ( $rt ) {
                                        $article = Article::newFromTitle( $this->mNewPage, $this->getContext() );
@@ -1169,13 +1171,13 @@ class DifferenceEngine extends ContextSource {
                }
                if ( $this->mOldRev ) {
                        $this->mOldContent = $this->mOldRev->getContent( Revision::FOR_THIS_USER, $this->getUser() );
-                       if ( $this->mOldContent === false ) {
+                       if ( $this->mOldContent === null ) {
                                return false;
                        }
                }
                if ( $this->mNewRev ) {
                        $this->mNewContent = $this->mNewRev->getContent( Revision::FOR_THIS_USER, $this->getUser() );
-                       if ( $this->mNewContent === false ) {
+                       if ( $this->mNewContent === null ) {
                                return false;
                        }
                }

diff --git a/includes/job/jobs/DoubleRedirectJob.php b/includes/job/jobs/DoubleRedirectJob.php
index b1b96b6..ddd4fcc 100644 (file)
--- a/includes/job/jobs/DoubleRedirectJob.php
+++ b/includes/job/jobs/DoubleRedirectJob.php
@@ -94,7 +94,7 @@ class DoubleRedirectJob extends Job {
                        return true;
                }
                $content = $targetRev->getContent();
-               $currentDest = $content->getRedirectTarget();
+               $currentDest = $content ? $content->getRedirectTarget() : null;
                if ( !$currentDest || !$currentDest->equals( $this->redirTitle ) ) {
                        wfDebug( __METHOD__.": Redirect has changed since the job was queued\n" );
                        return true;

diff --git a/includes/job/jobs/RefreshLinksJob.php b/includes/job/jobs/RefreshLinksJob.php
index a29f29f..384244f 100644 (file)
--- a/includes/job/jobs/RefreshLinksJob.php
+++ b/includes/job/jobs/RefreshLinksJob.php
@@ -70,18 +70,17 @@ class RefreshLinksJob extends Job {
        }
 
        public static function runForTitleInternal( Title $title, Revision $revision, $fname ) {
-               global $wgContLang;
+               wfProfileIn( $fname );
+               $content = $revision->getContent( Revision::RAW );
 
-               wfProfileIn( $fname . '-parse' );
-               $options = ParserOptions::newFromUserAndLang( new User, $wgContLang );
-               $content = $revision->getContent();
-               $parserOutput = $content->getParserOutput( $title, $revision->getId(), $options, false );
-               wfProfileOut( $fname . '-parse' );
+               if ( !$content ) {
+                       // if there is no content, pretend the content is empty
+                       $content = $revision->getContentHandler()->makeEmptyContent();
+               }
 
-               wfProfileIn( $fname . '-update' );
-               $updates = $content->getSecondaryDataUpdates( $title, null, false, $parserOutput  );
+               $updates = $content->getSecondaryDataUpdates( $title, null, false );
                DataUpdate::runUpdates( $updates );
-               wfProfileOut( $fname . '-update' );
+               wfProfileOut( $fname );
        }
 }
 

diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index b31288f..9dad1e5 100644 (file)
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -3617,7 +3617,7 @@ class Parser {
 
                        if ( $rev ) {
                                $content = $rev->getContent();
-                               $text = $content->getWikitextForTransclusion();
+                               $text = $content ? $content->getWikitextForTransclusion() : null;
 
                                if ( $text === false || $text === null ) {
                                        $text = false;

diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php
index 28c3426..1e61a3e 100644 (file)
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -77,10 +77,16 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
                }
 
                $content = $revision->getContent( Revision::RAW );
+
+               if ( !$content ) {
+                       wfDebug( __METHOD__ . "failed to load content of JS/CSS page!\n" );
+                       return null;
+               }
+
                $model = $content->getModel();
 
                if ( $model !== CONTENT_MODEL_CSS && $model !== CONTENT_MODEL_JAVASCRIPT ) {
-                       wfDebug( __METHOD__ . "bad content model #$model for JS/CSS page!\n" );
+                       wfDebug( __METHOD__ . "bad content model $model for JS/CSS page!\n" );
                        return null;
                }
 

diff --git a/includes/search/SearchEngine.php b/includes/search/SearchEngine.php
index f8f5fa5..ec542a6 100644 (file)
--- a/includes/search/SearchEngine.php
+++ b/includes/search/SearchEngine.php
@@ -795,7 +795,7 @@ class SearchResult {
                                //TODO: if we could plug in some code that knows about special content models *and* about
                                //      special features of the search engine, the search could benefit.
                                $content = $this->mRevision->getContent();
-                               $this->mText = $content->getTextForSearchIndex();
+                               $this->mText = $content ? $content->getTextForSearchIndex() : '';
                        } else { // TODO: can we fetch raw wikitext for commons images?
                                $this->mText = '';
                        }