[Twisted-Python] startTLS errors not propagating to Factory

[Richard van der Hoff]

On 29/04/2021 07:36, Glyph wrote:
>
>> In my tests at least, TLSMemoryBIOProtocol.connectionLost is doing 
>> exactly the right thing - it is called with an unhelpful reason, but 
>> substitutes back in the helpful reason which has already been stashed.
>>
>> Rather, the problem, as I see it, is that it's not 
>> TLSMemoryBIOProtocol.connectionLost that calls 
>> Factory.clientConnectionLost. That is done by 
>> tcp.Client.connectionLost, via one of tcp.Client's myriad of base 
>> classes, at 
>> https://github.com/twisted/twisted/blob/3c868ac11786eef7ea269caa3056f00854128957/src/twisted/internet/tcp.py#L508. 
>> Of course, that doesn't get the benefit of TLSMemoryBIOProtocol's 
>> reason switcheroo.
>>
>> I'm still not quite sure who is in the wrong here.
>
> Aah, yeah, this is a weird quirk of the ancient-style layering in the 
> SMTP code :-|.  The way this /should/ work is by using HostnameEndpoint.
>
> I'm not sure /exactly/ where we're going off the rails, but by using 
> /both /the old 'startTLS' style of starting a TLS connection, as well 
> as relying on ClientFactory rather than an Endpoint of some kind, 
> means that we're getting this duplicate notification; the one that you 
> get to Protocol.connectionLost will come from TLS and have useful 
> information, but the one that goes to the Connector will be coming 
> straight from TCP.
>
> The right thing to fix here, I think, is to ignore 
> clientConnectionLost entirely, and instead to have the protocol object 
> relay its failure to some other differently-named method on 
> SMTPSenderFactory.
>
Right! That sounds plausible, and certainly gives me some places to 
poke. I'll have another look later. Thanks very much!

R
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20210429/75f2594a/attachment.htm>