Support email.tlsname with old Twisted versions

Old twisted versions' ESMTPSenderFactory doesn't support the `hostname`
parameter used to implement the email.tlsname configuration option (and
also the email.enable_tls option).  This backports that parameter by
extending ESMTPSenderFactory when twisted is old.

Were shadowing the attribute `ESMTPSenderFactory.protocol` with the
method `_BackportESMTPSenderFactory.protocol` here, that is on purpose.
That attribute usually contains the `ESMTPSender` class.  It is used by
`ESMTPSenderFactory.buildProtocol` as a Callable to instantiate an
`ESMTPSender`.  By providing this Callable as a method, the previously
set `_BackportESMTPSenderFactory.__hostname` can be provided to the
newly instantiated `_BackportESMTPSender`.

Note the `# type: ignore` annotation required to pass the mypy type
checker.  Mypy assigns the type `Type[ESMTPSender]` to
`ESMTPSenderFactory.protocol`, which is valid (twisted does assign a
`Type[ESMTPSender]`), but AFAICT it is only ever used to instantiate
values of that type, i.e. as a `Callable[[...], ESMTPSender]`, so
overriding it with a method with that signature should be fine.  It is a
public attribute though, so technically it could be accessed somewhere
else, actually requiring a `Type[ESMTPSender]`.  But we do need to get
the `hostname` parameter to our `_BackportESMTPSender` somehow, and this
seems neater compared to the two alternatives I considered:
* Override `ESMTPSenderFactory.buildProtocol` instead of
  `ESMTPSenderFactory.protocol`.  That would require copying the entire
  body of `ESMTPSenderFactory.buildProtocol` into
  `_BackportESMTPSenderFactory.buildProtocol`, most of which isn't
  related to the change we're trying to implement.  That just seems
  unnecessarily brittle (e.g. maybe different old twisted versions could
  have different versions `buildProtocol` (I didn't check), picking one
  of them could lead to an inconsistent object).
* Set `self.protocol` in `_BackportESMTPSenderFactory.__init__` to a
  class defined within `_BackportESMTPSenderFactory.__init__`, such that
  it can close over the `hostname` parameter---i.e. each instantiation
  of the `_BackportESMTPSenderFactory` would define a new class
  extending `ESMTPSender` specific to that instantiation's `hostname`
  parameter.  That's a nice (functional-style) solution, but I think the
  solution implemented here might feel more natural to the average
  python programmer.

This effectively retrofits modern twisted's [1] `hostname` parameter to
old twisted's [2] classes.

[1]: https://github.com/twisted/twisted/blob/twisted-24.11.0/src/twisted/mail/smtp.py#L1992
[2]: https://github.com/twisted/twisted/blob/twisted-21.2.0/src/twisted/mail/smtp.py#L2005

synapse/handlers/send_email.py

@@ -47,15 +47,45 @@
 _is_old_twisted = parse_version(twisted.__version__) < parse_version("21")
 
 
-class _NoTLSESMTPSender(ESMTPSender):
-    """Extend ESMTPSender to disable TLS
+class _BackportESMTPSender(ESMTPSender):
+    """Extend old versions of ESMTPSender to configure TLS.
 
-    Unfortunately, before Twisted 21.2, ESMTPSender doesn't give an easy way to disable
-    TLS, so we override its internal method which it uses to generate a context factory.
+    Unfortunately, before Twisted 21.2, ESMTPSender doesn't give an easy way to
+    disable TLS, or to configure the hostname used for TLS certificate validation.
+    This backports the `hostname` parameter for that functionality.
     """
 
+    __hostname: Optional[str]
+
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        """"""
+        self.__hostname = kwargs.pop("hostname", None)
+        super().__init__(*args, **kwargs)
+
     def _getContextFactory(self) -> Optional[IOpenSSLContextFactory]:
-        return None
+        if self.context is not None:
+            return self.context
+        elif self.__hostname is None:
+            return None  # disable TLS if hostname is None
+        return optionsForClientTLS(self.__hostname)
+
+
+class _BackportESMTPSenderFactory(ESMTPSenderFactory):
+    """An ESMTPSenderFactory for _BackportESMTPSender.
+
+    This backports the `hostname` parameter, to disable or configure TLS.
+    """
+
+    __hostname: Optional[str]
+
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        self.__hostname = kwargs.pop("hostname", None)
+        super().__init__(*args, **kwargs)
+
+    def protocol(self, *args: Any, **kwargs: Any) -> ESMTPSender:  # type: ignore
+        # this overrides ESMTPSenderFactory's `protocol` attribute, with a Callable
+        # instantiating our _BackportESMTPSender, providing the hostname parameter
+        return _BackportESMTPSender(*args, **kwargs, hostname=self.__hostname)
 
 
 async def _sendmail(
@@ -94,35 +124,25 @@ async def _sendmail(
     """
     msg = BytesIO(msg_bytes)
     d: "Deferred[object]" = Deferred()
-    if tlsname is None:
+    if not enable_tls:
+        tlsname = None
+    elif tlsname is None:
         tlsname = smtphost
 
-    def build_sender_factory(**kwargs: Any) -> ESMTPSenderFactory:
-        return ESMTPSenderFactory(
-            username,
-            password,
-            from_addr,
-            to_addr,
-            msg,
-            d,
-            heloFallback=True,
-            requireAuthentication=require_auth,
-            requireTransportSecurity=require_tls,
-            **kwargs,
-        )
-
-    factory: IProtocolFactory
-    if _is_old_twisted:
-        # before twisted 21.2, we have to override the ESMTPSender protocol to disable
-        # TLS
-        factory = build_sender_factory()
-
-        if not enable_tls:
-            factory.protocol = _NoTLSESMTPSender
-    else:
-        # for twisted 21.2 and later, there is a 'hostname' parameter which we should
-        # set to enable TLS.
-        factory = build_sender_factory(hostname=tlsname if enable_tls else None)
+    factory: IProtocolFactory = (
+        _BackportESMTPSenderFactory if _is_old_twisted else ESMTPSenderFactory
+    )(
+        username,
+        password,
+        from_addr,
+        to_addr,
+        msg,
+        d,
+        heloFallback=True,
+        requireAuthentication=require_auth,
+        requireTransportSecurity=require_tls,
+        hostname=tlsname,
+    )
 
     if force_tls:
         factory = TLSMemoryBIOFactory(optionsForClientTLS(tlsname), True, factory)