Request to merge the newssl branch

[Andreas Hartmetz]

Am Freitag 14 Dezember 2007 16:49:13 schrieb David Faure:
> On Thursday 13 December 2007, Andreas Hartmetz wrote:
> > > Maybe there are volunteers to give the branch some more testing? Right
> > > now however the merge of this monday's changes into the branch is still
> > > pending so kdelibs from the branch may or may not work with the rest of
> > > your KDE4 install. There is a known bug in the SMTP(IIRC) ioslave that
> > > is a simple consequence of changing the return type of
> > > TCPSlaveBase::startTLS() to an enum with values different from the
> > > previous int. That will be easy to fix one way or another.
> > >
> > > Comments please!
> >
> > The number of comments gives me some hint about why nobody else took care
> > of SSL. I should mention that I'm doing this on paid time by KDAB; it's
> > not like I like to read books about SSL in my free time :P
> > However, I am very interested in getting a KDE 4.0(.0) out of the door in
> > a condition that at least the lunatic fringe
> > http://www.sunsite.ualberta.ca/jargon/html/L/lunatic-fringe.html
> > can enjoy.
>
> I'm missing one important bit of information: is this patch source+binary
> compatible, assuming it affects any public API?
> If it's BIC (and if it has to be BIC), then there is no choice, it -has- to
> go in now...
>
> If we don't merge it now and the kdelibs-4.0 api forces us to keep broken
> ssl stuff (at the API level) forever, we'll regret it.
AFAICS no part of KSSL is being exported or otherwise exposed now except 
indirectly by TCPSlaveBase. TCPSlaveBase however has seen little maintenance 
and there are inappropriate and (in the future/branch) unused SSL metadata as 
well as an inappropriate method or three. stopTLS() for example is a method 
that is not implementable with QSslSocket and it is unused, too. In practice 
you always just disconnect. TCPSlaveBase::connectResult() could easily be 
replaced by something that just returns the state() of the internal 
KTcpSocket, which would be more useful and less error prone.
TCPSlaveBase::setDefaultPort() is just weird as you can also give a port *and* 
a protocol which has a default port in connectToHost().

Inappropriate metadata are for example "ssl_militant" which I have renamed 
to "ssl_no_ui" and "ssl_peer_cert" which is useless because there is 
already "ssl_cert_chain" which includes this certificate. Nothing really big 
and important, just things that will look a little strange.

So, while a SC/BC break would bring good things it is not very important.
The whole partially encrypted sites thing which I mentioned before also needs 
metadata to support its operation but usage of that metadata is limited to 
code that lives in kdelibs and kdebase (konqueror).

In fact, given the possibility to make SIC changes I'd remove all unused 
methods and also remove the questionable ones. They can always be re-added 
and chances are that some will not come back while others can be made a lot 
more logical and obvious.
Replacing KSSL is a nontrivial operation anyway and a clean start where 
appropriate will need little effort now but make working with and improving 
the code in the future a lot easier.
There is not much to fear when removing the strange methods; at the moment 
they basically return "something" and everything still works as of the 
current state of testing. It's stuff that was added without a real need 
apparently and the client code I have seen just does the simplest thing that 
works which does not involve calling poorly documented old methods.

In a nutshell, *I* would really like to get this thing clean but alas, I have 
no very hard arguments for it. It's not good API design that makes programs 
run but instructions on CPUs and there are workarounds for almost everything.