On Mon, 20 Dec 2010 19:44:47 +0000
"Adam D. Barratt" <adam@adam-barratt.org.uk> wrote:
> On Fri, 2010-12-17 at 21:29 +0000, Neil Williams wrote:
> > I've tidied up the patch which turns this silent error into a more
> > noisy warning but does not try to fix the underlying issue. The
> > patch is based on one by Johannes Ernst <johannes.ernst@gmail.com>,
> > as detailed in the attached patch. (The only other change is to put
> > the patch into the series file *in the middle* due to problems with
> > the gnutls changes needing to be last.)
>
> From the bug log, it looks like this hasn't been fixed in unstable
> yet; is that correct?
Yes.
> I appreciate that the unstable package won't
> be able to migrate, but I'd prefer to see this fixed there as well
> rather than the patch being dropped straight in to squeeze.
So two uploads? Or shall I clone this bug and mark the clone as found
in the version in sid and this one not found in sid for the maintainer
to look at the underlying problem not fixed by this patch?
> I might be missing something, but this change looks wrong:
>
> + nonblocking?0:(int)timeout_ms?1000:timeout_ms);
>
> If timeout_ms is non-zero, a hard-coded value of 1000 will be used;
> otherwise timeout_ms will be passed, which seems to be exactly what
> the change was trying to avoid?
You'd think so, so did I. After another run through the tests, it makes
absolutely no difference. Alternative code, expanded for a bit of
clarity:
int repeat = 0;
if(!nonblocking) {
repeat = ((int)timeout_ms) ? timeout_ms : 1000;
}
what = Curl_socket_ready(readfd, writefd, repeat);
With this version of the patch, apt-transport-https works just as it
does with the other version of the patch. Furthermore the change to the
client-cert described in my previous message (commenting out the force
for SSHv3) produces the same handshake error message with the patch and
the timeout message without it.
I'm still dubious about this whole bug/patch - especially that this
entire process has been only tested against a single setup, the bug
only shows up in a single frontend and the bug has not had any input
from the maintainer who has been otherwise active with uploads and
fixes.
This would appear to be a minor bug in curl [0] which either of
these patches does not fix (merely makes more noisy) and the issue
itself only affects apt-transport-https if a particular client setup is
deployed and the example config easily hides the real issue by
retaining a force setting inherited from a previous Lenny setup.
SslForceVersion "SSLv3"; // This is required to get it to work
// in lenny; not sure why.
Overall, maybe the best solution here is to downgrade this bug rather
than potentially confuse things further with a patch which itself
doesn't provide a clear fix for the original problem and may be based
on a poorly understood test case.
I'd like some feedback from others involved in this bug and from the RT
before downgrading. Alternatively, some kind of indication that there is
more than just one https server/cert/config combination affected by the
top-level apt-transport-https issue.
[0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=594150#44
--
Neil Williams
=============
http://www.linux.codehelp.co.uk/
Attachment:
pgpXwu6l21iab.pgp
Description: PGP signature