Fix /key/v2/server calls with URL-unsafe key IDs (#14490)

Co-authored-by: Patrick Cloke <[email protected]>

Author: David Robertson

changelog.d/14490.misc

@@ -0,0 +1 @@
+Fix a bug introduced in Synapse 0.9 where it would fail to fetch server keys whose IDs contain a forward slash.

synapse/crypto/keyring.py

@@ -857,7 +857,7 @@ async def get_server_verify_key_v2_direct(
                 response = await self.client.get_json(
                     destination=server_name,
                     path="/_matrix/key/v2/server/"
-                    + urllib.parse.quote(requested_key_id),
+                    + urllib.parse.quote(requested_key_id, safe=""),
                     ignore_backoff=True,
                     # we only give the remote server 10s to respond. It should be an
                     # easy request to handle, so if it doesn't reply within 10s, it's

tests/crypto/test_keyring.py

@@ -469,6 +469,18 @@ async def get_json(destination, path, **kwargs):
         keys = self.get_success(fetcher.get_keys(SERVER_NAME, ["key1"], 0))
         self.assertEqual(keys, {})
 
+    def test_keyid_containing_forward_slash(self) -> None:
+        """We should url-encode any url unsafe chars in key ids.
+
+        Detects https://github.com/matrix-org/synapse/issues/14488.
+        """
+        fetcher = ServerKeyFetcher(self.hs)
+        self.get_success(fetcher.get_keys("example.com", ["key/potato"], 0))
+
+        self.http_client.get_json.assert_called_once()
+        args, kwargs = self.http_client.get_json.call_args
+        self.assertEqual(kwargs["path"], "/_matrix/key/v2/server/key%2Fpotato")
+
 
 class PerspectivesKeyFetcherTestCase(unittest.HomeserverTestCase):
     def make_homeserver(self, reactor, clock):