i should have given up x2

2 files changed, 52 insertions, 18 deletions

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 8ad28d1e21..74ea6bcf8e 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -190,11 +190,11 @@ class MatrixFederationHttpClient(object):
 
     @defer.inlineCallbacks
     def _send_request_with_optional_trailing_slash(
-            self,
-            request,
-            try_trailing_slash_on_400=False,
-            backoff_on_404=False,
-            send_request_args={},
+        self,
+        request,
+        try_trailing_slash_on_400=False,
+        backoff_on_404=False,
+        send_request_args={},
     ):
         """Wrapper for _send_request which can optionally retry the request
         upon receiving a combination of a 400 HTTP response code and a
@@ -213,8 +213,7 @@ class MatrixFederationHttpClient(object):
                 `_send_request()`.
 
         Returns:
-            Deferred[twisted.web.client.Response]: resolves with the HTTP
-            response object on success.
+            Deferred[Dict]: Parsed JSON response body.
         """
         response = yield self._send_request(**send_request_args)
 
@@ -236,7 +235,11 @@ class MatrixFederationHttpClient(object):
             send_request_args["path"] += "/"
             response = yield self._send_request(**send_request_args)
 
-        defer.returnValue(response)
+            body = yield _handle_json_response(
+                self.hs.get_reactor(), self.default_timeout, request, response,
+            )
+
+        defer.returnValue(body)
 
     @defer.inlineCallbacks
     def _send_request(
@@ -585,14 +588,10 @@ class MatrixFederationHttpClient(object):
             "backoff_on_404": False if try_trailing_slash_on_400 else backoff_on_404,
         }
 
-        response = yield self._send_request_with_optional_trailing_slash(
+        body = yield self._send_request_with_optional_trailing_slash(
             request, try_trailing_slash_on_400, backoff_on_404, send_request_args,
         )
 
-        body = yield _handle_json_response(
-            self.hs.get_reactor(), self.default_timeout, request, response,
-        )
-
         defer.returnValue(body)
 
     @defer.inlineCallbacks
@@ -706,14 +705,10 @@ class MatrixFederationHttpClient(object):
             "backoff_on_404": False,
         }
 
-        response = yield self._send_request_with_optional_trailing_slash(
+        body = yield self._send_request_with_optional_trailing_slash(
             request, try_trailing_slash_on_400, False, send_request_args,
         )
 
-        body = yield _handle_json_response(
-            self.hs.get_reactor(), self.default_timeout, request, response,
-        )
-
         defer.returnValue(body)
 
     @defer.inlineCallbacks
diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py
index b03b37affe..0d0161d13d 100644
--- a/tests/http/test_fedclient.py
+++ b/tests/http/test_fedclient.py
@@ -268,6 +268,45 @@ class FederationClientTests(HomeserverTestCase):
 
         self.assertIsInstance(f.value, TimeoutError)
 
+    def test_client_requires_trailing_slashes(self):
+        """
+        If a connection is made to a client but the client rejects it due to
+        requiring a trailing slash. We need to retry the request with a
+        trailing slash. Workaround for Synapse <=v0.99.2, explained in #3622.
+        """
+        d = self.cl.get_json(
+            "testserv:8008", "foo/bar", try_trailing_slash_on_400=True,
+        )
+
+        self.pump()
+
+        # there should have been a call to connectTCP
+        clients = self.reactor.tcpClients
+        self.assertEqual(len(clients), 1)
+        (_host, _port, factory, _timeout, _bindAddress) = clients[0]
+
+        # complete the connection and wire it up to a fake transport
+        client = factory.buildProtocol(None)
+        conn = StringTransport()
+        client.makeConnection(conn)
+
+        # that should have made it send the request to the connection
+        self.assertRegex(conn.value(), b"^GET /foo/bar")
+
+        # Send the HTTP response
+        client.dataReceived(
+            b"HTTP/1.1 400 Bad Request\r\n"
+            b"Content-Type: application/json\r\n"
+            b"Content-Length: 59\r\n"
+            b"\r\n"
+            b'{"errcode":"M_UNRECOGNIZED","error":"Unrecognized request"}'
+        )
+
+        # We should get a successful response
+        r = self.successResultOf(d)
+        self.assertEqual(r.code, 400)
+        self.assertEqual(r, {})
+
     def test_client_sends_body(self):
         self.cl.post_json(
             "testserv:8008", "foo/bar", timeout=10000,