Make WikiPage::doDeleteArticle more robust
authordaniel <daniel.kinzler@wikimedia.de>
Sat, 25 Jun 2016 16:25:13 +0000 (18:25 +0200)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 29 Aug 2016 23:18:41 +0000 (23:18 +0000)
When it becomes impossible to load the content of a page due to some
error or misconfiguration, we still want to be able to delete that
page. This change makes WikiPage::doDeleteArticle more robust by catching
any exceptions that may be thrown while trying to load the page content
during the deletion process.

See T128466 for context.

Change-Id: I19f2d16850a3c1af5b504a70a27b9bf1330bc68d

docs/hooks.txt
includes/page/WikiPage.php

index c0c01f4..a1c30eb 100644 (file)
@@ -606,7 +606,7 @@ $outputPage: OutputPage that can be used to append the output.
 &$user: the user that deleted the article
 $reason: the reason the article was deleted
 $id: id of the article that was deleted
-$content: the Content of the deleted page
+$content: the Content of the deleted page (or null, when deleting a broken page)
 $logEntry: the ManualLogEntry used to record the deletion
 
 'ArticleEditUpdateNewTalk': Before updating user_newtalk when a user talk page
@@ -3621,7 +3621,8 @@ a page is deleted. Called in WikiPage::getDeletionUpdates(). Note that updates
 specific to a content model should be provided by the respective Content's
 getDeletionUpdates() method.
 $page: the WikiPage
-$content: the Content to generate updates for
+$content: the Content to generate updates for (or null, if the Content could not be loaded
+due to an error)
 &$updates: the array of DataUpdate objects. Hook function may want to add to it.
 
 'XmlDumpWriterOpenPage': Called at the end of XmlDumpWriter::openPage, to allow
index a416d56..ba20c10 100644 (file)
@@ -2848,7 +2848,14 @@ class WikiPage implements Page, IDBAccessObject {
                // unless they actually try to catch exceptions (which is rare).
 
                // we need to remember the old content so we can use it to generate all deletion updates.
-               $content = $this->getContent( Revision::RAW );
+               try {
+                       $content = $this->getContent( Revision::RAW );
+               } catch ( Exception $ex ) {
+                       wfLogWarning( __METHOD__ . ': failed to load content during deletion! '
+                               . $ex->getMessage() );
+
+                       $content = null;
+               }
 
                // Bitfields to further suppress the content
                if ( $suppress ) {
@@ -2982,8 +2989,16 @@ class WikiPage implements Page, IDBAccessObject {
         *   may already return null when the page proper was deleted.
         */
        public function doDeleteUpdates( $id, Content $content = null ) {
+               try {
+                       $countable = $this->isCountable();
+               } catch ( Exception $ex ) {
+                       // fallback for deleting broken pages for which we cannot load the content for
+                       // some reason. Note that doDeleteArticleReal() already logged this problem.
+                       $countable = false;
+               }
+
                // Update site status
-               DeferredUpdates::addUpdate( new SiteStatsUpdate( 0, 1, - (int)$this->isCountable(), -1 ) );
+               DeferredUpdates::addUpdate( new SiteStatsUpdate( 0, 1, - (int)$countable, -1 ) );
 
                // Delete pagelinks, update secondary indexes, etc
                $updates = $this->getDeletionUpdates( $content );
@@ -3575,7 +3590,14 @@ class WikiPage implements Page, IDBAccessObject {
                if ( !$content ) {
                        // load content object, which may be used to determine the necessary updates.
                        // XXX: the content may not be needed to determine the updates.
-                       $content = $this->getContent( Revision::RAW );
+                       try {
+                               $content = $this->getContent( Revision::RAW );
+                       } catch ( Exception $ex ) {
+                               // If we can't load the content, something is wrong. Perhaps that's why
+                               // the user is trying to delete the page, so let's not fail in that case.
+                               // Note that doDeleteArticleReal() will already have logged an issue with
+                               // loading the content.
+                       }
                }
 
                if ( !$content ) {