From a0669412da24ebda718c7ddc8243ca960ff77509 Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 08:25:30 -0700 Subject: [PATCH 01/13] add tests for checking if room search works with non-ascii char --- tests/rest/admin/test_room.py | 44 +++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 40e032df7f8c..33244ca841d7 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -941,6 +941,50 @@ def _search_test( _search_test(None, "bar") _search_test(None, "", expected_http_code=400) + def test_search_term_non_ASCII(self): + """Test that searching for a room with non-ASCII characters works correctly""" + + # Create test room + room_id = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok) + + room_name = "ж" + + # Set the name for the room + self.helper.send_state( + room_id, + "m.room.name", + {"name": room_name}, + tok=self.admin_user_tok, + ) + + def _search_test_utf8( + expected_room_id, + search_term: str, + expected_http_code: int = 200, + ): + """Search for a room and check that the returned room's id is a match + + Args: + expected_room_id: The room_id expected to be returned by the API. Set + to None to expect zero results for the search + search_term: The term to search for room names with + expected_http_code: The expected http code for the request + """ + url = "/_synapse/admin/v1/rooms?search_term=%s" % (search_term,) + channel = self.make_request( + "GET", + url.encode("UTF-8"), + access_token=self.admin_user_tok, + ) + self.assertEqual(expected_http_code, channel.code, msg=channel.json_body) + self.assertIn(expected_room_id, channel.json_body.get('rooms')[0].get('room_id')) + self.assertIn("ж", channel.json_body.get('rooms')[0].get('name')) + + search_term = "ж" + _search_test_utf8(room_id, search_term) + + + def test_single_room(self): """Test that a single room can be requested correctly""" # Create two test rooms From 6435581cac034a7e123a6264664b50183cacc51f Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 08:26:25 -0700 Subject: [PATCH 02/13] change encoding on parse_string to UTF-8 --- synapse/http/servlet.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 91ba93372c2c..6fb3d7c6b739 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -331,7 +331,7 @@ def parse_string( default: Optional[str] = None, required: bool = False, allowed_values: Optional[Iterable[str]] = None, - encoding: str = "ascii", + encoding: str = "UTF-8", ) -> Optional[str]: """ Parse a string parameter from the request query string. From 65d34f9bb4f0c3431ae200d65bfe51075070ae3d Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 08:39:46 -0700 Subject: [PATCH 03/13] lints --- tests/rest/admin/test_room.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 33244ca841d7..e7e3f8d1299a 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -977,14 +977,14 @@ def _search_test_utf8( access_token=self.admin_user_tok, ) self.assertEqual(expected_http_code, channel.code, msg=channel.json_body) - self.assertIn(expected_room_id, channel.json_body.get('rooms')[0].get('room_id')) - self.assertIn("ж", channel.json_body.get('rooms')[0].get('name')) + self.assertIn( + expected_room_id, channel.json_body.get("rooms")[0].get("room_id") + ) + self.assertIn("ж", channel.json_body.get("rooms")[0].get("name")) search_term = "ж" _search_test_utf8(room_id, search_term) - - def test_single_room(self): """Test that a single room can be requested correctly""" # Create two test rooms From bd4fc33151a438943cb8d40baf1caef2ae9736bd Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 10:11:18 -0700 Subject: [PATCH 04/13] properly encode search term --- tests/rest/admin/test_room.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index e7e3f8d1299a..35781e5ad8a6 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -962,7 +962,8 @@ def _search_test_utf8( search_term: str, expected_http_code: int = 200, ): - """Search for a room and check that the returned room's id is a match + """Search for a room with a non-ascii character in name + and check that the returned room's id is a match Args: expected_room_id: The room_id expected to be returned by the API. Set @@ -970,10 +971,11 @@ def _search_test_utf8( search_term: The term to search for room names with expected_http_code: The expected http code for the request """ - url = "/_synapse/admin/v1/rooms?search_term=%s" % (search_term,) + encoded_search_term = urllib.parse.quote(search_term, 'utf-8') + url = "/_synapse/admin/v1/rooms?search_term=%s" % (encoded_search_term,) channel = self.make_request( "GET", - url.encode("UTF-8"), + url.encode("ascii"), access_token=self.admin_user_tok, ) self.assertEqual(expected_http_code, channel.code, msg=channel.json_body) From 7f910f03f6057dd8e58ec8d68dcc5b8047308c0d Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 10:38:39 -0700 Subject: [PATCH 05/13] lints --- tests/rest/admin/test_room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 35781e5ad8a6..11a5bc000bcb 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -971,7 +971,7 @@ def _search_test_utf8( search_term: The term to search for room names with expected_http_code: The expected http code for the request """ - encoded_search_term = urllib.parse.quote(search_term, 'utf-8') + encoded_search_term = urllib.parse.quote(search_term, "utf-8") url = "/_synapse/admin/v1/rooms?search_term=%s" % (encoded_search_term,) channel = self.make_request( "GET", From 056a5188563f2d2904924c8fc7cdde0dbcebe719 Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 10:51:26 -0700 Subject: [PATCH 06/13] add changelog file --- changelog.d/10858.misc | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog.d/10858.misc diff --git a/changelog.d/10858.misc b/changelog.d/10858.misc new file mode 100644 index 000000000000..c632bfe11b63 --- /dev/null +++ b/changelog.d/10858.misc @@ -0,0 +1,2 @@ +Made a change that Synapse Admin API's Rooms Search so that it can now accept non-ASCII characters, allowing admins +to find rooms with non-ASCII characters. \ No newline at end of file From 0f20682af4c875d01c2f5f2b50fd96f968f4b83a Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 11:01:43 -0700 Subject: [PATCH 07/13] update changelog number --- changelog.d/{10858.misc => 10859.misc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10858.misc => 10859.misc} (100%) diff --git a/changelog.d/10858.misc b/changelog.d/10859.misc similarity index 100% rename from changelog.d/10858.misc rename to changelog.d/10859.misc From be8e5a314251438ec4ec7dbc59ba32162c93e550 Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 13:19:35 -0700 Subject: [PATCH 08/13] set changelog entry filetype to .bugfix --- changelog.d/{10859.misc => 10859.bugfix} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10859.misc => 10859.bugfix} (100%) diff --git a/changelog.d/10859.misc b/changelog.d/10859.bugfix similarity index 100% rename from changelog.d/10859.misc rename to changelog.d/10859.bugfix From 7a333e68ccf023245d90e6930a4c44ebffeac59c Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 13:24:26 -0700 Subject: [PATCH 09/13] Revert "set changelog entry filetype to .bugfix" This reverts commit be8e5a314251438ec4ec7dbc59ba32162c93e550. --- changelog.d/{10859.bugfix => 10859.misc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10859.bugfix => 10859.misc} (100%) diff --git a/changelog.d/10859.bugfix b/changelog.d/10859.misc similarity index 100% rename from changelog.d/10859.bugfix rename to changelog.d/10859.misc From a05e152b2b12ba51cf29d4ff3954ce3c7d3a4bb6 Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 13:27:34 -0700 Subject: [PATCH 10/13] update changelog message and file type --- changelog.d/10859.bugfix | 1 + changelog.d/10859.misc | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) create mode 100644 changelog.d/10859.bugfix delete mode 100644 changelog.d/10859.misc diff --git a/changelog.d/10859.bugfix b/changelog.d/10859.bugfix new file mode 100644 index 000000000000..c1bfe22d5405 --- /dev/null +++ b/changelog.d/10859.bugfix @@ -0,0 +1 @@ +Fix a bug in Unicode support of the room search admin API. It is now possible to search for rooms with non-ASCII characters. \ No newline at end of file diff --git a/changelog.d/10859.misc b/changelog.d/10859.misc deleted file mode 100644 index c632bfe11b63..000000000000 --- a/changelog.d/10859.misc +++ /dev/null @@ -1,2 +0,0 @@ -Made a change that Synapse Admin API's Rooms Search so that it can now accept non-ASCII characters, allowing admins -to find rooms with non-ASCII characters. \ No newline at end of file From bb24df467e7704fda1f848c1f65cba7513346d0f Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 13:30:08 -0700 Subject: [PATCH 11/13] change parse_string default encoding back to ascii and update room search admin api calll to parse string --- synapse/http/servlet.py | 2 +- synapse/rest/admin/rooms.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 6fb3d7c6b739..91ba93372c2c 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -331,7 +331,7 @@ def parse_string( default: Optional[str] = None, required: bool = False, allowed_values: Optional[Iterable[str]] = None, - encoding: str = "UTF-8", + encoding: str = "ascii", ) -> Optional[str]: """ Parse a string parameter from the request query string. diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index ad83d4b54c64..8f781f745fb9 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -125,7 +125,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: errcode=Codes.INVALID_PARAM, ) - search_term = parse_string(request, "search_term") + search_term = parse_string(request, "search_term", encoding="utf-8") if search_term == "": raise SynapseError( 400, From 679f9fa5310176229755840d1f5027e3931318a6 Mon Sep 17 00:00:00 2001 From: "H.Shay" Date: Mon, 20 Sep 2021 13:39:04 -0700 Subject: [PATCH 12/13] refactor tests --- tests/rest/admin/test_room.py | 43 ++++++++++------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 11a5bc000bcb..88aa6aba8ff2 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -941,12 +941,11 @@ def _search_test( _search_test(None, "bar") _search_test(None, "", expected_http_code=400) - def test_search_term_non_ASCII(self): + def test_search_term_non_ascii(self): """Test that searching for a room with non-ASCII characters works correctly""" # Create test room room_id = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok) - room_name = "ж" # Set the name for the room @@ -957,35 +956,17 @@ def test_search_term_non_ASCII(self): tok=self.admin_user_tok, ) - def _search_test_utf8( - expected_room_id, - search_term: str, - expected_http_code: int = 200, - ): - """Search for a room with a non-ascii character in name - and check that the returned room's id is a match - - Args: - expected_room_id: The room_id expected to be returned by the API. Set - to None to expect zero results for the search - search_term: The term to search for room names with - expected_http_code: The expected http code for the request - """ - encoded_search_term = urllib.parse.quote(search_term, "utf-8") - url = "/_synapse/admin/v1/rooms?search_term=%s" % (encoded_search_term,) - channel = self.make_request( - "GET", - url.encode("ascii"), - access_token=self.admin_user_tok, - ) - self.assertEqual(expected_http_code, channel.code, msg=channel.json_body) - self.assertIn( - expected_room_id, channel.json_body.get("rooms")[0].get("room_id") - ) - self.assertIn("ж", channel.json_body.get("rooms")[0].get("name")) - - search_term = "ж" - _search_test_utf8(room_id, search_term) + # make the request and test that the response is what we wanted + search_term = urllib.parse.quote("ж", "utf-8") + url = "/_synapse/admin/v1/rooms?search_term=%s" % (search_term,) + channel = self.make_request( + "GET", + url.encode("ascii"), + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertIn(room_id, channel.json_body.get("rooms")[0].get("room_id")) + self.assertIn("ж", channel.json_body.get("rooms")[0].get("name")) def test_single_room(self): """Test that a single room can be requested correctly""" From 5ae94e6837864f4f870e24a6c4c775e05e79b5d1 Mon Sep 17 00:00:00 2001 From: Hillery Shay Date: Tue, 21 Sep 2021 07:41:21 -0700 Subject: [PATCH 13/13] Update tests/rest/admin/test_room.py Co-authored-by: Patrick Cloke --- tests/rest/admin/test_room.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 88aa6aba8ff2..e798513ac1c9 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -965,8 +965,8 @@ def test_search_term_non_ascii(self): access_token=self.admin_user_tok, ) self.assertEqual(200, channel.code, msg=channel.json_body) - self.assertIn(room_id, channel.json_body.get("rooms")[0].get("room_id")) - self.assertIn("ж", channel.json_body.get("rooms")[0].get("name")) + self.assertEqual(room_id, channel.json_body.get("rooms")[0].get("room_id")) + self.assertEqual("ж", channel.json_body.get("rooms")[0].get("name")) def test_single_room(self): """Test that a single room can be requested correctly"""