Request & follow redirects for /media/v3/download (#16701)

Implement MSC3860 to follow redirects for federated media downloads.

Note that the Client-Server API doesn't support this (yet) since the media
repository in Synapse doesn't have a way of supporting redirects.

1 files changed, 57 insertions, 20 deletions

diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index d5013e8e97..cc1db763ae 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -153,12 +153,18 @@ class MatrixFederationRequest:
     """Query arguments.
     """
 
-    txn_id: Optional[str] = None
-    """Unique ID for this request (for logging)
+    txn_id: str = attr.ib(init=False)
+    """Unique ID for this request (for logging), this is autogenerated.
     """
 
-    uri: bytes = attr.ib(init=False)
-    """The URI of this request
+    uri: bytes = b""
+    """The URI of this request, usually generated from the above information.
+    """
+
+    _generate_uri: bool = True
+    """True to automatically generate the uri field based on the above information.
+
+    Set to False if manually configuring the URI.
     """
 
     def __attrs_post_init__(self) -> None:
@@ -168,22 +174,23 @@ class MatrixFederationRequest:
 
         object.__setattr__(self, "txn_id", txn_id)
 
-        destination_bytes = self.destination.encode("ascii")
-        path_bytes = self.path.encode("ascii")
-        query_bytes = encode_query_args(self.query)
-
-        # The object is frozen so we can pre-compute this.
-        uri = urllib.parse.urlunparse(
-            (
-                b"matrix-federation",
-                destination_bytes,
-                path_bytes,
-                None,
-                query_bytes,
-                b"",
+        if self._generate_uri:
+            destination_bytes = self.destination.encode("ascii")
+            path_bytes = self.path.encode("ascii")
+            query_bytes = encode_query_args(self.query)
+
+            # The object is frozen so we can pre-compute this.
+            uri = urllib.parse.urlunparse(
+                (
+                    b"matrix-federation",
+                    destination_bytes,
+                    path_bytes,
+                    None,
+                    query_bytes,
+                    b"",
+                )
             )
-        )
-        object.__setattr__(self, "uri", uri)
+            object.__setattr__(self, "uri", uri)
 
     def get_json(self) -> Optional[JsonDict]:
         if self.json_callback:
@@ -513,6 +520,7 @@ class MatrixFederationHttpClient:
         ignore_backoff: bool = False,
         backoff_on_404: bool = False,
         backoff_on_all_error_codes: bool = False,
+        follow_redirects: bool = False,
     ) -> IResponse:
         """
         Sends a request to the given server.
@@ -555,6 +563,9 @@ class MatrixFederationHttpClient:
             backoff_on_404: Back off if we get a 404
             backoff_on_all_error_codes: Back off if we get any error response
 
+            follow_redirects: True to follow the Location header of 307/308 redirect
+                responses. This does not recurse.
+
         Returns:
             Resolves with the HTTP response object on success.
 
@@ -714,6 +725,26 @@ class MatrixFederationHttpClient:
                             response.code,
                             response_phrase,
                         )
+                    elif (
+                        response.code in (307, 308)
+                        and follow_redirects
+                        and response.headers.hasHeader("Location")
+                    ):
+                        # The Location header *might* be relative so resolve it.
+                        location = response.headers.getRawHeaders(b"Location")[0]
+                        new_uri = urllib.parse.urljoin(request.uri, location)
+
+                        return await self._send_request(
+                            attr.evolve(request, uri=new_uri, generate_uri=False),
+                            retry_on_dns_fail,
+                            timeout,
+                            long_retries,
+                            ignore_backoff,
+                            backoff_on_404,
+                            backoff_on_all_error_codes,
+                            # Do not continue following redirects.
+                            follow_redirects=False,
+                        )
                     else:
                         logger.info(
                             "{%s} [%s] Got response headers: %d %s",
@@ -1383,6 +1414,7 @@ class MatrixFederationHttpClient:
         retry_on_dns_fail: bool = True,
         max_size: Optional[int] = None,
         ignore_backoff: bool = False,
+        follow_redirects: bool = False,
     ) -> Tuple[int, Dict[bytes, List[bytes]]]:
         """GETs a file from a given homeserver
         Args:
@@ -1392,6 +1424,8 @@ class MatrixFederationHttpClient:
             args: Optional dictionary used to create the query string.
             ignore_backoff: true to ignore the historical backoff data
                 and try the request anyway.
+            follow_redirects: True to follow the Location header of 307/308 redirect
+                responses. This does not recurse.
 
         Returns:
             Resolves with an (int,dict) tuple of
@@ -1412,7 +1446,10 @@ class MatrixFederationHttpClient:
         )
 
         response = await self._send_request(
-            request, retry_on_dns_fail=retry_on_dns_fail, ignore_backoff=ignore_backoff
+            request,
+            retry_on_dns_fail=retry_on_dns_fail,
+            ignore_backoff=ignore_backoff,
+            follow_redirects=follow_redirects,
         )
 
         headers = dict(response.headers.getAllRawHeaders())