Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix a long-standing bug where the user directory would return 1 more row than requested. #14631

Merged
merged 4 commits into from
Dec 7, 2022
Merged

Fix a long-standing bug where the user directory would return 1 more row than requested. #14631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c5c38db
Respect the limit in the return of the user directory
reivilibre Dec 6, 2022
0501990
Add a test to ensure this doesn't happen again
reivilibre Dec 6, 2022
2f2528a
Clamp `limit` because it would be janky for negative numbers to slice…
reivilibre Dec 6, 2022
b9b4228
Newsfile
reivilibre Dec 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14631.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing bug where the user directory would return 1 more row than requested.
4 changes: 2 additions & 2 deletions synapse/rest/client/user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonMapping]:

body = parse_json_object_from_request(request)

limit = body.get("limit", 10)
limit = min(limit, 50)
limit = int(body.get("limit", 10))
limit = max(min(limit, 50), 0)
Comment on lines -66 to +67
Copy link
Contributor

Choose a reason for hiding this comment

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

The call to int sneakily allows the limit to be given as a string. I can look the other way for a drive-by fix though.


try:
search_term = body["search_term"]
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ async def search_user_dir(

limited = len(results) > limit

return {"limited": limited, "results": results}
return {"limited": limited, "results": results[0:limit]}


def _parse_query_sqlite(search_term: str) -> str:
Expand Down
6 changes: 6 additions & 0 deletions tests/storage/test_user_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,12 @@ def test_search_user_dir_all_users(self) -> None:
{"user_id": BOBBY, "display_name": "bobby", "avatar_url": None},
)

@override_config({"user_directory": {"search_all_users": True}})
def test_search_user_limit_correct(self) -> None:
r = self.get_success(self.store.search_user_dir(ALICE, "bob", 1))
self.assertTrue(r["limited"])
self.assertEqual(1, len(r["results"]))

@override_config({"user_directory": {"search_all_users": True}})
def test_search_user_dir_stop_words(self) -> None:
"""Tests that a user can look up another user by searching for the start if its
Expand Down