From 4d9d61460d9420cc184c22243b0da81855e8f3a1 Mon Sep 17 00:00:00 2001 From: Erik Bernhardson Date: Mon, 22 Jul 2019 12:10:54 -0700 Subject: [PATCH] Validate sort order in Special:Search Providing an invalid sort order to Special:Search could trigger an exception from the search engine when trying to apply it. Validate the sort order, much like API classes do, and let the user know that the sort they requested could not be applied. We also have a unreported error for invalid profile requested, so added that warning to the display while here. Bug: T228171 Change-Id: I79079eea8c03a90b5b65f1dad11c99e514de00e1 --- includes/specials/SpecialSearch.php | 31 +++++++++++++++---- languages/i18n/en.json | 2 ++ languages/i18n/qqq.json | 2 ++ .../includes/specials/SpecialSearchTest.php | 21 +++++++++++++ 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php index eacc7a0392..ad045e43f3 100644 --- a/includes/specials/SpecialSearch.php +++ b/includes/specials/SpecialSearch.php @@ -79,7 +79,7 @@ class SpecialSearch extends SpecialPage { /** * @var string */ - protected $sort; + protected $sort = SearchEngine::DEFAULT_SORT; /** * @var bool @@ -92,6 +92,12 @@ class SpecialSearch extends SpecialPage { */ protected $searchConfig; + /** + * @var Status Holds any parameter validation errors that should + * be displayed back to the user. + */ + private $loadStatus; + const NAMESPACES_CURRENT = 'sense'; public function __construct() { @@ -204,6 +210,8 @@ class SpecialSearch extends SpecialPage { * @see tests/phpunit/includes/specials/SpecialSearchTest.php */ public function load() { + $this->loadStatus = new Status(); + $request = $this->getRequest(); list( $this->limit, $this->offset ) = $request->getLimitOffset( 20, '' ); $this->mPrefix = $request->getVal( 'prefix', '' ); @@ -211,8 +219,13 @@ class SpecialSearch extends SpecialPage { $this->setExtraParam( 'prefix', $this->mPrefix ); } - $this->sort = $request->getVal( 'sort', SearchEngine::DEFAULT_SORT ); - if ( $this->sort !== SearchEngine::DEFAULT_SORT ) { + $sort = $request->getVal( 'sort', SearchEngine::DEFAULT_SORT ); + $validSorts = $this->getSearchEngine()->getValidSorts(); + if ( !in_array( $sort, $validSorts ) ) { + $this->loadStatus->warning( 'search-invalid-sort-order', $sort, + implode( ', ', $validSorts ) ); + } elseif ( $sort !== $this->sort ) { + $this->sort = $sort; $this->setExtraParam( 'sort', $this->sort ); } @@ -247,6 +260,7 @@ class SpecialSearch extends SpecialPage { $this->namespaces = $profiles[$profile]['namespaces']; } else { // Unknown profile requested + $this->loadStatus->warning( 'search-unknown-profile', $profile ); $profile = 'default'; $this->namespaces = $profiles['default']['namespaces']; } @@ -375,7 +389,7 @@ class SpecialSearch extends SpecialPage { $out->addHTML( $dymWidget->render( $term, $textMatches ) ); } - $hasErrors = $textStatus && $textStatus->getErrors() !== []; + $hasSearchErrors = $textStatus && $textStatus->getErrors() !== []; $hasOtherResults = $textMatches && $textMatches->hasInterwikiResults( ISearchResultSet::INLINE_RESULTS ); @@ -385,7 +399,12 @@ class SpecialSearch extends SpecialPage { $out->addHTML( '
' ); } - if ( $hasErrors ) { + if ( $hasSearchErrors || $this->loadStatus->getErrors() ) { + if ( $textStatus === null ) { + $textStatus = $this->loadStatus; + } else { + $textStatus->merge( $this->loadStatus ); + } list( $error, $warning ) = $textStatus->splitByErrorType(); if ( $error->getErrors() ) { $out->addHTML( Html::errorBox( @@ -405,7 +424,7 @@ class SpecialSearch extends SpecialPage { Hooks::run( 'SpecialSearchResults', [ $term, &$titleMatches, &$textMatches ] ); // If we have no results and have not already displayed an error message - if ( $num === 0 && !$hasErrors ) { + if ( $num === 0 && !$hasSearchErrors ) { $out->wrapWikiMsg( "

\n$1

", [ $hasOtherResults ? 'search-nonefound-thiswiki' : 'search-nonefound', wfEscapeWikiText( $term ) diff --git a/languages/i18n/en.json b/languages/i18n/en.json index d654a521c7..01e1007da4 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -1025,6 +1025,8 @@ "search-interwiki-more": "(more)", "search-interwiki-more-results": "more results", "search-relatedarticle": "Related", + "search-invalid-sort-order": "Sort order of $1 is unrecognized, default sorting will be applied. Valid sort orders are: $2", + "search-unknown-profile": "Search profile of $1 is unrecognized, default search profile will be applied.", "searchrelated": "related", "searchall": "all", "showingresults": "Showing below up to {{PLURAL:$1|1 result|$1 results}} starting with #$2.", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index c831a710ad..081bb66f43 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -1234,6 +1234,8 @@ "search-interwiki-more": "{{Identical|More}}", "search-interwiki-more-results": "Label for a link that leads to more search results from a given wiki.", "search-relatedarticle": "This is a search result (and I guess search engine) dependent messages. I do not know how to trigger the feature. The message is displayed if the search result contains information that related pages can also be provided from the search engine. I assume this is \"More Like This\" functionality. Microsoft glossary defines MLT as \"A way to refine search by identifying the right set of documents and then locating similar documents. This allows the searcher to control the direction of the search and focus on the most fruitful lines of inquiry.\"[http://www.microsoft.com/enterprisesearch/en/us/search-glossary.aspx]\n{{Identical|Related}}", + "search-invalid-sort-order": "Warning displayed on Special:Search when an unrecognized sorting order is requested.", + "search-unknown-profile": "Warning displayed on Special:Search when an unrecognized search profile is requested.", "searchrelated": "This is a search result (and I guess search engine) dependent messages. I do not know how to trigger the feature. The message is displayed if the search result contains information that related pages can also be provided from the search engine. I assume this is \"More Like This\" functionality. Microsoft glossary defines MLT as \"A way to refine search by identifying the right set of documents and then locating similar documents. This allows the searcher to control the direction of the search and focus on the most fruitful lines of inquiry.\"[http://www.microsoft.com/enterprisesearch/en/us/search-glossary.aspx]\n{{Identical|Related}}", "searchall": "{{Identical|All}}", "showingresults": "This message is used on some special pages such as [[Special:WantedCategories]]. Parameters:\n* $1 - the total number of results in the batch shown\n* $2 - the number of the first item listed\nSee also:\n* {{msg-mw|Showingresultsnum}}", diff --git a/tests/phpunit/includes/specials/SpecialSearchTest.php b/tests/phpunit/includes/specials/SpecialSearchTest.php index 4dd6c8045e..eeb4b003c6 100644 --- a/tests/phpunit/includes/specials/SpecialSearchTest.php +++ b/tests/phpunit/includes/specials/SpecialSearchTest.php @@ -11,6 +11,27 @@ use MediaWiki\MediaWikiServices; */ class SpecialSearchTest extends MediaWikiTestCase { + /** + * @covers SpecialSearch::load + * @covers SpecialSearch::showResults + */ + public function testValidateSortOrder() { + $ctx = new RequestContext(); + $ctx->setRequest( new FauxRequest( [ + 'search' => 'foo', + 'fulltext' => 1, + 'sort' => 'invalid', + ] ) ); + $sp = Title::makeTitle( NS_SPECIAL, 'Search' ); + MediaWikiServices::getInstance() + ->getSpecialPageFactory() + ->executePath( $sp, $ctx ); + $html = $ctx->getOutput()->getHTML(); + $this->assertRegExp( '/class="warningbox"/', $html, 'must contain warnings' ); + $this->assertRegExp( '/Sort order of invalid is unrecognized/', + $html, 'must tell user sort order is invalid' ); + } + /** * @covers SpecialSearch::load * @dataProvider provideSearchOptionsTests -- 2.20.1