Prevent fatal PHP errors when PageRestriction::getTitle() returns null.

Update usages of PageRestriction::getTitle() to handle a null response.

Bug: T214763
Change-Id: Ied33e2c3c9442c47ae8084a97bb0921869fb9d49

diff --git a/includes/api/ApiQueryBlocks.php b/includes/api/ApiQueryBlocks.php
index 95f8cda..8aff2aa 100644 (file)
--- a/includes/api/ApiQueryBlocks.php
+++ b/includes/api/ApiQueryBlocks.php
@@ -305,7 +305,9 @@ class ApiQueryBlocks extends ApiQueryBase {
                        switch ( $restriction->getType() ) {
                                case 'page':
                                        $value = [ 'id' => $restriction->getValue() ];
-                                       self::addTitleInfo( $value, $restriction->getTitle() );
+                                       if ( $restriction->getTitle() ) {
+                                               self::addTitleInfo( $value, $restriction->getTitle() );
+                                       }
                                        break;
                                default:
                                        $value = $restriction->getValue();

diff --git a/includes/block/Restriction/PageRestriction.php b/includes/block/Restriction/PageRestriction.php
index bf7ef04..5d3fabb 100644 (file)
--- a/includes/block/Restriction/PageRestriction.php
+++ b/includes/block/Restriction/PageRestriction.php
@@ -35,7 +35,7 @@ class PageRestriction extends AbstractRestriction {
        const TYPE_ID = 1;
 
        /**
-        * @var \Title
+        * @var \Title|bool
         */
        protected $title;
 
@@ -43,6 +43,10 @@ class PageRestriction extends AbstractRestriction {
         * {@inheritdoc}
         */
        public function matches( \Title $title ) {
+               if ( !$this->getTitle() ) {
+                       return false;
+               }
+
                return $title->equals( $this->getTitle() );
        }
 
@@ -66,11 +70,17 @@ class PageRestriction extends AbstractRestriction {
         * @return \Title|null
         */
        public function getTitle() {
-               if ( !$this->title ) {
+               if ( $this->title === null ) {
                        $this->title = \Title::newFromID( $this->value );
+
+                       // If the title does not exist, set to false to prevent multiple database
+                       // queries.
+                       if ( $this->title === null ) {
+                               $this->title = false;
+                       }
                }
 
-               return $this->title;
+               return $this->title ?? null;
        }
 
        /**

diff --git a/includes/specials/SpecialBlock.php b/includes/specials/SpecialBlock.php
index bab3c8c..287dbb3 100644 (file)
--- a/includes/specials/SpecialBlock.php
+++ b/includes/specials/SpecialBlock.php
@@ -406,7 +406,9 @@ class SpecialBlock extends FormSpecialPage {
                                foreach ( $block->getRestrictions() as $restriction ) {
                                        switch ( $restriction->getType() ) {
                                                case PageRestriction::TYPE:
-                                                       $pageRestrictions[] = $restriction->getTitle()->getPrefixedText();
+                                                       if ( $restriction->getTitle() ) {
+                                                               $pageRestrictions[] = $restriction->getTitle()->getPrefixedText();
+                                                       }
                                                        break;
                                                case NamespaceRestriction::TYPE:
                                                        $namespaceRestrictions[] = $restriction->getValue();

diff --git a/includes/specials/pagers/BlockListPager.php b/includes/specials/pagers/BlockListPager.php
index 8fc586b..69dce53 100644 (file)
--- a/includes/specials/pagers/BlockListPager.php
+++ b/includes/specials/pagers/BlockListPager.php
@@ -262,11 +262,13 @@ class BlockListPager extends TablePager {
 
                        switch ( $restriction->getType() ) {
                                case PageRestriction::TYPE:
-                                       $items[$restriction->getType()][] = HTML::rawElement(
-                                               'li',
-                                               [],
-                                               Linker::link( $restriction->getTitle() )
-                                       );
+                                       if ( $restriction->getTitle() ) {
+                                               $items[$restriction->getType()][] = HTML::rawElement(
+                                                       'li',
+                                                       [],
+                                                       Linker::link( $restriction->getTitle() )
+                                               );
+                                       }
                                        break;
                                case NamespaceRestriction::TYPE:
                                        $text = $restriction->getValue() === NS_MAIN

diff --git a/tests/phpunit/includes/api/ApiQueryBlocksTest.php b/tests/phpunit/includes/api/ApiQueryBlocksTest.php
index 03198a8..6e00842 100644 (file)
--- a/tests/phpunit/includes/api/ApiQueryBlocksTest.php
+++ b/tests/phpunit/includes/api/ApiQueryBlocksTest.php
@@ -112,6 +112,12 @@ class ApiQueryBlocksTest extends ApiTestCase {
                        'ir_type' => PageRestriction::TYPE_ID,
                        'ir_value' => $pageId,
                ] );
+               // Page that has been deleted.
+               $this->db->insert( 'ipblocks_restrictions', [
+                       'ir_ipb_id' => $block->getId(),
+                       'ir_type' => PageRestriction::TYPE_ID,
+                       'ir_value' => 999999,
+               ] );
                $this->db->insert( 'ipblocks_restrictions', [
                        'ir_ipb_id' => $block->getId(),
                        'ir_type' => NamespaceRestriction::TYPE_ID,

diff --git a/tests/phpunit/includes/block/Restriction/PageRestrictionTest.php b/tests/phpunit/includes/block/Restriction/PageRestrictionTest.php
index 95cb3b7..5882279 100644 (file)
--- a/tests/phpunit/includes/block/Restriction/PageRestrictionTest.php
+++ b/tests/phpunit/includes/block/Restriction/PageRestrictionTest.php
@@ -20,6 +20,11 @@ class PageRestrictionTest extends RestrictionTestCase {
 
                $page = $this->getExistingTestPage( 'Mars' );
                $this->assertFalse( $restriction->matches( $page->getTitle() ) );
+
+               // Deleted page.
+               $restriction = new $class( 2, 99999 );
+               $page = $this->getExistingTestPage( 'Saturn' );
+               $this->assertFalse( $restriction->matches( $page->getTitle() ) );
        }
 
        public function testGetType() {

diff --git a/tests/phpunit/includes/specials/SpecialBlockTest.php b/tests/phpunit/includes/specials/SpecialBlockTest.php
index 55a8b66..3d88bc2 100644 (file)
--- a/tests/phpunit/includes/specials/SpecialBlockTest.php
+++ b/tests/phpunit/includes/specials/SpecialBlockTest.php
@@ -119,6 +119,8 @@ class SpecialBlockTest extends SpecialPageTestBase {
                        new PageRestriction( 0, $pageSaturn->getId() ),
                        new PageRestriction( 0, $pageMars->getId() ),
                        new NamespaceRestriction( 0, NS_TALK ),
+                       // Deleted page.
+                       new PageRestriction( 0, 999999 ),
                ] );
 
                $block->insert();

diff --git a/tests/phpunit/includes/specials/pagers/BlockListPagerTest.php b/tests/phpunit/includes/specials/pagers/BlockListPagerTest.php
index 570291c..6ffd03a 100644 (file)
--- a/tests/phpunit/includes/specials/pagers/BlockListPagerTest.php
+++ b/tests/phpunit/includes/specials/pagers/BlockListPagerTest.php
@@ -140,7 +140,9 @@ class BlockListPagerTest extends MediaWikiTestCase {
 
                $restrictions = [
                        ( new PageRestriction( 0, $pageId ) )->setTitle( $title ),
-                       new NamespaceRestriction( 0, NS_MAIN )
+                       new NamespaceRestriction( 0, NS_MAIN ),
+                       // Deleted page.
+                       new PageRestriction( 0, 999999 ),
                ];
 
                $wrappedPager = TestingAccessWrapper::newFromObject( $pager );