Fix capitalization in ApiQueryBase::titlePartToKey()
authorbtongminh <bryan.tongminh@gmail.com>
Sun, 17 Nov 2013 17:52:54 +0000 (18:52 +0100)
committerBrad Jorsch <bjorsch@wikimedia.org>
Mon, 23 Dec 2013 17:53:43 +0000 (12:53 -0500)
ApiQueryBase::titlePartToKey now allows an extra parameter that
indicates the namespace in order to properly capitalize the title part.

This allows list=allcategories, list=allimages, list=alllinks,
list=allpages, list=deletedrevs and list=filearchive to
handle case-sensitivity properly for all parameters.

Bug: 25702
Change-Id: Iaa5a71ec536f3716f54bc84b39f645545dfd8660

12 files changed:
RELEASE-NOTES-1.23
includes/api/ApiQueryAllCategories.php
includes/api/ApiQueryAllImages.php
includes/api/ApiQueryAllLinks.php
includes/api/ApiQueryAllPages.php
includes/api/ApiQueryBase.php
includes/api/ApiQueryDeletedrevs.php
includes/api/ApiQueryFilearchive.php
tests/TestsAutoLoader.php
tests/phpunit/includes/api/ApiQueryAllPagesTest.php [new file with mode: 0644]
tests/phpunit/includes/api/MockApiQueryBase.php [new file with mode: 0644]
tests/phpunit/includes/api/query/ApiQueryTest.php

index 3e40f7f..416a7fc 100644 (file)
@@ -85,6 +85,11 @@ production.
   MediaWiki 1.24.
 * action=parse now has disabletoc flag to disable table of contents in output.
 * SpecialRecentChanges::feedSetup() was removed.
+* (bug 25702) list=allcategories, list=allimages, list=alllinks, list=allpages,
+  list=deletedrevs and list=filearchive did not handle case-sensitivity
+  properly for all parameters.
+* ApiQueryBase::titlePartToKey allows an extra parameter that indicates the
+  namespace in order to properly capitalize the title part.
 
 === Languages updated in 1.23 ===
 
index d0ab59e..6bf8075 100644 (file)
@@ -67,8 +67,8 @@ class ApiQueryAllCategories extends ApiQueryGeneratorBase {
                }
 
                $dir = ( $params['dir'] == 'descending' ? 'older' : 'newer' );
-               $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) );
-               $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) );
+               $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], NS_CATEGORY ) );
+               $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], NS_CATEGORY ) );
                $this->addWhereRange( 'cat_title', $dir, $from, $to );
 
                $min = $params['min'];
@@ -80,8 +80,9 @@ class ApiQueryAllCategories extends ApiQueryGeneratorBase {
                }
 
                if ( isset( $params['prefix'] ) ) {
-                       $this->addWhere( 'cat_title' .
-                               $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) );
+                       $this->addWhere( 'cat_title' . $db->buildLike(
+                               $this->titlePartToKey( $params['prefix'], NS_CATEGORY ),
+                               $db->anyString() ) );
                }
 
                $this->addOption( 'LIMIT', $params['limit'] + 1 );
index 9f97cac..4095bd8 100644 (file)
@@ -132,13 +132,14 @@ class ApiQueryAllImages extends ApiQueryGeneratorBase {
                        }
 
                        // Image filters
-                       $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) );
-                       $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) );
+                       $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], NS_FILE ) );
+                       $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], NS_FILE ) );
                        $this->addWhereRange( 'img_name', ( $ascendingOrder ? 'newer' : 'older' ), $from, $to );
 
                        if ( isset( $params['prefix'] ) ) {
-                               $this->addWhere( 'img_name' .
-                                       $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) );
+                               $this->addWhere( 'img_name' . $db->buildLike(
+                                       $this->titlePartToKey( $params['prefix'], NS_FILE ),
+                                       $db->anyString() ) );
                        }
                } else {
                        // Check mutually exclusive params
index ff53d0f..5be304d 100644 (file)
@@ -148,15 +148,16 @@ class ApiQueryAllLinks extends ApiQueryGeneratorBase {
                }
 
                // 'continue' always overrides 'from'
-               $from = $continue || is_null( $params['from'] )
-                       ? null
-                       : $this->titlePartToKey( $params['from'] );
-               $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) );
+               $from = ( $continue || $params['from'] === null ? null :
+                       $this->titlePartToKey( $params['from'], $params['namespace'] ) );
+               $to = ( $params['to'] === null ? null :
+                       $this->titlePartToKey( $params['to'], $params['namespace'] ) );
                $this->addWhereRange( $pfx . $fieldTitle, 'newer', $from, $to );
 
+
                if ( isset( $params['prefix'] ) ) {
-                       $this->addWhere( $pfx . $fieldTitle .
-                               $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) );
+                       $this->addWhere( $pfx . $fieldTitle . $db->buildLike( $this->titlePartToKey(
+                               $params['prefix'], $params['namespace'] ), $db->anyString() ) );
                }
 
                $this->addFields( array( 'pl_title' => $pfx . $fieldTitle ) );
