Chad has submitted this change and it was merged.
Change subject: MediaWiki.php: Factor out tryNormaliseRedirect
......................................................................
MediaWiki.php: Factor out tryNormaliseRedirect
This is in preparation for fixing T67402, which requires adding
logic inside this condition block. However the to-be-added code
will influences whether or not a redirect should be made.
In case a redirect is not made, it has to fall through to the next
'elseif' handler in MediaWiki::performRequest(), which is not possible
from inside the 'if' block.
Hence, move it out in a separate block and use a boolean return value
to communicate whether the case has been handled.
This also allows us to unit test this thing. Which is desperately
needed. Albeit ugly as it requires lots of mocking.
Change-Id: If3157f2ff1fd3ab2ca20a5d1f550d864ea62c493
---
M includes/MediaWiki.php
A tests/phpunit/includes/MediaWikiTest.php
2 files changed, 240 insertions(+), 60 deletions(-)
Approvals:
Chad: Verified; Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 402494e..e06877f 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -225,76 +225,99 @@
$output->redirect( $url, 301 );
} else {
$this->context->setTitle(
SpecialPage::getTitleFor( 'Badtitle' ) );
- wfProfileOut( __METHOD__ );
throw new BadTitleError();
}
- // Redirect loops, no title in URL, $wgUsePathInfo URLs, and
URLs with a variant
- } elseif ( $request->getVal( 'action', 'view' ) == 'view' &&
!$request->wasPosted()
- && ( $request->getVal( 'title' ) === null
- || $title->getPrefixedDBkey() !=
$request->getVal( 'title' ) )
- && !count( $request->getValueNames( array( 'action',
'title' ) ) )
- && wfRunHooks( 'TestCanonicalRedirect', array(
$request, $title, $output ) )
- ) {
- if ( $title->isSpecialPage() ) {
- list( $name, $subpage ) =
SpecialPageFactory::resolveAlias( $title->getDBkey() );
- if ( $name ) {
- $title = SpecialPage::getTitleFor(
$name, $subpage );
- }
- }
- $targetUrl = wfExpandUrl( $title->getFullURL(),
PROTO_CURRENT );
- // Redirect to canonical url, make it a 301 to allow
caching
- if ( $targetUrl == $request->getFullRequestURL() ) {
- $message = "Redirect loop detected!\n\n" .
- "This means the wiki got confused about
what page was " .
- "requested; this sometimes happens when
moving a wiki " .
- "to a new server or changing the server
configuration.\n\n";
+ // Handle any other redirects.
+ // Redirect loops, titleless URL, $wgUsePathInfo URLs, and URLs
with a variant
+ } elseif ( !$this->tryNormaliseRedirect( $title ) ) {
- if ( $this->config->get( 'UsePathInfo' ) ) {
- $message .= "The wiki is trying to
interpret the page " .
- "title from the URL path
portion (PATH_INFO), which " .
- "sometimes fails depending on
the web server. Try " .
- "setting \"\$wgUsePathInfo =
false;\" in your " .
- "LocalSettings.php, or check
that \$wgArticlePath " .
- "is correct.";
+ // Special pages
+ if ( NS_SPECIAL == $title->getNamespace() ) {
+ // Actions that need to be made when we have a
special pages
+ SpecialPageFactory::executePath( $title,
$this->context );
+ } else {
+ // ...otherwise treat it as an article view.
The article
+ // may still be a wikipage redirect to another
article or URL.
+ $article = $this->initializeArticle();
+ if ( is_object( $article ) ) {
+ $this->performAction( $article,
$requestTitle );
+ } elseif ( is_string( $article ) ) {
+ $output->redirect( $article );
} else {
- $message .= "Your web server was
detected as possibly not " .
- "supporting URL path components
(PATH_INFO) correctly; " .
- "check your LocalSettings.php
for a customized " .
- "\$wgArticlePath setting and/or
toggle \$wgUsePathInfo " .
- "to true.";
+ throw new MWException( "Shouldn't
happen: MediaWiki::initializeArticle()"
+ . " returned neither an object
nor a URL" );
}
- throw new HttpError( 500, $message );
- } else {
- $output->setSquidMaxage( 1200 );
- $output->redirect( $targetUrl, '301' );
- }
- // Special pages
- } elseif ( NS_SPECIAL == $title->getNamespace() ) {
- $pageView = true;
- // Actions that need to be made when we have a special
pages
- SpecialPageFactory::executePath( $title, $this->context
);
- } else {
- // ...otherwise treat it as an article view. The article
- // may be a redirect to another article or URL.
- $article = $this->initializeArticle();
- if ( is_object( $article ) ) {
- $pageView = true;
- $this->performAction( $article, $requestTitle );
- } elseif ( is_string( $article ) ) {
- $output->redirect( $article );
- } else {
- wfProfileOut( __METHOD__ );
- throw new MWException( "Shouldn't happen:
MediaWiki::initializeArticle()"
- . " returned neither an object nor a
URL" );
}
}
+ }
- if ( $pageView ) {
- // Promote user to any groups they meet the criteria for
- $user->addAutopromoteOnceGroups( 'onView' );
+ /**
+ * Handle redirects for uncanonical title requests.
+ *
+ * Handles:
+ * - Redirect loops.
+ * - No title in URL.
+ * - $wgUsePathInfo URLs.
+ * - URLs with a variant.
+ * - Other non-standard URLs (as long as they have no extra query
parameters).
+ *
+ * Behaviour:
+ * - Normalise title values:
+ * /wiki/Foo%20Bar -> /wiki/Foo_Bar
+ * - Normalise empty title:
+ * /wiki/ -> /wiki/Main
+ * /w/index.php?title= -> /wiki/Main
+ * - Don't redirect anything with query parameters other than 'title'
or 'action=view'.
+ *
+ * @return bool True if a redirect was set.
+ */
+ private function tryNormaliseRedirect( $title ) {
+ $request = $this->context->getRequest();
+ $output = $this->context->getOutput();
+
+ if ( $request->getVal( 'action', 'view' ) != 'view'
+ || $request->wasPosted()
+ || ( $request->getVal( 'title' ) !== null
+ && $title->getPrefixedDBkey() ==
$request->getVal( 'title' ) )
+ || count( $request->getValueNames( array( 'action',
'title' ) ) )
+ || !Hooks::run( 'TestCanonicalRedirect', array(
$request, $title, $output ) )
+ ) {
+ return false;
}
- wfProfileOut( __METHOD__ );
+ if ( $title->isSpecialPage() ) {
+ list( $name, $subpage ) =
SpecialPageFactory::resolveAlias( $title->getDBkey() );
+ if ( $name ) {
+ $title = SpecialPage::getTitleFor( $name,
$subpage );
+ }
+ }
+ // Redirect to canonical url, make it a 301 to allow caching
+ $targetUrl = wfExpandUrl( $title->getFullURL(), PROTO_CURRENT );
+ if ( $targetUrl == $request->getFullRequestURL() ) {
+ $message = "Redirect loop detected!\n\n" .
+ "This means the wiki got confused about what
page was " .
+ "requested; this sometimes happens when moving
a wiki " .
+ "to a new server or changing the server
configuration.\n\n";
+
+ if ( $this->config->get( 'UsePathInfo' ) ) {
+ $message .= "The wiki is trying to interpret
the page " .
+ "title from the URL path portion
(PATH_INFO), which " .
+ "sometimes fails depending on the web
server. Try " .
+ "setting \"\$wgUsePathInfo = false;\"
in your " .
+ "LocalSettings.php, or check that
\$wgArticlePath " .
+ "is correct.";
+ } else {
+ $message .= "Your web server was detected as
possibly not " .
+ "supporting URL path components
(PATH_INFO) correctly; " .
+ "check your LocalSettings.php for a
customized " .
+ "\$wgArticlePath setting and/or toggle
\$wgUsePathInfo " .
+ "to true.";
+ }
+ throw new HttpError( 500, $message );
+ }
+ $output->setSquidMaxage( 1200 );
+ $output->redirect( $targetUrl, '301' );
+ return true;
}
/**
diff --git a/tests/phpunit/includes/MediaWikiTest.php
b/tests/phpunit/includes/MediaWikiTest.php
new file mode 100644
index 0000000..df94d3e
--- /dev/null
+++ b/tests/phpunit/includes/MediaWikiTest.php
@@ -0,0 +1,157 @@
+<?php
+
+class MediaWikiTest extends MediaWikiTestCase {
+ protected function setUp() {
+ parent::setUp();
+
+ $this->setMwGlobals( array(
+ 'wgServer' => 'http://example.org',
+ 'wgScriptPath' => '/w',
+ 'wgScript' => '/w/index.php',
+ 'wgArticlePath' => '/wiki/$1',
+ 'wgActionPaths' => array(),
+ ) );
+ }
+
+ public static function provideTryNormaliseRedirect() {
+ return array(
+ array(
+ // View: Canonical
+ 'url' => 'http://example.org/wiki/Foo_Bar',
+ 'query' => array(),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Escaped title
+ 'url' => 'http://example.org/wiki/Foo%20Bar',
+ 'query' => array(),
+ 'title' => 'Foo_Bar',
+ 'redirect' => 'http://example.org/wiki/Foo_Bar',
+ ),
+ array(
+ // View: Script path
+ 'url' =>
'http://example.org/w/index.php?title=Foo_Bar',
+ 'query' => array( 'title' => 'Foo_Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Script path with implicit title from
page id
+ 'url' =>
'http://example.org/w/index.php?curid=123',
+ 'query' => array( 'curid' => '123' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Script path with implicit title from
revision id
+ 'url' =>
'http://example.org/w/index.php?oldid=123',
+ 'query' => array( 'oldid' => '123' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Script path without title
+ 'url' => 'http://example.org/w/index.php',
+ 'query' => array(),
+ 'title' => 'Main_Page',
+ 'redirect' =>
'http://example.org/wiki/Main_Page',
+ ),
+ array(
+ // View: Script path with empty title
+ 'url' =>
'http://example.org/w/index.php?title=',
+ 'query' => array( 'title' => '' ),
+ 'title' => 'Main_Page',
+ 'redirect' =>
'http://example.org/wiki/Main_Page',
+ ),
+ array(
+ // View: Index with escaped title
+ 'url' =>
'http://example.org/w/index.php?title=Foo%20Bar',
+ 'query' => array( 'title' => 'Foo Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => 'http://example.org/wiki/Foo_Bar',
+ ),
+ array(
+ // View: Script path with escaped title
+ 'url' => 'http://example.org/w/?title=Foo_Bar',
+ 'query' => array( 'title' => 'Foo_Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Root path with escaped title
+ 'url' => 'http://example.org/?title=Foo_Bar',
+ 'query' => array( 'title' => 'Foo_Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Canonical with redundant query
+ 'url' =>
'http://example.org/wiki/Foo_Bar?action=view',
+ 'query' => array( 'action' => 'view' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // Edit: Canonical view url with action query
+ 'url' =>
'http://example.org/wiki/Foo_Bar?action=edit',
+ 'query' => array( 'action' => 'edit' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Index with action query
+ 'url' =>
'http://example.org/w/index.php?title=Foo_Bar&action=view',
+ 'query' => array( 'title' => 'Foo_Bar',
'action' => 'view' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // Edit: Index with action query
+ 'url' =>
'http://example.org/w/index.php?title=Foo_Bar&action=edit',
+ 'query' => array( 'title' => 'Foo_Bar',
'action' => 'edit' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideTryNormaliseRedirect
+ * @covers MediaWiki::tryNormaliseRedirect
+ */
+ public function testTryNormaliseRedirect( $url, $query, $title,
$expectedRedirect = false ) {
+ // Set SERVER because interpolateTitle() doesn't use
getRequestURL(),
+ // whereas tryNormaliseRedirect does().
+ $_SERVER['REQUEST_URI'] = $url;
+
+ $req = new FauxRequest( $query );
+ $req->setRequestURL( $url );
+ // This adds a virtual 'title' query parameter. Normally called
from Setup.php
+ $req->interpolateTitle();
+
+ $titleObj = Title::newFromText( $title );
+
+ // Set global context since some involved code paths don't yet
have context
+ $context = RequestContext::getMain();
+ $context->setRequest( $req );
+ $context->setTitle( $titleObj );
+
+ $mw = new MediaWiki( $context );
+
+ $method = new ReflectionMethod( $mw, 'tryNormaliseRedirect' );
+ $method->setAccessible( true );
+ $ret = $method->invoke( $mw, $titleObj );
+
+ $this->assertEquals(
+ $expectedRedirect !== false,
+ $ret,
+ 'Return true only when redirecting'
+ );
+
+ $this->assertEquals(
+ $expectedRedirect ?: '',
+ $context->getOutput()->getRedirect()
+ );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/259918
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If3157f2ff1fd3ab2ca20a5d1f550d864ea62c493
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_24
Gerrit-Owner: Chad <[email protected]>
Gerrit-Reviewer: Chad <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
