Page MenuHomePhabricator
Create Task
Maniphest T276385

[Log noise] "Prefix search request was longer than the maximum allowed length."
Closed, ResolvedPublic3 Estimated Story PointsPRODUCTION ERROR
Actions

Description

Error

MediaWiki version: 1.36.0-wmf.32

message
Prefix search request was longer than the maximum allowed length.
exception.trace
from /srv/mediawiki/php-1.36.0-wmf.32/includes/api/ApiUsageException.php(71)
#0 /srv/mediawiki/php-1.36.0-wmf.32/extensions/CirrusSearch/includes/Query/QueryBuilderTraits.php(25): ApiUsageException::newWithMessage(NULL, array, string, array, integer)
#1 /srv/mediawiki/php-1.36.0-wmf.32/extensions/CirrusSearch/includes/Query/CompSuggestQueryBuilder.php(87): CirrusSearch\Query\CompSuggestQueryBuilder->checkTitleSearchRequestLength(string)
#2 /srv/mediawiki/php-1.36.0-wmf.32/extensions/CirrusSearch/includes/CompletionSuggester.php(309): CirrusSearch\Query\CompSuggestQueryBuilder->build(string, array)
#3 /srv/mediawiki/php-1.36.0-wmf.32/extensions/CirrusSearch/includes/CompletionSuggester.php(186): CirrusSearch\CompletionSuggester->getSuggestSearchRequest(string, array)
#4 /srv/mediawiki/php-1.36.0-wmf.32/extensions/CirrusSearch/includes/CirrusSearch.php(360): CirrusSearch\CompletionSuggester->suggest(string, array)
#5 /srv/mediawiki/php-1.36.0-wmf.32/extensions/CirrusSearch/includes/CirrusSearch.php(441): CirrusSearch\CirrusSearch->getSuggestions(string, array, CirrusSearch\SearchConfig)
#6 /srv/mediawiki/php-1.36.0-wmf.32/includes/search/SearchEngine.php(556): CirrusSearch\CirrusSearch->completionSearchBackend(string)
#7 /srv/mediawiki/php-1.36.0-wmf.32/includes/search/SearchEngine.php(602): SearchEngine->completionSearchBackendOverfetch(string)
#8 /srv/mediawiki/php-1.36.0-wmf.32/extensions/CirrusSearch/includes/CirrusSearch.php(452): SearchEngine->completionSearch(string)
#9 /srv/mediawiki/php-1.36.0-wmf.32/includes/Rest/Handler/SearchHandler.php(156): CirrusSearch\CirrusSearch->completionSearchWithVariants(string)
#10 /srv/mediawiki/php-1.36.0-wmf.32/includes/Rest/Handler/SearchHandler.php(311): MediaWiki\Rest\Handler\SearchHandler->doSearch(CirrusSearch\CirrusSearch)
#11 /srv/mediawiki/php-1.36.0-wmf.32/includes/Rest/Router.php(395): MediaWiki\Rest\Handler\SearchHandler->execute()
#12 /srv/mediawiki/php-1.36.0-wmf.32/includes/Rest/Router.php(322): MediaWiki\Rest\Router->executeHandler(MediaWiki\Rest\Handler\SearchHandler)
#13 /srv/mediawiki/php-1.36.0-wmf.32/includes/Rest/EntryPoint.php(165): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#14 /srv/mediawiki/php-1.36.0-wmf.32/includes/Rest/EntryPoint.php(130): MediaWiki\Rest\EntryPoint->execute()
#15 /srv/mediawiki/php-1.36.0-wmf.32/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#16 /srv/mediawiki/w/rest.php(3): require(string)
#17 {main}

Impact

User can trigger log spam by sending large search strings.

Notes

Details

Request ID
YD-b0DxbM5M10y4YaozxWwAAAFc
Request URL
https://fr.wikipedia.org/w/rest.php/v1/search/title?q=je%20suis%20sur%20que%20le%20matin%20les%20gens%20ne%20respectent%20pas%20le%20couvre%20feux%20macron%20de%20mission%20je%20veux%20plus%20le%20coronavirus%20comment%20on%20fait%20%C3%A7a%20et%20la%20dict%C3%A9e%20que%20la%20maitresse%20nous%20a%20fais%20est%20tr%C3%A8s%20%20bien%20hgdfgyuegyugeuysgfuyrfygrygyfhfhfhffhgyertyteytryettezurtetryteyrttela%20d&limit=10
Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Return no results for over-length title searchesmediawiki/extensions/CirrusSearchmaster+95 -70
Customize query in gerrit

Related Objects

Event Timeline

daniel created this task.Mar 3 2021, 7:12 PM
Restricted Application added a project: Discovery-Search. · View Herald TranscriptMar 3 2021, 7:12 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
thcipriani moved this task from Untriaged to Mar 2021 on the Wikimedia-production-error board.Mar 3 2021, 7:47 PM
Umherirrender subscribed.Mar 3 2021, 8:08 PM