index 363d657..430dd51 100644 (file)
@@ -87,13 +87,14 @@ class ApiQueryAllPages extends ApiQueryGeneratorBase {
 
                $this->addWhereFld( 'page_namespace', $params['namespace'] );
                $dir = ( $params['dir'] == 'descending' ? 'older' : 'newer' );
-               $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) );
-               $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) );
+               $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], $params['namespace'] ) );
+               $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], $params['namespace'] ) );
                $this->addWhereRange( 'page_title', $dir, $from, $to );
 
                if ( isset( $params['prefix'] ) ) {
-                       $this->addWhere( 'page_title' .
-                               $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) );
+                       $this->addWhere( 'page_title' . $db->buildLike(
+                               $this->titlePartToKey( $params['prefix'], $params['namespace'] ),
+                               $db->anyString() ) );
                }
 
                if ( is_null( $resultPageSet ) ) {
index fcd3180..d9aacaa 100644 (file)
@@ -480,12 +480,28 @@ abstract class ApiQueryBase extends ApiBase {
        }
 
        /**
-        * An alternative to titleToKey() that doesn't trim trailing spaces
+        * An alternative to titleToKey() that doesn't trim trailing spaces, and
+        * does not mangle the input if starts with something that looks like a
+        * namespace. It is advisable to pass the namespace parameter in order to
+        * handle per-namespace capitalization settings.
         * @param string $titlePart Title part with spaces
+        * @param $defaultNamespace int Namespace to assume
         * @return string Title part with underscores
         */
-       public function titlePartToKey( $titlePart ) {
-               return substr( $this->titleToKey( $titlePart . 'x' ), 0, -1 );
+       public function titlePartToKey( $titlePart, $defaultNamespace = NS_MAIN ) {
+               $t = Title::makeTitleSafe( $defaultNamespace, $titlePart . 'x' );
+               if ( !$t ) {
+                       $this->dieUsageMsg( array( 'invalidtitle', $titlePart ) );
+               }
+               if ( $defaultNamespace != $t->getNamespace() || $t->getInterwiki() !== '' ) {
+                       // This can happen in two cases. First, if you call titlePartToKey with a title part
+                       // that looks like a namespace, but with $defaultNamespace = NS_MAIN. It would be very
+                       // difficult to handle such a case. Such cases cannot exist and are therefore treated
+                       // as invalid user input. The second case is when somebody specifies a title interwiki
+                       // prefix.
+                       $this->dieUsageMsg( array( 'invalidtitle', $titlePart ) );
+               }
+               return substr( $t->getDbKey(), 0, -1 );
        }
 
        /**
index f63e033..4e4b2cc 100644 (file)
@@ -170,13 +170,14 @@ class ApiQueryDeletedrevs extends ApiQueryBase {
                } elseif ( $mode == 'all' ) {
                        $this->addWhereFld( 'ar_namespace', $params['namespace'] );
 
-                       $from = is_null( $params['from'] ) ? null : $this->titleToKey( $params['from'] );
-                       $to = is_null( $params['to'] ) ? null : $this->titleToKey( $params['to'] );
+                       $from = $params['from'] === null ? null : $this->titlePartToKey( $params['from'], $params['namespace'] );
+                       $to = $params['to'] === null ? null : $this->titlePartToKey( $params['to'], $params['namespace'] );
                        $this->addWhereRange( 'ar_title', $dir, $from, $to );
 
                        if ( isset( $params['prefix'] ) ) {
-                               $this->addWhere( 'ar_title' .
-                                       $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) );
+                               $this->addWhere( 'ar_title' . $db->buildLike(
+                                       $this->titlePartToKey( $params['prefix'], $params['namespace'] ),
+                                       $db->anyString() ) );
                        }
                }
 
index f8f4558..e2c9964 100644 (file)
@@ -88,15 +88,16 @@ class ApiQueryFilearchive extends ApiQueryBase {
 
                // Image filters
                $dir = ( $params['dir'] == 'descending' ? 'older' : 'newer' );
-               $from = ( is_null( $params['from'] ) ? null : $this->titlePartToKey( $params['from'] ) );
+               $from = ( $params['from'] === null ? null : $this->titlePartToKey( $params['from'], NS_FILE ) );
                if ( !is_null( $params['continue'] ) ) {
                        $from = $params['continue'];
                }
-               $to = ( is_null( $params['to'] ) ? null : $this->titlePartToKey( $params['to'] ) );
+               $to = ( $params['to'] === null ? null : $this->titlePartToKey( $params['to'], NS_FILE ) );
                $this->addWhereRange( 'fa_name', $dir, $from, $to );
                if ( isset( $params['prefix'] ) ) {
-                       $this->addWhere( 'fa_name' .
-                               $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) );
+                       $this->addWhere( 'fa_name' . $db->buildLike(
+                               $this->titlePartToKey( $params['prefix'], NS_FILE ),
+                               $db->anyString() ) );
                }
 
                $sha1Set = isset( $params['sha1'] );
index 46a894c..58df552 100644 (file)
@@ -57,6 +57,7 @@ $wgAutoloadClasses += array(
        'ApiTestCase' => "$testDir/phpunit/includes/api/ApiTestCase.php",
        'ApiTestContext' => "$testDir/phpunit/includes/api/ApiTestContext.php",
        'MockApi' => "$testDir/phpunit/includes/api/MockApi.php",
+       'MockApiQueryBase' => "$testDir/phpunit/includes/api/MockApiQueryBase.php",
        'UserWrapper' => "$testDir/phpunit/includes/api/UserWrapper.php",
        'RandomImageGenerator' => "$testDir/phpunit/includes/api/RandomImageGenerator.php",
 
diff --git a/tests/phpunit/includes/api/ApiQueryAllPagesTest.php b/tests/phpunit/includes/api/ApiQueryAllPagesTest.php
new file mode 100644 (file)
index 0000000..bc08afe
--- /dev/null
@@ -0,0 +1,30 @@
+<?php
+
+/**
+ * @group API
+ * @group Database
+ * @group medium
+ */
+class ApiQueryAllPagesTest extends ApiTestCase {
+       protected function setUp() {
+               parent::setUp();
+               $this->doLogin();
+       }
+
+       function testBug25702() {
+               $title = Title::newFromText( 'Category:Template:xyz' );
+               $page = WikiPage::factory( $title );
+               $page->doEdit( 'Some text', 'inserting content' );
+
+               $result = $this->doApiRequest( array(
+                       'action' => 'query',
+                       'list' => 'allpages',
+                       'apnamespace' => NS_CATEGORY,
+                       'apprefix' => 'Template:x' ) );
+
+               $this->assertArrayHasKey( 'query', $result[0] );
+               $this->assertArrayHasKey( 'allpages', $result[0]['query'] );
+               $this->assertNotEquals( 0, count( $result[0]['query']['allpages'] ),
+                       'allpages list does not contain page Category:Template:xyz' );
+       }
+}
diff --git a/tests/phpunit/includes/api/MockApiQueryBase.php b/tests/phpunit/includes/api/MockApiQueryBase.php
new file mode 100644 (file)
index 0000000..4bede51
--- /dev/null
@@ -0,0 +1,11 @@
+<?php
+class MockApiQueryBase extends ApiQueryBase {
+       public function execute() {
+       }
+
+       public function getVersion() {
+       }
+
+       public function __construct() {
+       }
+}
index 2ec5fe3..17da9a1 100644 (file)
@@ -7,10 +7,32 @@
  * @covers ApiQuery
  */
 class ApiQueryTest extends ApiTestCase {
+       /**
+        * @var array Storage for $wgHooks
+        */
+       protected $hooks;
 
        protected function setUp() {
+               global $wgHooks;
+
                parent::setUp();
                $this->doLogin();
+
+               // Setup en: as interwiki prefix
+               $this->hooks = $wgHooks;
+               $wgHooks['InterwikiLoadPrefix'][] = function ( $prefix, &$data ) {
+                       if ( $prefix == 'en' ) {
+                               $data = array( 'iw_url' => 'wikipedia' );
+                       }
+                       return false;
+               };
+       }
+
+       protected function tearDown() {
+               global $wgHooks;
+               $wgHooks = $this->hooks;
+
+               parent::tearDown();
        }
 
        public function testTitlesGetNormalized() {
@@ -64,4 +86,38 @@ class ApiQueryTest extends ApiTestCase {
                $this->assertArrayHasKey( 'missing', $data[0]['query']['pages'][-2] );
                $this->assertArrayHasKey( 'invalid', $data[0]['query']['pages'][-1] );
        }
+
+       /**
+        * Test the ApiBase::titlePartToKey function
+        *
+        * @param string $titlePart
+        * @param int $namespace
+        * @param string $expected
+        * @param string $description
+        * @dataProvider provideTestTitlePartToKey
+        */
+       function testTitlePartToKey( $titlePart, $namespace, $expected, $expectException ) {
+               $api = new MockApiQueryBase();
+               $exceptionCaught = false;
+               try {
+                       $this->assertEquals( $expected, $api->titlePartToKey( $titlePart, $namespace ) );
+               } catch ( UsageException $e ) {
+                       $exceptionCaught = true;
+               }
+               $this->assertEquals( $expectException, $exceptionCaught,
+                       'UsageException thrown by titlePartToKey' );
+       }
+
+       function provideTestTitlePartToKey() {
+               return array(
+                       array( 'a  b  c', NS_MAIN, 'A_b_c', false ),
+                       array( 'x', NS_MAIN, 'X', false ),
+                       array( 'y ', NS_MAIN, 'Y_', false ),
+                       array( 'template:foo', NS_CATEGORY, 'Template:foo', false ),
+                       array( 'en:foo', NS_CATEGORY, 'En:foo', false ),
+                       array( "\xF7", NS_MAIN, null, true ),
+                       array( 'template:foo', NS_MAIN, null, true ),
+                       array( 'en:foo', NS_MAIN, null, true ),
+               );
+       }
 }