Matt Sergeant wrote:
Currently I think the only thing that uses the "capabilities" notes
field is the new tls plugin. My suggestion is to make this not a notes
field any more, but an integral part of the $transaction object.
The reason being that currently we have a horrible hoop jump with it
being in notes. Things like this:
my @capabilities = $self->transaction->notes('capabilities')
? @{ $self->transaction->notes('capabilities') }
: ();
And this:
my $cap = $transaction->notes('capabilities');
$cap ||= [];
push @$cap, 'STARTTLS';
$transaction->notes('capabilities', $cap);
Which would be much easier if we could just say:
$transaction->add_capability('STARTTLS');
And:
my @capabilities = $transaction->capabilities();
Any objections?
That sounds rfc-consistent. STARTTLS has to be advertised
in the beginning of the conversation, and as a matter of
fact, STARTTLS is rfc-required to be removed after a successful
socket conversion to tls, to prevent looping, and then the new
capabilities advertised within the tls context. That change could
be the only clue to the client that tls is working--like when we
in different plugins are trying to guess what some other plugin
did and have to look for clues. That implies that nothing but the
tls plugin can decide to advertise STARTTLS or to remove
STARTTLS once tls starts, or leave it in on fail, and certainly then
"capabilities" must be writeable for the tls plugin and have a
more robust nature than notes, if possible, because notes can
get lost. Just the existence of "||=[]" and ":()" is kind of scary,
although I think what that really means is protect against
undefined by initializing, a good thing to do. But it would be
nice to initialize capabilities with a better default value than
()!
-Bob
