⏺ Here's a summary of the HTTP client migration:

Completed:
  - Created NativeSimpleHttpClient with a HomeServer-compatible constructor that accepts both hs and explicit params
  - Made session creation lazy (deferred to first use) to avoid "no running event loop" during homeserver init
  - Updated get_file to return dict[bytes, list[bytes]] headers for backward compatibility
  - Added SimpleHttpClient = NativeSimpleHttpClient alias for drop-in replacement
  - Wired into server.py: get_simple_http_client(), get_proxied_http_client(), get_proxied_blocklisted_http_client() all return NativeSimpleHttpClient
  - Updated all imports: url_previewer.py, appservice/api.py, identity.py, module_api/__init__.py, matrixfederationclient.py
  - Fixed handlers/oidc.py to use aiohttp response API (.status, .reason, .read(), plain dict headers)
  - Updated test mock tests/test_utils/oidc.py to use plain dict headers and NativeFakeResponse
  - Created NativeFakeResponse test utility class

  Still using Twisted HTTP client:
  - ReplicationClient — routes to worker instances via TCP/UNIX sockets
  - matrixfederationclient.py — federation agent with SRV resolution, well-known lookup, TLS verification
  - connectproxyclient.py — CONNECT proxy protocol implementation
  - Various test files using FakeResponse (old Twisted version)