The limit is 255 (using mb_strlen), not sure what to do instead.
Having a user visible exception instead to show an error message. But api part needs some error reporting as well.

		$requestLength = mb_strlen( $term );
		if ( $requestLength > CirrusSearch::MAX_TITLE_SEARCH ) {
			// TODO: stop using ApiUsageException we are not even sure that we are in an API call
			throw ApiUsageException::newWithMessage(
				null,
				[ 'apierror-cirrus-requesttoolong', $requestLength, CirrusSearch::MAX_TITLE_SEARCH ],
				'request_too_long',
				[],
				400
			);
		}
daniel added a comment.EditedMar 3 2021, 10:01 PM

An ApiUsageException generally doesn't make it into the error log... why is this one logged?

Oh I see - ApiUsageException is handled by ApiMain, but the request in question is coming in an REST endpoint. Rest\Router and Rest\Handler don't have handling for this kind of exception, and probably should not.

I think the correct thing to do here is to throw a different exception, and convert it as appropriate in the code that implements search for the different API endpoints.

Alternatively, the API handlers (action API module and REST API handler) could apply the length check themselves, so they have control over the error handling. The "deeper" code could throw an InvalidArgumentException, but hopefully we'd never trigger that.

Krinkle subscribed.EditedMar 4 2021, 10:16 PM

@daniel See also ErrorPageError (and ILocalizedException) which are a similar abstraction used by Actions and Special pages. Seems like maybe there's a reasonable way we can minimally implement ILocalizedException such that we don't need a ErrorPageError, ApiUsageException and now REST-specific implementation, but just the general one and rendered into the appropiate response by the entry point catch handler (where the rendering would not itself be an Exception subclass).

• MPhamWMF moved this task from needs triage to Current work on the Discovery-Search board.Mar 8 2021, 4:23 PM
• MPhamWMF edited projects, added Discovery-Search (Current work); removed Discovery-Search.
• MPhamWMF set the point value for this task to 3.Mar 8 2021, 4:59 PM
• MPhamWMF moved this task from Incoming to Ready for Dev -- SWE on the Discovery-Search (Current work) board.
brennen subscribed.Mar 9 2021, 5:03 PM

As I start the train for 1.36.0-wmf.34 (T274938), this continues at a rate of 100-200 an hour. While it's filtered on the mw-new-errors dashboard, it's not great for using the mediawiki-errors dashboard or logspam-watch. Deployers would appreciate a fix.

EBernhardson subscribed.Mar 10 2021, 12:17 AM

While pondering a fix, i wonder if cirrussearch really even needs to throw an exception here. Page titles are well documented as limited to 255 bytes. In other cases where we receive queries with no possible answer we simply return no results as the correct answer. That seems sane here, and if we want to provide a better UX in this narrow case (i suspect these are mostly automated and it helps the rare human) that can be implemented in a sensible layer.

gerritbot added a comment.Mar 10 2021, 1:23 AM

Change 670329 had a related patch set uploaded (by Ebernhardson; owner: Ebernhardson):
[mediawiki/extensions/CirrusSearch@master] Return no results for over-length title searches

https://gerrit.wikimedia.org/r/670329

gerritbot added a project: Patch-For-Review.Mar 10 2021, 1:23 AM
EBernhardson claimed this task.Mar 10 2021, 1:24 AM
EBernhardson moved this task from Ready for Dev -- SWE to Needs review on the Discovery-Search (Current work) board.
Krinkle renamed this task from Unnecessary error log entry: Prefix search request was longer than the maximum allowed length. to [Log noise] "Prefix search request was longer than the maximum allowed length.".Mar 10 2021, 7:42 PM
Krinkle updated the task description. (Show Details)
Krinkle edited Stack Trace. (Show Details)
gerritbot added a comment.Mar 11 2021, 3:08 PM

Change 670329 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Return no results for over-length title searches

https://gerrit.wikimedia.org/r/670329

Maintenance_bot removed a project: Patch-For-Review.Mar 11 2021, 3:10 PM
ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.35; 2021-03-16).Mar 11 2021, 4:00 PM
EBernhardson moved this task from Needs review to To Be Deployed on the Discovery-Search (Current work) board.Mar 11 2021, 5:12 PM
dancy subscribed.Mar 15 2021, 3:22 PM

Much appreciation for this change!

dancy mentioned this in T93123: Log the "Prefix search request" too long exceptions separate from main exception log.Mar 17 2021, 7:10 PM
EBernhardson moved this task from To Be Deployed to Needs Reporting on the Discovery-Search (Current work) board.Mar 23 2021, 6:58 PM
Gehel closed this task as Resolved.Mar 24 2021, 1:26 PM
daniel mentioned this in T285984: PHP REST router should gracefully handle exceptions..Jul 1 2021, 9:43 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits