Fix and re-apply "RedirectSpecialPage: handle interwiki redirects"
authorGergő Tisza <gtisza@wikimedia.org>
Tue, 16 Jul 2019 13:28:28 +0000 (13:28 +0000)
committerTim Starling <tstarling@wikimedia.org>
Wed, 24 Jul 2019 03:55:49 +0000 (03:55 +0000)
This re-applies commit 41106688abbe6dfff61c5642924ced42af3f0d33
(thereby reverting commit 6c57748aeee6e4f2a197d64785102306fbd4a297)
and fixes it for local interwiki redirects by adding and using a
forcing parameter in Special:GoToInterwiki to treat local redirects
like external ones.

Bug: T227700
Change-Id: I4bc2ed998430fc2bac71baf850b8988fdb24c1ac

includes/MediaWiki.php
includes/specials/SpecialGoToInterwiki.php
tests/phpunit/includes/specials/SpecialGoToInterwikiTest.php [new file with mode: 0644]

index 69f23c1..7a6987e 100644 (file)
@@ -260,8 +260,16 @@ class MediaWiki {
                                        ) {
                                                list( , $subpage ) = $spFactory->resolveAlias( $title->getDBkey() );
                                                $target = $specialPage->getRedirect( $subpage );
-                                               // target can also be true. We let that case fall through to normal processing.
+                                               // Target can also be true. We let that case fall through to normal processing.
                                                if ( $target instanceof Title ) {
+                                                       if ( $target->isExternal() ) {
+                                                               // Handle interwiki redirects
+                                                               $target = SpecialPage::getTitleFor(
+                                                                       'GoToInterwiki',
+                                                                       'force/' . $target->getPrefixedDBkey()
+                                                               );
+                                                       }
+
                                                        $query = $specialPage->getRedirectQuery( $subpage ) ?: [];
                                                        $request = new DerivativeRequest( $this->context->getRequest(), $query );
                                                        $request->setRequestURL( $this->context->getRequest()->getRequestURL() );
index 22a7612..e7f4107 100644 (file)
@@ -39,6 +39,19 @@ class SpecialGoToInterwiki extends UnlistedSpecialPage {
        }
 
        public function execute( $par ) {
+               // Allow forcing an interstitial for local interwikis. This is used
+               // when a redirect page is reached via a special page which resolves
+               // to a user-dependent value (as defined by
+               // RedirectSpecialPage::personallyIdentifiableTarget). See the hack
+               // for avoiding T109724 in MediaWiki::performRequest (which also
+               // explains why we can't use a query parameter instead).
+               //
+               // HHVM dies when substr_compare is used on an empty string so ensure it's not.
+               $force = ( substr_compare( $par ?: 'x', 'force/', 0, 6 ) === 0 );
+               if ( $force ) {
+                       $par = substr( $par, 6 );
+               }
+
                $this->setHeaders();
                $target = Title::newFromText( $par );
                // Disallow special pages as a precaution against
@@ -50,9 +63,9 @@ class SpecialGoToInterwiki extends UnlistedSpecialPage {
                }
 
                $url = $target->getFullURL();
-               if ( !$target->isExternal() || $target->isLocal() ) {
+               if ( !$target->isExternal() || ( $target->isLocal() && !$force ) ) {
                        // Either a normal page, or a local interwiki.
-                       // just redirect.
+                       // Just redirect.
                        $this->getOutput()->redirect( $url, '301' );
                } else {
                        $this->getOutput()->addWikiMsg(
diff --git a/tests/phpunit/includes/specials/SpecialGoToInterwikiTest.php b/tests/phpunit/includes/specials/SpecialGoToInterwikiTest.php
new file mode 100644 (file)
index 0000000..05ec710
--- /dev/null
@@ -0,0 +1,81 @@
+<?php
+
+use MediaWiki\Interwiki\InterwikiLookupAdapter;
+use MediaWiki\MediaWikiServices;
+
+/**
+ * @covers SpecialGoToInterwiki
+ */
+class SpecialGoToInterwikiTest extends MediaWikiTestCase {
+
+       public function testExecute() {
+               $this->setService( 'InterwikiLookup', new InterwikiLookupAdapter(
+                       new HashSiteStore(), // won't be used
+                       [
+                               'local' => new Interwiki( 'local', 'https://local.example.com/$1',
+                                       'https://local.example.com/api.php', 'unittest_localwiki', 1 ),
+                               'nonlocal' => new Interwiki( 'nonlocal', 'https://nonlocal.example.com/$1',
+                                       'https://nonlocal.example.com/api.php', 'unittest_nonlocalwiki', 0 ),
+                       ]
+               ) );
+               MediaWikiServices::getInstance()->resetServiceForTesting( 'TitleFormatter' );
+               MediaWikiServices::getInstance()->resetServiceForTesting( 'TitleParser' );
+               MediaWikiServices::getInstance()->resetServiceForTesting( '_MediaWikiTitleCodec' );
+
+               // sanity check
+               $this->assertTrue( !Title::newFromText( 'Foo' )->isExternal() );
+               $this->assertTrue( Title::newFromText( 'local:Foo' )->isExternal() );
+               $this->assertTrue( Title::newFromText( 'nonlocal:Foo' )->isExternal() );
+               $this->assertTrue( Title::newFromText( 'local:Foo' )->isLocal() );
+               $this->assertTrue( !Title::newFromText( 'nonlocal:Foo' )->isLocal() );
+
+               $goToInterwiki = MediaWikiServices::getInstance()->getSpecialPageFactory()
+                       ->getPage( 'GoToInterwiki' );
+
+               RequestContext::resetMain();
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $goToInterwiki->setContext( $context );
+               $goToInterwiki->execute( 'Foo' );
+               $this->assertSame( Title::newFromText( 'Foo' )->getFullURL(),
+                       $context->getOutput()->getRedirect() );
+
+               RequestContext::resetMain();
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $goToInterwiki->setContext( $context );
+               $goToInterwiki->execute( 'local:Foo' );
+               $this->assertSame( Title::newFromText( 'local:Foo' )->getFullURL(),
+                       $context->getOutput()->getRedirect() );
+
+               RequestContext::resetMain();
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $goToInterwiki->setContext( $context );
+               $goToInterwiki->execute( 'nonlocal:Foo' );
+               $this->assertSame( '', $context->getOutput()->getRedirect() );
+               $this->assertContains( Title::newFromText( 'nonlocal:Foo' )->getFullURL(),
+                       $context->getOutput()->getHTML() );
+
+               RequestContext::resetMain();
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $goToInterwiki->setContext( $context );
+               $goToInterwiki->execute( 'force/Foo' );
+               $this->assertSame( Title::newFromText( 'Foo' )->getFullURL(),
+                       $context->getOutput()->getRedirect() );
+
+               RequestContext::resetMain();
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $goToInterwiki->setContext( $context );
+               $goToInterwiki->execute( 'force/local:Foo' );
+               $this->assertSame( '', $context->getOutput()->getRedirect() );
+               $this->assertContains( Title::newFromText( 'local:Foo' )->getFullURL(),
+                       $context->getOutput()->getHTML() );
+
+               RequestContext::resetMain();
+               $context = new DerivativeContext( RequestContext::getMain() );
+               $goToInterwiki->setContext( $context );
+               $goToInterwiki->execute( 'force/nonlocal:Foo' );
+               $this->assertSame( '', $context->getOutput()->getRedirect() );
+               $this->assertContains( Title::newFromText( 'nonlocal:Foo' )->getFullURL(),
+                       $context->getOutput()->getHTML() );
+       }
+
+}