Skip to content

Conversation

@mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Apr 3, 2025

An empty search query is expensive and should not be permitted, as it returns every record in the keyboard table, rather than a subset.

Note: this is a mitigation, as no rate limiting is supported. v2.0 search supports paging, so perhaps we should consider retiring v1.0 search altogether.

An empty search query is expensive and should not be permitted, as it
returns every record in the keyboard table, rather than a subset.

Note: this is a mitigation, as no rate limiting is supported. v2.0
search supports paging, so perhaps we should consider retiring v1.0
search altogether.
@mcdurdin mcdurdin added this to the B18S5 milestone Apr 3, 2025
@mcdurdin mcdurdin added the chore label Apr 3, 2025
@mcdurdin mcdurdin requested review from darcywong00 and tim-eves April 3, 2025 11:18
$q = $_REQUEST['q'];
$q = trim($_REQUEST['q']);

if($q == '') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we allow q=*?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we do for now. That returns 45kb result set and is much quicker to run.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q=% on the other hand has the same problem as q=. But one step at a time huh?

Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcdurdin mcdurdin merged commit c3dddb0 into master Apr 4, 2025
3 checks passed
@mcdurdin mcdurdin deleted the chore/restrict-empty-search-query branch April 4, 2025 01:42
@github-project-automation github-project-automation bot moved this to Done in Keyman Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants