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

diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php
index eacc7a0..ad045e4 100644 (file)
--- 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( '<div class="searchresults">' );
                }
 
-               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( "<p class=\"mw-search-nonefound\">\n$1</p>", [
                                $hasOtherResults ? 'search-nonefound-thiswiki' : 'search-nonefound',
                                wfEscapeWikiText( $term )

diff --git a/languages/i18n/en.json b/languages/i18n/en.json
index d654a52..01e1007 100644 (file)
--- 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|<strong>1</strong> result|<strong>$1</strong> results}} starting with #<strong>$2</strong>.",

diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index c831a71..081bb66 100644 (file)
--- 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 4dd6c80..eeb4b00 100644 (file)
--- 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