This commit is contained in:
Matthew Hodgson
2026-03-22 02:40:41 +00:00
parent ac8077bcee
commit caaeb75508
10 changed files with 146 additions and 99 deletions
+2 -1
View File
@@ -41,7 +41,8 @@ from synapse.appservice import (
)
from synapse.events import EventBase
from synapse.events.utils import SerializeEventConfig, serialize_event
from synapse.http.client import SimpleHttpClient, is_unknown_endpoint
from synapse.http.client import is_unknown_endpoint
from synapse.http.native_client import SimpleHttpClient
from synapse.logging import opentracing
from synapse.metrics import SERVER_NAME_LABEL
from synapse.types import DeviceListUpdates, JsonDict, JsonMapping, ThirdPartyInstanceID
+1 -1
View File
@@ -36,7 +36,7 @@ from synapse.api.errors import (
)
from synapse.api.ratelimiting import Ratelimiter
from synapse.http import RequestTimedOutError
from synapse.http.client import SimpleHttpClient
from synapse.http.native_client import SimpleHttpClient
from synapse.http.site import SynapseRequest
from synapse.types import JsonDict, Requester
from synapse.util.hash import sha256_and_url_safe_base64
+12 -22
View File
@@ -49,12 +49,6 @@ from pymacaroons.exceptions import (
MacaroonInvalidSignatureException,
)
try:
from twisted.web.client import readBody
from twisted.web.http_headers import Headers
except ImportError:
pass
from synapse.api.errors import SynapseError
from synapse.config import ConfigError
from synapse.config.oidc import OidcProviderClientSecretJwtKey, OidcProviderConfig
@@ -62,7 +56,6 @@ from synapse.handlers.sso import MappingException, UserAttributes
from synapse.http.server import finish_request
from synapse.http.servlet import parse_string
from synapse.http.site import SynapseRequest
from synapse.logging.context import make_deferred_yieldable
from synapse.module_api import ModuleApi
from synapse.types import JsonDict, UserID, map_username_to_mxid_localpart
from synapse.util.caches.cached_call import RetryOnExceptionCachedCall
@@ -760,7 +753,7 @@ class OidcProvider:
token_endpoint = metadata.get("token_endpoint")
raw_headers: dict[str, str] = {
"Content-Type": "application/x-www-form-urlencoded",
"User-Agent": self._http_client.user_agent.decode("ascii"),
"User-Agent": self._http_client.user_agent if isinstance(self._http_client.user_agent, str) else self._http_client.user_agent.decode("ascii"),
"Accept": "application/json",
}
@@ -777,7 +770,7 @@ class OidcProvider:
uri, raw_headers, body = self._client_auth.prepare(
method="POST", uri=token_endpoint, headers=raw_headers, body=body
)
headers = Headers({k: [v] for (k, v) in raw_headers.items()})
headers = {k: v for (k, v) in raw_headers.items()}
# Do the actual request
# We're not using the SimpleHttpClient util methods as we don't want to
@@ -791,12 +784,12 @@ class OidcProvider:
# This is used in multiple error messages below
status = "{code} {phrase}".format(
code=response.code, phrase=response.phrase.decode("utf-8")
code=response.status, phrase=response.reason or ""
)
resp_body = await make_deferred_yieldable(readBody(response))
resp_body = await response.read()
if response.code >= 500:
if response.status >= 500:
# In case of a server error, we should first try to decode the body
# and check for an error field. If not, we respond with a generic
# error message.
@@ -825,7 +818,7 @@ class OidcProvider:
# In case the authorization server responded with an error field,
# it should be a 4xx code. If not, warn about it but don't do
# anything special and report the original error message.
if response.code < 400:
if response.status < 400:
logger.debug(
"Invalid response from the authorization server: "
'responded with a "%s" '
@@ -840,7 +833,7 @@ class OidcProvider:
# Now, this should not be an error. According to RFC6749 sec 5.1, it
# should be a 200 code. We're a bit more flexible than that, and will
# only throw on a 4xx code.
if response.code >= 400:
if response.status >= 400:
description = (
'Authorization server responded with a "{status}" error '
'but did not include an "error" field in its response.'.format(
@@ -870,18 +863,15 @@ class OidcProvider:
resp = await self._http_client.request(
"GET",
metadata["userinfo_endpoint"],
headers=Headers(
{"Authorization": ["Bearer {}".format(token["access_token"])]}
),
headers={"Authorization": "Bearer {}".format(token["access_token"])},
)
body = await readBody(resp)
body = await resp.read()
content_type_headers = resp.headers.getRawHeaders("Content-Type")
assert content_type_headers
content_type = resp.headers.get("Content-Type", "")
# We use `startswith` because the header value can contain the `charset` parameter
# even if it is useless, and Twisted doesn't take care of that for us.
if content_type_headers[0].startswith("application/jwt"):
# even if it is useless.
if content_type.startswith("application/jwt"):
alg_values = metadata.get(
"id_token_signing_alg_values_supported", ["RS256"]
)
+1 -1
View File
@@ -74,12 +74,12 @@ from synapse.http.client import (
BlocklistingAgentWrapper,
BodyExceededMaxSize,
ByteWriteable,
SimpleHttpClient,
_make_scheduler,
encode_query_args,
read_body_with_max_size,
read_multipart_response,
)
from synapse.http.native_client import SimpleHttpClient
from synapse.http.connectproxyclient import BearerProxyCredentials
from synapse.http.federation.matrix_federation_agent import MatrixFederationAgent
from synapse.http.proxyagent import ProxyAgent
+73 -40
View File
@@ -14,11 +14,8 @@
"""asyncio-native HTTP client using aiohttp.
Phase 4 of the Twisted asyncio migration. Provides NativeSimpleHttpClient
as a replacement for SimpleHttpClient, using aiohttp.ClientSession instead
of treq + Twisted Agent.
This module is unused until later phases switch callers to use it.
Provides NativeSimpleHttpClient as a replacement for SimpleHttpClient, using
aiohttp.ClientSession instead of treq + Twisted Agent.
"""
import asyncio
@@ -28,6 +25,7 @@ import urllib.parse
from http import HTTPStatus
from io import BytesIO
from typing import (
TYPE_CHECKING,
Any,
BinaryIO,
Callable,
@@ -47,6 +45,9 @@ from synapse.http.client import (
redact_uri,
)
if TYPE_CHECKING:
from synapse.server import HomeServer
logger = logging.getLogger(__name__)
# Reuse metrics from the existing client module where possible
@@ -141,57 +142,82 @@ class NativeSimpleHttpClient:
def __init__(
self,
user_agent: str,
hs_or_user_agent: "HomeServer | str",
ip_allowlist: IPSet | None = None,
ip_blocklist: IPSet | None = None,
use_proxy: bool = False,
# Explicit params (used when hs_or_user_agent is a string)
proxy_url: str | None = None,
ssl_context: ssl.SSLContext | None = None,
max_connections: int = 100,
connection_timeout: float = 15.0,
request_timeout: float = _DEFAULT_REQUEST_TIMEOUT,
# Extra kwargs accepted for BaseHttpClient compat (ignored)
**kwargs: Any,
) -> None:
"""
Args:
user_agent: User-Agent header value.
ip_allowlist: IP addresses to allow even if in blocklist.
ip_blocklist: IP addresses to disallow.
proxy_url: HTTP proxy URL (e.g., "http://proxy:8080").
ssl_context: SSL context for TLS connections.
max_connections: Max persistent connections per host.
connection_timeout: Timeout for establishing connections.
request_timeout: Default timeout for request headers.
Can be constructed with either a HomeServer or explicit params.
HomeServer mode (matches SimpleHttpClient API):
NativeSimpleHttpClient(hs, ip_allowlist=..., ip_blocklist=..., use_proxy=...)
Explicit mode:
NativeSimpleHttpClient(user_agent, ip_blocklist=..., proxy_url=..., ...)
"""
if isinstance(hs_or_user_agent, str):
# Explicit mode
user_agent = hs_or_user_agent
else:
# HomeServer mode
hs = hs_or_user_agent
ua = hs.version_string
user_agent = ua.decode("ascii") if isinstance(ua, bytes) else ua
max_connections = max(int(100 * hs.config.caches.global_factor), 5)
if use_proxy:
proxy_config = hs.config.server.proxy_config
if proxy_config:
# Prefer HTTPS proxy, fall back to HTTP proxy
proxy_url = proxy_config.https_proxy or proxy_config.http_proxy
self.user_agent = user_agent
self._ip_allowlist = ip_allowlist
self._ip_blocklist = ip_blocklist or IPSet()
self._proxy_url = proxy_url
self._request_timeout = request_timeout
self._ssl_context = ssl_context
self._max_connections = max_connections
self._connection_timeout = connection_timeout
self._session: aiohttp.ClientSession | None = None
# Build connector with optional IP blocklisting resolver
resolver = None
if ip_blocklist:
resolver = _BlocklistingResolver(ip_allowlist, ip_blocklist)
def _get_session(self) -> aiohttp.ClientSession:
"""Lazily create the aiohttp session on first use."""
if self._session is None:
resolver = None
if self._ip_blocklist:
resolver = _BlocklistingResolver(self._ip_allowlist, self._ip_blocklist)
connector = aiohttp.TCPConnector(
limit_per_host=max_connections,
resolver=resolver,
ssl=ssl_context or False,
)
connector = aiohttp.TCPConnector(
limit_per_host=self._max_connections,
resolver=resolver,
ssl=self._ssl_context or False,
)
timeout = aiohttp.ClientTimeout(
sock_connect=connection_timeout,
total=None, # We manage total timeout per-request
)
timeout = aiohttp.ClientTimeout(
sock_connect=self._connection_timeout,
total=None, # We manage total timeout per-request
)
self._session = aiohttp.ClientSession(
connector=connector,
timeout=timeout,
headers={"User-Agent": user_agent},
)
self._session = aiohttp.ClientSession(
connector=connector,
timeout=timeout,
headers={"User-Agent": self.user_agent},
)
return self._session
async def close(self) -> None:
"""Close the underlying aiohttp session."""
await self._session.close()
if self._session is not None:
await self._session.close()
async def request(
self,
@@ -230,8 +256,9 @@ class NativeSimpleHttpClient:
logger.debug("Sending request %s %s", method, redact_uri(uri))
try:
session = self._get_session()
response = await asyncio.wait_for(
self._session.request(
session.request(
method,
uri,
data=data,
@@ -470,7 +497,7 @@ class NativeSimpleHttpClient:
max_size: int | None = None,
headers: RawHeaders | None = None,
is_allowed_content_type: Callable[[str], bool] | None = None,
) -> tuple[int, dict[str, list[str]], str, int]:
) -> tuple[int, dict[bytes, list[bytes]], str, int]:
"""Download a file from a URL, streaming to output_stream.
Args:
@@ -482,6 +509,7 @@ class NativeSimpleHttpClient:
Returns:
Tuple of (length, response_headers, final_url, status_code).
Headers are bytes-keyed for compatibility with existing callers.
Raises:
SynapseError: On non-2xx response, oversized body, or timeout.
@@ -489,9 +517,10 @@ class NativeSimpleHttpClient:
h = self._build_headers(headers)
response = await self.request("GET", url, headers=h)
resp_headers: dict[str, list[str]] = {}
# Build bytes-keyed headers for backward compatibility
resp_headers: dict[bytes, list[bytes]] = {}
for key, value in response.headers.items():
resp_headers.setdefault(key, []).append(value)
resp_headers.setdefault(key.encode("ascii"), []).append(value.encode("utf-8"))
if response.status > 299:
logger.warning("Got %d when downloading %s", response.status, url)
@@ -501,8 +530,8 @@ class NativeSimpleHttpClient:
Codes.UNKNOWN,
)
if is_allowed_content_type and "Content-Type" in resp_headers:
content_type = resp_headers["Content-Type"][0]
if is_allowed_content_type and b"Content-Type" in resp_headers:
content_type = resp_headers[b"Content-Type"][0].decode("ascii")
if not is_allowed_content_type(content_type):
raise SynapseError(
HTTPStatus.BAD_GATEWAY,
@@ -563,3 +592,7 @@ class NativeSimpleHttpClient:
for v in raw_values:
h[key_str] = v.decode("ascii") if isinstance(v, bytes) else str(v)
return h
# Alias for drop-in replacement of synapse.http.client.SimpleHttpClient
SimpleHttpClient = NativeSimpleHttpClient
+1 -1
View File
@@ -41,7 +41,7 @@ except ImportError:
pass
from synapse.api.errors import Codes, SynapseError
from synapse.http.client import SimpleHttpClient
from synapse.http.native_client import SimpleHttpClient
from synapse.logging.context import make_deferred_yieldable, run_in_background
from synapse.media._base import FileInfo, get_filename_from_headers
from synapse.media.media_storage import MediaStorage, SHA256TransparentIOWriter
+1 -1
View File
@@ -68,7 +68,7 @@ from synapse.handlers.auth import (
AuthHandler,
)
from synapse.handlers.push_rules import RuleSpec, check_actions
from synapse.http.client import SimpleHttpClient
from synapse.http.native_client import SimpleHttpClient
from synapse.http.server import (
DirectServeHtmlResource,
DirectServeJsonResource,
+7 -6
View File
@@ -147,6 +147,7 @@ from synapse.http.client import (
ReplicationClient,
SimpleHttpClient,
)
from synapse.http.native_client import NativeSimpleHttpClient
from synapse.http.matrixfederationclient import MatrixFederationHttpClient
from synapse.logging.context import PreserveLoggingContext
from synapse.media.media_repository import MediaRepository
@@ -765,26 +766,26 @@ class HomeServer(metaclass=abc.ABCMeta):
return RegularPolicyForHTTPS()
@cache_in_self
def get_simple_http_client(self) -> SimpleHttpClient:
def get_simple_http_client(self) -> NativeSimpleHttpClient:
"""
An HTTP client with no special configuration.
"""
return SimpleHttpClient(self)
return NativeSimpleHttpClient(self)
@cache_in_self
def get_proxied_http_client(self) -> SimpleHttpClient:
def get_proxied_http_client(self) -> NativeSimpleHttpClient:
"""
An HTTP client that uses configured HTTP(S) proxies.
"""
return SimpleHttpClient(self, use_proxy=True)
return NativeSimpleHttpClient(self, use_proxy=True)
@cache_in_self
def get_proxied_blocklisted_http_client(self) -> SimpleHttpClient:
def get_proxied_blocklisted_http_client(self) -> NativeSimpleHttpClient:
"""
An HTTP client that uses configured HTTP(S) proxies and blocks IPs
based on the configured IP ranges.
"""
return SimpleHttpClient(
return NativeSimpleHttpClient(
self,
ip_allowlist=self.config.server.ip_range_allowlist,
ip_blocklist=self.config.server.ip_range_blocklist,
+32
View File
@@ -155,6 +155,38 @@ class FakeResponse: # type: ignore[misc]
return cls(code=code, body=body, headers=headers)
@attr.s(slots=True, frozen=True, auto_attribs=True)
class NativeFakeResponse:
"""A fake aiohttp.ClientResponse-compatible object for tests.
Provides the subset of aiohttp.ClientResponse API used by
NativeSimpleHttpClient and its callers.
"""
# HTTP status code
status: int = 200
# body of the response
body: bytes = b""
# response headers (dict-like)
headers: dict[str, str] = attr.Factory(dict)
@property
def reason(self) -> str:
code_phrase = RESPONSES.get(self.status, b"Unknown Status")
return code_phrase.decode("ascii") if isinstance(code_phrase, bytes) else str(code_phrase)
async def read(self) -> bytes:
return self.body
@classmethod
def json(cls, *, code: int = 200, payload: JsonSerializable) -> "NativeFakeResponse":
headers = {"Content-Type": "application/json"}
body = json.dumps(payload).encode("utf-8")
return cls(status=code, body=body, headers=headers)
# A small image used in some tests.
#
# Resolution: 1×1, MIME type: image/png, Extension: png, Size: 67 B
+16 -26
View File
@@ -29,17 +29,11 @@ from urllib.parse import parse_qs
import attr
try:
from twisted.web.http_headers import Headers
from twisted.web.iweb import IResponse
except ImportError:
pass
from synapse.server import HomeServer
from synapse.util.clock import Clock
from synapse.util.stringutils import random_string
from tests.test_utils import FakeResponse
from tests.test_utils import NativeFakeResponse as FakeResponse
@attr.s(slots=True, frozen=True, auto_attribs=True)
@@ -281,7 +275,7 @@ class FakeOidcServer:
token: If True, makes the token endpoint return a 500 error.
userinfo: If True, makes the userinfo endpoint return a 500 error.
"""
buggy = FakeResponse(code=500, body=b"Internal server error")
buggy = FakeResponse(status=500, body=b"Internal server error")
patches = {}
if jwks:
@@ -300,26 +294,22 @@ class FakeOidcServer:
method: str,
uri: str,
data: bytes | None = None,
headers: Headers | None = None,
) -> IResponse:
headers: dict[str, str] | None = None,
) -> FakeResponse:
"""The override of the SimpleHttpClient#request() method"""
access_token: str | None = None
if headers is None:
headers = Headers()
headers = {}
# Try to find the access token in the headers if any
auth_headers = headers.getRawHeaders(b"Authorization")
if auth_headers:
parts = auth_headers[0].split(b" ")
if parts[0] == b"Bearer" and len(parts) == 2:
access_token = parts[1].decode("ascii")
auth_header = headers.get("Authorization", "")
if auth_header.startswith("Bearer "):
access_token = auth_header[len("Bearer "):]
if method == "POST":
# If the method is POST, assume it has an url-encoded body
if data is None or headers.getRawHeaders(b"Content-Type") != [
b"application/x-www-form-urlencoded"
]:
if data is None or headers.get("Content-Type") != "application/x-www-form-urlencoded":
return FakeResponse.json(code=400, payload={"error": "invalid_request"})
params = parse_qs(data.decode("utf-8"))
@@ -338,28 +328,28 @@ class FakeOidcServer:
elif uri == self.userinfo_endpoint:
return self.get_userinfo_handler(access_token=access_token)
return FakeResponse(code=404, body=b"404 not found")
return FakeResponse(status=404, body=b"404 not found")
# Request handlers
def _get_jwks_handler(self) -> IResponse:
def _get_jwks_handler(self) -> FakeResponse:
"""Handles requests to the JWKS URI."""
return FakeResponse.json(payload=self.get_jwks())
def _get_metadata_handler(self) -> IResponse:
def _get_metadata_handler(self) -> FakeResponse:
"""Handles requests to the OIDC well-known document."""
return FakeResponse.json(payload=self.get_metadata())
def _get_userinfo_handler(self, access_token: str | None) -> IResponse:
def _get_userinfo_handler(self, access_token: str | None) -> FakeResponse:
"""Handles requests to the userinfo endpoint."""
if access_token is None:
return FakeResponse(code=401)
return FakeResponse(status=401)
user_info = self.get_userinfo(access_token)
if user_info is None:
return FakeResponse(code=401)
return FakeResponse(status=401)
return FakeResponse.json(payload=user_info)
def _post_token_handler(self, params: dict[str, list[str]]) -> IResponse:
def _post_token_handler(self, params: dict[str, list[str]]) -> FakeResponse:
"""Handles requests to the token endpoint."""
code = params.get("code", [])