Project

General

Profile

Actions

Feature #73626

closed

numberOfResults should be configurable and report overflow

Added by Daniel Neugebauer about 8 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Should have
Category:
Indexed Search
Target version:
Start date:
2016-02-23
Due date:
% Done:

100%

Estimated time:
PHP Version:
Tags:
Complexity:
medium
Sprint Focus:
On Location Sprint

Description

The patch for TYPO3-CORE-SA-2016-008 limits the number of search results to a fixed value of 100 without a chance to override it. That change restricts indexed_search to use a paged browser which may not be desirable on all websites (some websites need to show all results on one page without a page browser). As there is no way of configuring the upper limit, increasing the number of search results on one page is now only possible by manually patching TYPO3 Core.

I therefore propose the following changes:

  • The upper limit should be configurable, preferably via Setup/Constant TS. If that's not possible, it should at least be configurable via extension configuration or similar means.
  • If more results exist (can be checked by querying the DB for n+1 results, true if n is exceeded) a configurable message should be shown ("too many hits, some results are not shown, try to be more specific").
  • It should be possible to disable frontend limit configuration altogether so, if no FE selection of the number of results per page is desired, only a fixed, BE/template configured value is used instead.

As the security patch broke/limited functionality in 6.2, at least the configurable limit should be introduced to 6.2 as well to restore former operation.

Actions #1

Updated by Mathias Schreiber about 8 years ago

Daniel Neugebauer wrote:

That change restricts indexed_search to use a paged browser which may not be desirable on all websites (some websites need to show all results on one page without a page browser).

Do you have a usecase at hand?
I can't imagine any.

As there is no way of configuring the upper limit, increasing the number of search results on one page is now only possible by manually patching TYPO3 Core.

Correct.
In order to prevent your server from drowning.

I therefore propose the following changes:

  • The upper limit should be configurable, preferably via Setup/Constant TS. If that's not possible, it should at least be configurable via extension configuration or similar means.
  • If more results exist (can be checked by querying the DB for n+1 results, true if n is exceeded) a configurable message should be shown ("too many hits, some results are not shown, try to be more specific").

No, unfortunately it can't.
The way indexed search works is that it does not allow to use "LIMIT" in its queries.

  • It should be possible to disable frontend limit configuration altogether so, if no FE selection of the number of results per page is desired, only a fixed, BE/template configured value is used instead.

As the security patch broke/limited functionality in 6.2, at least the configurable limit should be introduced to 6.2 as well to restore former operation.

Somehow every change changes the way a system behaves, doesn't it?
Currently I value the DOS protection higher than a revert.
In general your ideas are feasible, let's discuss them either here or on slack and make a patch out of it.
I highly doubt it will be fixed in 6.2, though.

Actions #2

Updated by Mathias Schreiber about 8 years ago

  • Status changed from New to Needs Feedback
  • Assignee set to Mathias Schreiber
Actions #3

Updated by Daniel Neugebauer about 8 years ago

Mathias Schreiber wrote:

Do you have a usecase at hand?
I can't imagine any.

We have a website which introduces a categorization filter for all search results (based on the page paths) to allow the user to select/deselect results concerning "products/services", "news" and "other".

Pagination does not work in that case: The first page may mostly yield results for products but only 1-2 results for news. If pagination is used, the user only sees those few results and then needs to change pages. The next page may not contain any more results. And we can't count the total results either. A workaround would be to use pagination but hide it from the user, querying all pages one-by-one using AJAX. However, doing that is just stupid as it does not yield any advantage, neither to the user (who has to wait for all results to become available) nor for the server as it actually increases the work load compared to just listing all results at once.

Another use case would be to alleviate issues with highly broken result count/page numbers. I'm not sure what causes it but I think none of our websites counts search results accurately, often resulting in presenting pages 5 and 6 when it can only display results up to page 4. In the past, a workaround, we increased the number of search results to a higher number as all results shown on one page are accurately counted and it thus helps to attempt to just list all of them at once.

100000 results (as was up to 6.2.18) really is an unreasonably high number, almost equal to no limit at all. However, 100 is just too low. The website I'm referring to easily has ~200-300 results for common search keywords so we need a raised limit. The new limit introduces very tight restrictions which were not present before (neither in 4.x nor 6.x/7.x) so I regard it as a breaking change which could have been avoided by a proper fix (making the limit configurable with a lower default setting instead of just severely lowering the magic number).

Of course it could just be increased to 500 or 1000 which would be sufficient in our case and shouldn't stress servers so far as to be able to cause DoS scenarios. But instead of working with hardcoded magic numbers (which may not suffice someone elses use case, again) the limit should just be made configurable - it can't be that much work to implement it.

I'm happy to provide a patch but it may take some time as I got other things to attend to first.

Actions #4

Updated by Tymoteusz Motylewski about 8 years ago

Indexed search is has already a build in feature used for configuring list of available results per page (blind. numberOfResults).

I think the good solution is as follows:
- remove hardcoded limit of 100 results
- use the configured list of available results per page for validation (it can be changed from TS). So we only allows user to only use value from the prefefined list.

@Daniel Hinderink, are you able to provide a patch like that in gerrit?

Actions #5

Updated by Tymoteusz Motylewski over 7 years ago

  • Status changed from Needs Feedback to New
  • Target version set to 8.4
  • Complexity set to medium
  • Sprint Focus set to On Location Sprint
Actions #6

Updated by Tom Voigt over 7 years ago

I've just run into the same issue on a TYPO3 7.6.9 Project. I've integrated buttons to check if there are some special results eg. files or websites. Depending on the types of the results I'm displaying button to filter the results or not. All works fine but if I got for example files on resultpage 2 but not on page 1 I don't have any chance to show the filterd content. Because of the pagereload I'm starting with the first 10 results again.

Let's say I have 32 results. Even if I increase $defaultResultNumber to a higher Limit I get 4 Pages in the pagebrowser with the same results on every page. So the pagebrowser is useless. I'd like to use the paginate.widget but I'm not able because I can't set the defaultResultNumber.

Actions #7

Updated by Gerrit Code Review over 7 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50110

Actions #8

Updated by Gerrit Code Review over 7 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50110

Actions #9

Updated by Benni Mack over 7 years ago

  • Target version changed from 8.4 to 8.5
Actions #10

Updated by Gerrit Code Review over 7 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50110

Actions #11

Updated by Gerrit Code Review over 7 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50110

Actions #12

Updated by Gerrit Code Review over 7 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50110

Actions #13

Updated by Gerrit Code Review over 7 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50110

Actions #14

Updated by Karol Lamparski over 7 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #15

Updated by Riccardo De Contardi over 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF