From e8779e62934c6a61a6d056204c9bd404ee6ba1cd Mon Sep 17 00:00:00 2001 From: Krombel Date: Thu, 20 Jul 2017 17:30:04 +0200 Subject: [PATCH 1/7] send 404 as http-status when filter-id is unknown to the server This fixed the weirdness of 400 vs 404 as http status code in the case the filter id is not known by the server. As e.g. matrix-js-sdk expects 404 to catch this situation this leads to unwanted behaviour. --- synapse/rest/client/v2_alpha/filter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/filter.py b/synapse/rest/client/v2_alpha/filter.py index d2b2fd66e67f..2e6ba1b4ea52 100644 --- a/synapse/rest/client/v2_alpha/filter.py +++ b/synapse/rest/client/v2_alpha/filter.py @@ -61,7 +61,7 @@ def on_GET(self, request, user_id, filter_id): defer.returnValue((200, filter.get_filter_json())) except (KeyError, StoreError): - raise SynapseError(400, "No such filter", errcode=Codes.NOT_FOUND) + raise SynapseError(404, "No such filter", errcode=Codes.NOT_FOUND) class CreateFilterRestServlet(RestServlet): From 707f2b5802607f2f1279b1c2da6f1a5d394d37e5 Mon Sep 17 00:00:00 2001 From: Krombel Date: Thu, 20 Jul 2017 18:02:51 +0200 Subject: [PATCH 2/7] fix test to expect 404 instead of 400 for missing filter --- tests/rest/client/v2_alpha/test_filter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/v2_alpha/test_filter.py b/tests/rest/client/v2_alpha/test_filter.py index 76b833e11961..017419f298ad 100644 --- a/tests/rest/client/v2_alpha/test_filter.py +++ b/tests/rest/client/v2_alpha/test_filter.py @@ -118,7 +118,7 @@ def test_get_filter_non_existant(self): (code, response) = yield self.mock_resource.trigger_get( "/user/%s/filter/12382148321" % (self.USER_ID) ) - self.assertEquals(400, code) + self.assertEquals(404, code) self.assertEquals(response['errcode'], Codes.NOT_FOUND) # Currently invalid params do not have an appropriate errcode From 9480612a25e57ca3ed35cbfe855fa087abfd2245 Mon Sep 17 00:00:00 2001 From: Krombel Date: Fri, 21 Jul 2017 16:49:30 +0200 Subject: [PATCH 3/7] Enhance error for /sync when passed filter-id is not known respond 404 - No such filter with errorcode "M_NOT_FOUND" instead of 404 - No such row with errorcode "M_UNKNOWN" when filter-id is unknown to the server --- synapse/rest/client/v2_alpha/sync.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index 6dcc40745117..8a32f5deaa0f 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -25,7 +25,7 @@ serialize_event, format_event_for_client_v2_without_room_id, ) from synapse.api.filtering import FilterCollection, DEFAULT_FILTER_COLLECTION -from synapse.api.errors import SynapseError +from synapse.api.errors import Codes, StoreError, SynapseError from synapse.api.constants import PresenceState from ._base import client_v2_patterns from ._base import set_timeline_upper_limit @@ -130,9 +130,15 @@ def on_GET(self, request): self.filtering.check_valid_filter(filter_object) filter = FilterCollection(filter_object) else: - filter = yield self.filtering.get_user_filter( - user.localpart, filter_id - ) + try: + filter = yield self.filtering.get_user_filter( + user.localpart, filter_id + ) + except StoreError as err: + if err.code == 404 and err.errcode == Codes.UNKNOWN: + raise SynapseError(404, "No such filter", errcode=Codes.NOT_FOUND) + else: + raise err else: filter = DEFAULT_FILTER_COLLECTION From 86b7c8d26f97c1ae6ff2d36112c9074fade7bdbb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 8 May 2018 13:55:59 +0100 Subject: [PATCH 4/7] minor tweaks to sync code for non-existent filters --- synapse/rest/client/v2_alpha/sync.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index 8a32f5deaa0f..ceb2f682e583 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -136,9 +136,10 @@ def on_GET(self, request): ) except StoreError as err: if err.code == 404 and err.errcode == Codes.UNKNOWN: + # fix up the description and errcode to be more useful raise SynapseError(404, "No such filter", errcode=Codes.NOT_FOUND) else: - raise err + raise else: filter = DEFAULT_FILTER_COLLECTION From 1afd06a02abab0744839b5daa48be3844ea5fff2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 26 Jul 2019 13:27:32 +0100 Subject: [PATCH 5/7] Add changelog and lint --- changelog.d/2380.bugfix | 1 + synapse/rest/client/v2_alpha/sync.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog.d/2380.bugfix diff --git a/changelog.d/2380.bugfix b/changelog.d/2380.bugfix new file mode 100644 index 000000000000..4e006edebd99 --- /dev/null +++ b/changelog.d/2380.bugfix @@ -0,0 +1 @@ +Return an HTTP 404 instead of 400 when requesting a filter by ID that is unknown to the server. diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index 5b05a61171b0..626f30c2be31 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -138,7 +138,9 @@ def on_GET(self, request): except StoreError as err: if err.code == 404 and err.errcode == Codes.UNKNOWN: # fix up the description and errcode to be more useful - raise SynapseError(404, "No such filter", errcode=Codes.NOT_FOUND) + raise SynapseError( + 404, "No such filter", errcode=Codes.NOT_FOUND + ) else: raise else: From 01f6139b840e8761db3874b48675b87ad2d289e0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 3 Oct 2019 15:44:03 +0100 Subject: [PATCH 6/7] minor cleanups --- changelog.d/2380.bugfix | 2 +- synapse/rest/client/v2_alpha/sync.py | 50 +++++++++++++--------------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/changelog.d/2380.bugfix b/changelog.d/2380.bugfix index 4e006edebd99..eae32060316f 100644 --- a/changelog.d/2380.bugfix +++ b/changelog.d/2380.bugfix @@ -1 +1 @@ -Return an HTTP 404 instead of 400 when requesting a filter by ID that is unknown to the server. +Return an HTTP 404 instead of 400 when requesting a filter by ID that is unknown to the server. Thanks to @krombel for contributing this! diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index 626f30c2be31..9f7909a19554 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -119,36 +119,32 @@ def on_GET(self, request): request_key = (user, timeout, since, filter_id, full_state, device_id) - if filter_id: - if filter_id.startswith("{"): - try: - filter_object = json.loads(filter_id) - set_timeline_upper_limit( - filter_object, self.hs.config.filter_timeline_limit - ) - except Exception: - raise SynapseError(400, "Invalid filter JSON") - self.filtering.check_valid_filter(filter_object) - filter = FilterCollection(filter_object) - else: - try: - filter = yield self.filtering.get_user_filter( - user.localpart, filter_id - ) - except StoreError as err: - if err.code == 404 and err.errcode == Codes.UNKNOWN: - # fix up the description and errcode to be more useful - raise SynapseError( - 404, "No such filter", errcode=Codes.NOT_FOUND - ) - else: - raise + if filter_id is None: + filter_collection = DEFAULT_FILTER_COLLECTION + elif filter_id.startswith("{"): + try: + filter_object = json.loads(filter_id) + set_timeline_upper_limit( + filter_object, self.hs.config.filter_timeline_limit + ) + except Exception: + raise SynapseError(400, "Invalid filter JSON") + self.filtering.check_valid_filter(filter_object) + filter_collection = FilterCollection(filter_object) else: - filter = DEFAULT_FILTER_COLLECTION + try: + filter_collection = yield self.filtering.get_user_filter( + user.localpart, filter_id + ) + except StoreError as err: + if err.code != 404: + raise + # fix up the description and errcode to be more useful + raise SynapseError(400, "No such filter", errcode=Codes.INVALID_PARAM) sync_config = SyncConfig( user=user, - filter_collection=filter, + filter_collection=filter_collection, is_guest=requester.is_guest, request_key=request_key, device_id=device_id, @@ -182,7 +178,7 @@ def on_GET(self, request): time_now = self.clock.time_msec() response_content = yield self.encode_response( - time_now, sync_result, requester.access_token_id, filter + time_now, sync_result, requester.access_token_id, filter_collection ) return (200, response_content) From 298137e4c58e2869cf93d5e455a06893e183c086 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 3 Oct 2019 16:15:28 +0100 Subject: [PATCH 7/7] more minor tweaks --- synapse/rest/client/v2_alpha/filter.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/synapse/rest/client/v2_alpha/filter.py b/synapse/rest/client/v2_alpha/filter.py index d378329236c9..17a8bc7366fd 100644 --- a/synapse/rest/client/v2_alpha/filter.py +++ b/synapse/rest/client/v2_alpha/filter.py @@ -17,7 +17,7 @@ from twisted.internet import defer -from synapse.api.errors import AuthError, Codes, StoreError, SynapseError +from synapse.api.errors import AuthError, NotFoundError, StoreError, SynapseError from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.types import UserID @@ -52,13 +52,15 @@ def on_GET(self, request, user_id, filter_id): raise SynapseError(400, "Invalid filter_id") try: - filter = yield self.filtering.get_user_filter( + filter_collection = yield self.filtering.get_user_filter( user_localpart=target_user.localpart, filter_id=filter_id ) + except StoreError as e: + if e.code != 404: + raise + raise NotFoundError("No such filter") - return 200, filter.get_filter_json() - except (KeyError, StoreError): - raise SynapseError(404, "No such filter", errcode=Codes.NOT_FOUND) + return 200, filter_collection.get_filter_json() class CreateFilterRestServlet(RestServlet):