Make federation endpoints more tolerant of trailing slashes v2 (#4935)

Redo of https://github.com/matrix-org/synapse/pull/4840

5 files changed, 13 insertions, 11 deletions

diff --git a/changelog.d/4793.feature b/changelog.d/4793.feature
new file mode 100644
index 0000000000..90dba7d122
--- /dev/null
+++ b/changelog.d/4793.feature
@@ -0,0 +1 @@
+Synapse is now permissive about trailing slashes on some of its federation endpoints, allowing zero or more to be present.
\ No newline at end of file
diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index 0cdb31178f..e424c40fdf 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -173,7 +173,7 @@ class TransportLayerClient(object):
         # generated by the json_data_callback.
         json_data = transaction.get_dict()
 
-        path = _create_v1_path("/send/%s/", transaction.transaction_id)
+        path = _create_v1_path("/send/%s", transaction.transaction_id)
 
         response = yield self.client.put_json(
             transaction.destination,
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 96d680a5ad..efb6bdca48 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -312,7 +312,7 @@ class BaseFederationServlet(object):
 
 
 class FederationSendServlet(BaseFederationServlet):
-    PATH = "/send/(?P<transaction_id>[^/]*)/"
+    PATH = "/send/(?P<transaction_id>[^/]*)/?"
 
     def __init__(self, handler, server_name, **kwargs):
         super(FederationSendServlet, self).__init__(
@@ -378,7 +378,7 @@ class FederationSendServlet(BaseFederationServlet):
 
 
 class FederationEventServlet(BaseFederationServlet):
-    PATH = "/event/(?P<event_id>[^/]*)/"
+    PATH = "/event/(?P<event_id>[^/]*)/?"
 
     # This is when someone asks for a data item for a given server data_id pair.
     def on_GET(self, origin, content, query, event_id):
@@ -386,7 +386,7 @@ class FederationEventServlet(BaseFederationServlet):
 
 
 class FederationStateServlet(BaseFederationServlet):
-    PATH = "/state/(?P<context>[^/]*)/"
+    PATH = "/state/(?P<context>[^/]*)/?"
 
     # This is when someone asks for all data for a given context.
     def on_GET(self, origin, content, query, context):
@@ -398,7 +398,7 @@ class FederationStateServlet(BaseFederationServlet):
 
 
 class FederationStateIdsServlet(BaseFederationServlet):
-    PATH = "/state_ids/(?P<room_id>[^/]*)/"
+    PATH = "/state_ids/(?P<room_id>[^/]*)/?"
 
     def on_GET(self, origin, content, query, room_id):
         return self.handler.on_state_ids_request(
@@ -409,7 +409,7 @@ class FederationStateIdsServlet(BaseFederationServlet):
 
 
 class FederationBackfillServlet(BaseFederationServlet):
-    PATH = "/backfill/(?P<context>[^/]*)/"
+    PATH = "/backfill/(?P<context>[^/]*)/?"
 
     def on_GET(self, origin, content, query, context):
         versions = [x.decode('ascii') for x in query[b"v"]]
@@ -1080,7 +1080,7 @@ class FederationGroupsCategoriesServlet(BaseFederationServlet):
     """Get all categories for a group
     """
     PATH = (
-        "/groups/(?P<group_id>[^/]*)/categories/"
+        "/groups/(?P<group_id>[^/]*)/categories/?"
     )
 
     @defer.inlineCallbacks
@@ -1150,7 +1150,7 @@ class FederationGroupsRolesServlet(BaseFederationServlet):
     """Get roles in a group
     """
     PATH = (
-        "/groups/(?P<group_id>[^/]*)/roles/"
+        "/groups/(?P<group_id>[^/]*)/roles/?"
     )
 
     @defer.inlineCallbacks
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 8e855d13d6..ff63d0b2a8 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -231,6 +231,7 @@ class MatrixFederationHttpClient(object):
             # Retry with a trailing slash if we received a 400 with
             # 'M_UNRECOGNIZED' which some endpoints can return when omitting a
             # trailing slash on Synapse <= v0.99.3.
+            logger.info("Retrying request with trailing slash")
             request.path += "/"
 
             response = yield self._send_request(
diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index 7decb22933..6460cbc708 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -180,7 +180,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
         put_json = self.hs.get_http_client().put_json
         put_json.assert_called_once_with(
             "farm",
-            path="/_matrix/federation/v1/send/1000000/",
+            path="/_matrix/federation/v1/send/1000000",
             data=_expect_edu_transaction(
                 "m.typing",
                 content={
@@ -202,7 +202,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
 
         (request, channel) = self.make_request(
             "PUT",
-            "/_matrix/federation/v1/send/1000000/",
+            "/_matrix/federation/v1/send/1000000",
             _make_edu_transaction_json(
                 "m.typing",
                 content={
@@ -258,7 +258,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
         put_json = self.hs.get_http_client().put_json
         put_json.assert_called_once_with(
             "farm",
-            path="/_matrix/federation/v1/send/1000000/",
+            path="/_matrix/federation/v1/send/1000000",
             data=_expect_edu_transaction(
                 "m.typing",
                 content={