From: Erik Bernhardson <ebernhardson@wikimedia.org>
Date: Wed, 17 Aug 2016 18:35:54 +0000 (-0700)
Subject: Issue 301 redirects for Special:Search/searchterm
X-Git-Tag: 1.31.0-rc.0~5998^2
X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=c2e8264b5349d3efed998b4f53abe0c289ec3b18;p=lhc%2Fweb%2Fwiklou.git

Issue 301 redirects for Special:Search/searchterm

Including the search term, which is PII, in the page title allows for
leaking this information through page view dumps. Instead of happily
handling these issue a 301 redirect to tell clients they should not
be issueing these requests, and should instead use the search query
parameter. Dumps at wikimedia remove 30[123] response codes from the
dump output so this will also stop leaking the PII.

Change-Id: Icce7cc4585e90742a8dd3513e7c9f7276e479cd7
---

diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php
index 9690d45bc9..26b86f9762 100644
--- a/includes/specials/SpecialSearch.php
+++ b/includes/specials/SpecialSearch.php
@@ -100,6 +100,25 @@ class SpecialSearch extends SpecialPage {
 	 * @param string $par
 	 */
 	public function execute( $par ) {
+		$request = $this->getRequest();
+
+		// Fetch the search term
+		$search = str_replace( "\n", " ", $request->getText( 'search' ) );
+
+		// Historically search terms have been accepted not only in the search query
+		// parameter, but also as part of the primary url. This can have PII implications
+		// in releasing page view data. As such issue a 301 redirect to the correct
+		// URL.
+		if ( strlen( $par ) && !strlen( $search ) ) {
+			$query = $request->getValues();
+			unset( $query['title'] );
+			// Strip underscores from title parameter; most of the time we'll want
+			// text form here. But don't strip underscores from actual text params!
+			$query['search'] = str_replace( '_', ' ', $par );
+			$this->getOutput()->redirect( $this->getPageTitle()->getFullURL( $query ), 301 );
+			return;
+		}
+
 		$this->setHeaders();
 		$this->outputHeader();
 		$out = $this->getOutput();
@@ -110,15 +129,6 @@ class SpecialSearch extends SpecialPage {
 		] );
 		$this->addHelpLink( 'Help:Searching' );
 
-		// Strip underscores from title parameter; most of the time we'll want
-		// text form here. But don't strip underscores from actual text params!
-		$titleParam = str_replace( '_', ' ', $par );
-
-		$request = $this->getRequest();
-
-		// Fetch the search term
-		$search = str_replace( "\n", " ", $request->getText( 'search', $titleParam ) );
-
 		$this->load();
 		if ( !is_null( $request->getVal( 'nsRemember' ) ) ) {
 			$this->saveNamespaces();
diff --git a/tests/phpunit/specials/SpecialSearchTest.php b/tests/phpunit/specials/SpecialSearchTest.php
new file mode 100644
index 0000000000..20e88f5a5b
--- /dev/null
+++ b/tests/phpunit/specials/SpecialSearchTest.php
@@ -0,0 +1,23 @@
+<?php
+
+class SpecialSearchText extends \PHPUnit_Framework_TestCase {
+	public function testSubPageRedirect() {
+		$ctx = new RequestContext;
+
+		SpecialPageFactory::executePath(
+			Title::newFromText( 'Special:Search/foo_bar' ),
+			$ctx
+		);
+		$url = $ctx->getOutput()->getRedirect();
+		// some older versions of hhvm have a bug that doesn't parse relative
+		// urls with a port, so help it out a little bit.
+		// https://github.com/facebook/hhvm/issues/7136
+		$url = wfExpandUrl( $url, PROTO_CURRENT );
+
+		$parts = parse_url( $url );
+		$this->assertEquals( '/w/index.php', $parts['path'] );
+		parse_str( $parts['query'], $query );
+		$this->assertEquals( 'Special:Search', $query['title'] );
+		$this->assertEquals( 'foo bar', $query['search'] );
+	}
+}