git-fetch in 1.5.4 fails versus 1.5.3.8

git-fetch in 1.5.4 fails versus 1.5.3.8

[Anand Kumria]

Hi,

Having recently upgraded to git-core 1.5.4, I seen to have stumbled onto 
a regression.

$ git --version
git version 1.5.4

$ cat .git/config
{{ ... }}
[remote "richard"]
    url = https://server.example.com/~richard/newfoo.git
    fetch = +refs/heads/*:refs/remotes/richard/*

$ git fetch richard
error:  (curl_result = 77, http_code = 0, sha1 = 
0bc27e5162d0e74053b71fc637cbbf8fc942e969)
Getting pack list for https://server.example.com/~richard/newfoo.git
error:
Getting alternates list for https://server.example.com/~richard/newfoo.git
error: Unable to find 0bc27e5162d0e74053b71fc637cbbf8fc942e969 under 
https://server.example.com/~richard/newfoo.git
Cannot obtain needed object 0bc27e5162d0e74053b71fc637cbbf8fc942e969
fatal: Fetch failed.

But:


$ git clone https://server.example.com/~richard/newfoo.git
Initialized empty Git repository in /home/anand/projects/newfoo/.git/
Getting alternates list for https://server.example.com/~richard/newfoo.git
Getting pack list for https://server.example.com/~richard/newfoo.git
Getting index for pack 4b4ae2516826a864230a1e2e83e3cf900e7dbbb3
Getting index for pack 28255a0fb8b9369afe9b46b79164de160c0c532d
Getting index for pack a43ad1dff20585c7f903f498c65260a26cf57a3c
Getting index for pack 78e01c912dc4987099c40a68cbb741cb69365522
Getting index for pack ee32d85b391fda784dd5afccb22434746f112acc
Getting pack a43ad1dff20585c7f903f498c65260a26cf57a3c
 which contains 12daf1b07314589a93c2e3dbe7cb0a2f3074f4af
walk 12daf1b07314589a93c2e3dbe7cb0a2f3074f4af
walk 15542942acf5021eb911ee80a8c89f7c2bdb471e
walk cf3558da086dccc24c76c371917df73c0cfd1b6f
Getting pack 4b4ae2516826a864230a1e2e83e3cf900e7dbbb3
 which contains c761504e5e08c9111a819a4a707d86f860a24afa
Getting pack 28255a0fb8b9369afe9b46b79164de160c0c532d
 which contains df0eb371252791a066a3ebdd7feeb445245fcb80
walk 1496f6f7ffc39f44a1dc26584baf68c4b62ebfb5
[snip]
walk bce69a02c5ea897a4a1302cb603a74d9f19afa9f
walk 7bec5801836ee2b2486093da74deee0b39e693c3
got 0bc27e5162d0e74053b71fc637cbbf8fc942e969
walk 0bc27e5162d0e74053b71fc637cbbf8fc942e969
[snip]
Getting alternates list for https://server.example.com/~richard/newfoo.git
got e7ddd78769bf781707c2fed5e6b9c3c8d827b89b
walk e326801f90a554bc0af089c8a7afffa45662fd7d
Getting pack list for https://server.example.com/~richard/newfoo.git
Getting pack 78e01c912dc4987099c40a68cbb741cb69365522
 which contains 0da3b81806aa51d854f06c4a61fe45dafbdc66d3
[snip
got ac162a07d389e110aa6725e4ef2a3eedc42f05bd
got 227139e3eef336917f8a50aba06cd5e172608899


Downgrading to git-core in Debian (1.5.3.8) and it works perfectly.

$ git fetch richard
Fetching refs/heads/master from https://server.example.com/~richard/
newfoo.git using https
Fetching refs/heads/master-richard from https://server.example.com/
~richard/newfoo.git using https
got 0bc27e5162d0e74053b71fc637cbbf8fc942e969
walk 0bc27e5162d0e74053b71fc637cbbf8fc942e969
got bb7bfc531acee412ea945928073fadef5eba0fb4
got 1223925015090bad2b4b4e3cc0524a23a9bd644c
got 785e2d3d4f9834cf9c5c81d89d590c82d82c032c
* refs/remotes/richard/master-richard: fast forward to branch 'master-
richard' of https://server.example.com/~richard/newfoo
  old..new: e326801..0bc27e5
Fetching refs/heads/resellers from https://server.example.com/~richard/
newfoo.git using https

Suggestions?

Anand

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Mon, Feb 04, 2008 at 06:25:25PM +0000, Anand Kumria wrote:

> $ cat .git/config
> {{ ... }}
> [remote "richard"]
>     url = https://server.example.com/~richard/newfoo.git
>     fetch = +refs/heads/*:refs/remotes/richard/*
> 
> $ git fetch richard
> error:  (curl_result = 77, http_code = 0, sha1 = 
> 0bc27e5162d0e74053b71fc637cbbf8fc942e969)

I was unable to reproduce your problem here. However, peeking in curl's
header files, it looks like error 77 is:

    CURLE_SSL_CACERT_BADFILE,  /* 77 - could not load CACERT file, missing
                                  or wrong format */

> Downgrading to git-core in Debian (1.5.3.8) and it works perfectly.

Since you are running Debian, can you confirm whether you have the
'ca-certificates' package installed? It creates the
/etc/ssl/certs/ca-certificates.crt file, which is presumably the source
of the complaining.

That being said, there seems to be some difference between 1.5.3.8 and
1.5.4 that made us care more about SSL certs (though I note that the
SSL_VERIFYPEER curl knob has been set since pre-1.0). Have you tried
setting GIT_SSL_NO_VERIFY?

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jari Aalto]

* Tue 2008-02-05 Jeff King <peff@peff.net> gmane.comp.version-control.git
* Message-Id: 20080205050741.GA4624@coredump.intra.peff.net
> On Mon, Feb 04, 2008 at 06:25:25PM +0000, Anand Kumria wrote:
> That being said, there seems to be some difference between 1.5.3.8 and
> 1.5.4 that made us care more about SSL certs (though I note that the
> SSL_VERIFYPEER curl knob has been set since pre-1.0). Have you tried
> setting GIT_SSL_NO_VERIFY?

Confirmed. The "git push" returned failure, but when compiled with this
options, it works ok.

    $ uname -a
    SunOS 5.9 Generic_118558-35 sun4u sparc SUNW,Serverblade1

Jari

-- 
Welcome to FOSS revolution: we fix and modify until it shines

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Anand Kumria]

On Tue, 05 Feb 2008 00:07:41 -0500, Jeff King wrote:

> On Mon, Feb 04, 2008 at 06:25:25PM +0000, Anand Kumria wrote:
> 
>> $ cat .git/config
>> {{ ... }}
>> [remote "richard"]
>>     url = https://server.example.com/~richard/newfoo.git fetch =
>>     +refs/heads/*:refs/remotes/richard/*
>> 
>> $ git fetch richard
>> error:  (curl_result = 77, http_code = 0, sha1 =
>> 0bc27e5162d0e74053b71fc637cbbf8fc942e969)
> 
> I was unable to reproduce your problem here. However, peeking in curl's

I bisected this down to the patch that makes git-fetch a builtin 
(b888d61c8308027433df9c243fa551f42db1c76a) -- which is where I guessed 
the functionality changed.

> header files, it looks like error 77 is:
> 
>     CURLE_SSL_CACERT_BADFILE,  /* 77 - could not load CACERT file,
>     missing
>                                   or wrong format */
> 
>> Downgrading to git-core in Debian (1.5.3.8) and it works perfectly.
> 
> Since you are running Debian, can you confirm whether you have the
> 'ca-certificates' package installed? 

Yes, I have it installed.

> It creates the
> /etc/ssl/certs/ca-certificates.crt file, which is presumably the source
> of the complaining.

Yes, I have this file.


> That being said, there seems to be some difference between 1.5.3.8 and
> 1.5.4 that made us care more about SSL certs (though I note that the
> SSL_VERIFYPEER curl knob has been set since pre-1.0). Have you tried
> setting GIT_SSL_NO_VERIFY?
> 
> -Peff

With git 1.5.4 I can merrily clone the repository via https without issue.

Only using git-fetch seems to have an issue.

With GIT_SSL_NO_VERIFY defined, it fails with:

$ GIT_SSL_NO_VERIFY=1 ../git/git-fetch richard
error: gnutls_handshake() failed: ASN1 parser: Element was not found. (curl_result = 35, http_code = 0, sha1 = 510567ca41e201253445528ca6eb89ed43e71fce)
Getting pack list for https://server.example.com/~richard/newfoo.git
error: gnutls_handshake() failed: ASN1 parser: Element was not found.
Getting alternates list for https://server.example.com/~richard/newfoo.git
error: Unable to find 510567ca41e201253445528ca6eb89ed43e71fce under https://server.example.com/~richard/newfoo.git
Cannot obtain needed object 510567ca41e201253445528ca6eb89ed43e71fce
fatal: Fetch failed.


I *think* what is happening is that it is it is trying for the object - not seeing it and then not attempting to get the pack file.

But I'm having a hard time debugging this as git-fetch launches git-rev-list internally and it seems to be failing in there, really.

Thanks,
Anand

Bisect log:

git-bisect start
# good: [aadd4efa715f56e0eac5ac459c8ff4933b56d4ce] GIT 1.5.3.8
git-bisect good aadd4efa715f56e0eac5ac459c8ff4933b56d4ce
# bad: [c3c135291a62a01f7fd385f46cde34091767259b] GIT 1.5.4
git-bisect bad c3c135291a62a01f7fd385f46cde34091767259b
# bad: [183f84365de7b4b1fe0e15cebce80a95023aa1d6] git-p4: Fix typo in --detect-labels
git-bisect bad 183f84365de7b4b1fe0e15cebce80a95023aa1d6
# bad: [6ca8b977e4f678050db8fcb0eec2091dd44a2bd0] Bisect: add "skip" to the short usage string.
git-bisect bad 6ca8b977e4f678050db8fcb0eec2091dd44a2bd0
# good: [e66273a6abb8e9cd0967d52113e29c8014a255f8] Merge branch 'lh/merge'
git-bisect good e66273a6abb8e9cd0967d52113e29c8014a255f8
# good: [f5bf6feb05b8c89c448ded6e6fad0eb58ef35463] Merge branch 'maint'
git-bisect good f5bf6feb05b8c89c448ded6e6fad0eb58ef35463
# bad: [2b5a06edca8f7237aad6464b349b79772024d2a2] Restore default verbosity for http fetches.
git-bisect bad 2b5a06edca8f7237aad6464b349b79772024d2a2
# bad: [3278cd0a39c30c6c3082fc5feed0f9bd98b5f628] Properly cleanup in http_cleanup so builtin-fetch does not segfault
git-bisect bad 3278cd0a39c30c6c3082fc5feed0f9bd98b5f628
# good: [c7a8a16239c6bdbb4041dd8a8773ae055d3cccf8] Add bundle transport
git-bisect good c7a8a16239c6bdbb4041dd8a8773ae055d3cccf8
# bad: [7a2bff45937a60d846abf3ccb42015539aedcb40] Replace custom memory growth allocator with ALLOC_GROW
git-bisect bad 7a2bff45937a60d846abf3ccb42015539aedcb40
# bad: [4ad1eada9774a1f340beb4fdf78f1735534741bb] Fix off by one bug in reflog messages written by builtin-fetch
git-bisect bad 4ad1eada9774a1f340beb4fdf78f1735534741bb
# bad: [1aad91f5a715af92892aea7764beb829938ab111] Correct builtin-fetch to handle + in refspecs
git-bisect bad 1aad91f5a715af92892aea7764beb829938ab111
# bad: [b888d61c8308027433df9c243fa551f42db1c76a] Make fetch a builtin
git-bisect bad b888d61c8308027433df9c243fa551f42db1c76a

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Wed, Feb 06, 2008 at 09:56:35PM +0000, Anand Kumria wrote:

> With GIT_SSL_NO_VERIFY defined, it fails with:
> 
> $ GIT_SSL_NO_VERIFY=1 ../git/git-fetch richard
> error: gnutls_handshake() failed: ASN1 parser: Element was not found. (curl_result = 35, http_code = 0, sha1 = 510567ca41e201253445528ca6eb89ed43e71fce)
> Getting pack list for https://server.example.com/~richard/newfoo.git
> error: gnutls_handshake() failed: ASN1 parser: Element was not found.
> Getting alternates list for https://server.example.com/~richard/newfoo.git
> error: Unable to find 510567ca41e201253445528ca6eb89ed43e71fce under https://server.example.com/~richard/newfoo.git
> Cannot obtain needed object 510567ca41e201253445528ca6eb89ed43e71fce
> fatal: Fetch failed.

OK, I was finally able to reproduce your bug. It seems that it _only_
happens when using curl built against gnutls. I built against the
libcurl4-openssl-dev in Debian unstable, and the problem goes away.

Can you confirm that building using the openssl version of curl fixes
the problem?

Googling for your error message turns up only one other instance: a bug
in pidgin where the result was "this seems like a bug in gnutls." I hate
to say "it's not our bug" without knowing exactly what is causing it,
though. And it does seem odd that it works with 1.5.3.8. I wonder if
there is some difference in the way we are calling curl that matters.

> I *think* what is happening is that it is it is trying for the object
> - not seeing it and then not attempting to get the pack file.

No, it only fails to see objects because of the curl failure (it tries
the loose and then the pack).

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Mike Hommey]

On Wed, Feb 06, 2008 at 11:23:33PM -0500, Jeff King wrote:
> On Wed, Feb 06, 2008 at 09:56:35PM +0000, Anand Kumria wrote:
> 
> > With GIT_SSL_NO_VERIFY defined, it fails with:
> > 
> > $ GIT_SSL_NO_VERIFY=1 ../git/git-fetch richard
> > error: gnutls_handshake() failed: ASN1 parser: Element was not found. (curl_result = 35, http_code = 0, sha1 = 510567ca41e201253445528ca6eb89ed43e71fce)
> > Getting pack list for https://server.example.com/~richard/newfoo.git
> > error: gnutls_handshake() failed: ASN1 parser: Element was not found.
> > Getting alternates list for https://server.example.com/~richard/newfoo.git
> > error: Unable to find 510567ca41e201253445528ca6eb89ed43e71fce under https://server.example.com/~richard/newfoo.git
> > Cannot obtain needed object 510567ca41e201253445528ca6eb89ed43e71fce
> > fatal: Fetch failed.
> 
> OK, I was finally able to reproduce your bug. It seems that it _only_
> happens when using curl built against gnutls. I built against the
> libcurl4-openssl-dev in Debian unstable, and the problem goes away.
> 
> Can you confirm that building using the openssl version of curl fixes
> the problem?
> 
> Googling for your error message turns up only one other instance: a bug
> in pidgin where the result was "this seems like a bug in gnutls." I hate
> to say "it's not our bug" without knowing exactly what is causing it,
> though. And it does seem odd that it works with 1.5.3.8. I wonder if
> there is some difference in the way we are calling curl that matters.

Nothing significant.

Mike

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Anand Kumria]

On Wed, 06 Feb 2008 23:23:33 -0500, Jeff King wrote:

> On Wed, Feb 06, 2008 at 09:56:35PM +0000, Anand Kumria wrote:
> 
>> With GIT_SSL_NO_VERIFY defined, it fails with:
>> 
>> $ GIT_SSL_NO_VERIFY=1 ../git/git-fetch richard error:
>> gnutls_handshake() failed: ASN1 parser: Element was not found.
>> (curl_result = 35, http_code = 0, sha1 =
>> 510567ca41e201253445528ca6eb89ed43e71fce) Getting pack list for
>> https://server.example.com/~richard/newfoo.git error:
>> gnutls_handshake() failed: ASN1 parser: Element was not found. Getting
>> alternates list for https://server.example.com/~richard/newfoo.git
>> error: Unable to find 510567ca41e201253445528ca6eb89ed43e71fce under
>> https://server.example.com/~richard/newfoo.git Cannot obtain needed
>> object 510567ca41e201253445528ca6eb89ed43e71fce fatal: Fetch failed.
> 
> OK, I was finally able to reproduce your bug. It seems that it _only_
> happens when using curl built against gnutls. I built against the
> libcurl4-openssl-dev in Debian unstable, and the problem goes away.
> 
> Can you confirm that building using the openssl version of curl fixes
> the problem?

Confirmed.

Thanks for figuring out how to reproduce it ... how did you btw?

> Googling for your error message turns up only one other instance: a bug
> in pidgin where the result was "this seems like a bug in gnutls." I hate
> to say "it's not our bug" without knowing exactly what is causing it,
> though. And it does seem odd that it works with 1.5.3.8. I wonder if
> there is some difference in the way we are calling curl that matters.

It appears that git 1.5.3.8 on Debian links to libcurl3-gnutls whereas, 
at least for me, git 1.5.4 on Debian links to libcurl4-gnutls 
(or libcurl4-openssl).

I agree with you, it is a bit problematic when the library (curl) relies
on another library (gnutls) and the bottom one is having a problem.

Gerrit - since I seem to be able to reproduce this fairly easily - would
it be useful to you to have me do anything to track this down. Or will you
switch the Debian build to openssl?

Thanks for looking into this Jeff.

Regards,
Anand

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Thu, Feb 07, 2008 at 10:15:02AM +0000, Anand Kumria wrote:

> > OK, I was finally able to reproduce your bug. It seems that it _only_
> > happens when using curl built against gnutls. I built against the
> > libcurl4-openssl-dev in Debian unstable, and the problem goes away.
> 
> Thanks for figuring out how to reproduce it ... how did you btw?

I saw the gnutls error message in your output and took a guess that it
was related. I was able to reproduce against the first https repository
that I tried (I don't think it has anything to do with the repository).

I wish we could more certainly blame it on something besides git,
though. I can't reproduce it using just 'curl', so it's possible that
there is a problem with the way git is calling libcurl.

> It appears that git 1.5.3.8 on Debian links to libcurl3-gnutls whereas, 
> at least for me, git 1.5.4 on Debian links to libcurl4-gnutls 
> (or libcurl4-openssl).
> 
> I agree with you, it is a bit problematic when the library (curl) relies
> on another library (gnutls) and the bottom one is having a problem.

It would be nice if we could generate a minimal test case that
demonstrates the problem, but I can't seem to reproduce it with a
smaller program. If we could, then we could probably get advice from
curl and/or gnutls people.

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Mike Hommey]

On Thu, Feb 07, 2008 at 06:06:02AM -0500, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 07, 2008 at 10:15:02AM +0000, Anand Kumria wrote:
> 
> > > OK, I was finally able to reproduce your bug. It seems that it _only_
> > > happens when using curl built against gnutls. I built against the
> > > libcurl4-openssl-dev in Debian unstable, and the problem goes away.
> > 
> > Thanks for figuring out how to reproduce it ... how did you btw?
> 
> I saw the gnutls error message in your output and took a guess that it
> was related. I was able to reproduce against the first https repository
> that I tried (I don't think it has anything to do with the repository).
> 
> I wish we could more certainly blame it on something besides git,
> though. I can't reproduce it using just 'curl', so it's possible that
> there is a problem with the way git is calling libcurl.
> 
> > It appears that git 1.5.3.8 on Debian links to libcurl3-gnutls whereas, 
> > at least for me, git 1.5.4 on Debian links to libcurl4-gnutls 
> > (or libcurl4-openssl).
> > 
> > I agree with you, it is a bit problematic when the library (curl) relies
> > on another library (gnutls) and the bottom one is having a problem.
> 
> It would be nice if we could generate a minimal test case that
> demonstrates the problem, but I can't seem to reproduce it with a
> smaller program. If we could, then we could probably get advice from
> curl and/or gnutls people.

Did you try to run with the GIT_SSL_NO_VERIFY environment variable set ?

Mike

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Thu, Feb 07, 2008 at 01:10:43PM +0100, Mike Hommey wrote:

> > It would be nice if we could generate a minimal test case that
> > demonstrates the problem, but I can't seem to reproduce it with a
> > smaller program. If we could, then we could probably get advice from
> > curl and/or gnutls people.
> 
> Did you try to run with the GIT_SSL_NO_VERIFY environment variable set ?

Yes (I even suggested this earlier in the thread). It returns a
different error from gnutls (see Anand's earlier response).

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Frank Lichtenheld]

On Thu, Feb 07, 2008 at 10:15:02AM +0000, Anand Kumria wrote:
> Gerrit - since I seem to be able to reproduce this fairly easily - would
> it be useful to you to have me do anything to track this down. Or will you
> switch the Debian build to openssl?

Since git has no OpenSSL link exception the resulting binary wouldn't be
distributable AFAIK.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Linus Torvalds]

On Thu, 7 Feb 2008, Frank Lichtenheld wrote:
> 
> Since git has no OpenSSL link exception the resulting binary wouldn't be
> distributable AFAIK.

For crazy people who think that regular libraries can change the copyright 
status of a program (not so), you can always decide to build without 
OpenSSL and use the included Mozilla-based SHA1 implementation for git.

Performance will probably suffer, and maybe something else breaks too (I 
doubt many people test the build that way very often), but I assume Debian 
people don't care.

After all, if you're a Debian person, it's likely more important to you to 
be difficult and anal and argue about theoretical license details than 
actually be *usable*.

		Linus

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Frank Lichtenheld]

On Thu, Feb 07, 2008 at 10:42:15AM -0800, Linus Torvalds wrote:
> On Thu, 7 Feb 2008, Frank Lichtenheld wrote:
> > 
> > Since git has no OpenSSL link exception the resulting binary wouldn't be
> > distributable AFAIK.
> 
> For crazy people who think that regular libraries can change the copyright 
> status of a program (not so), you can always decide to build without 
> OpenSSL and use the included Mozilla-based SHA1 implementation for git.
> 
> Performance will probably suffer, and maybe something else breaks too (I 
> doubt many people test the build that way very often), but I assume Debian 
> people don't care.
> 
> After all, if you're a Debian person, it's likely more important to you to 
> be difficult and anal and argue about theoretical license details than 
> actually be *usable*.

Easy to say for someone who only distributes source code... (AFAIK
anyway)

Anyway, since Debian will not change its opinion about this, my answer
was in the context of the question obviously useful. Whether it was
generally correct is probably off-topic here.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Thu, Feb 07, 2008 at 10:42:15AM -0800, Linus Torvalds wrote:

> For crazy people who think that regular libraries can change the copyright 
> status of a program (not so), you can always decide to build without 
> OpenSSL and use the included Mozilla-based SHA1 implementation for git.

FWIW, this is not about OpenSSL for SHA1; it is about the underlying
library used by curl to do SSL (gnutls vs openssl). And the problem is
that curl linked against gnutls seems _broken_, so Anand has asked if
Debian can ship a binary git linked against a curl that is linked
against openssl (and the answer is probably "no, Debian people think
that is wrong").

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Linus Torvalds]

On Thu, 7 Feb 2008, Frank Lichtenheld wrote:
> 
> Easy to say for someone who only distributes source code... (AFAIK
> anyway)

Sure. But I'd like to point out that there are tons of distributiors of 
Linux _and_ other operating systems - with real lawyers involved - that 
distribute things compiled with OpenSSL, and nobody sane actually thinks 
it is a problem. The fact that the OpenSSL license isn't compatible with 
GPL is a total non-issue: people compile GPL'd programs against totally 
proprietary libraries or other non-GPL-compatible things.

> Anyway, since Debian will not change its opinion about this, my answer
> was in the context of the question obviously useful. Whether it was
> generally correct is probably off-topic here.

Umm. You claimed that the result would not be "distributable". I just 
both corrected that total misunderstanding (on part of the Debian crowd) 
_and_ said that even crazy Debian people can work around it.

So please don't say that things are not "distributable" when they clearly 
are. 

			Linus

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Linus Torvalds]

On Thu, 7 Feb 2008, Jeff King wrote:
> 
> FWIW, this is not about OpenSSL for SHA1; it is about the underlying
> library used by curl to do SSL (gnutls vs openssl).

My comment was about claiming "not distributable". That was simply not 
true. It's perfectly distributable, it's just Debian that has issues with 
OpenSSL (but then they shouldn't link it against curl either, so there 
seems to be some _other_ problem there too).

> And the problem is that curl linked against gnutls seems _broken_, so 
> Anand has asked if Debian can ship a binary git linked against a curl 
> that is linked against openssl (and the answer is probably "no, Debian 
> people think that is wrong").

Sure. And you can probably fix it by using NO_OPENSSL, which uses the 
Mozilla SHA1 library. As I also pointed out.

In short - I just wanted to make sure that we do not make the insane 
Debian policies somehow official git ones.

			Linus

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Anand Kumria]

On 2/7/08, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Thu, 7 Feb 2008, Frank Lichtenheld wrote:
> >
> > Easy to say for someone who only distributes source code... (AFAIK
> > anyway)
>
> Sure. But I'd like to point out that there are tons of distributiors of
> Linux _and_ other operating systems - with real lawyers involved - that
> distribute things compiled with OpenSSL, and nobody sane actually thinks
> it is a problem.

Whoa.

I didn't mean to start a flamefest about distributable or not within
Debian. (disclaimer, I'm a Debian developer).

If there is anything I can do to debug the issue with gnutls - let me
know. I'm just stumped about how to debug the problem.

Anand

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Martin Langhoff]

On Feb 8, 2008 10:01 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> My comment was about claiming "not distributable". That was simply not
> true. It's perfectly distributable, it's just Debian that has issues with
> OpenSSL (but then they shouldn't link it against curl either, so there
> seems to be some _other_ problem there too).

I think tides are shifting and Debian may be seeing some common sense
prevail. Thanks to ubuntu perhaps, or solar flares...

> In short - I just wanted to make sure that we do not make the insane
> Debian policies somehow official git ones.

Good clarification anyway :-)


m

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Dmitry Potapov]

On Thu, Feb 07, 2008 at 01:01:09PM -0800, Linus Torvalds wrote:
> 
> On Thu, 7 Feb 2008, Jeff King wrote:
> > 
> > FWIW, this is not about OpenSSL for SHA1; it is about the underlying
> > library used by curl to do SSL (gnutls vs openssl).
> 
> My comment was about claiming "not distributable". That was simply not 
> true. It's perfectly distributable, it's just Debian that has issues with 
> OpenSSL (but then they shouldn't link it against curl either, so there 
> seems to be some _other_ problem there too).

The curl license is very permissive, so there is no problem to link it
against any GPL program. OTOH, the OpenSSL is more restrictive than GPL,
and because GPL is copyleft (i.e. it prevents adding any restriction on
any derived work) to distribute Git linked against OpenSSL is technically
illegal unless OpenSSL is the part of the standard OS libraries or Git
developers provide a special exemption that allows to link Git against
OpenSSL and to redistribute the result. For more details, see
http://www.gnome.org/~markmc/openssl-and-the-gpl.html

Dmitry

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Mike Hommey]

On Thu, Feb 07, 2008 at 07:28:42AM -0500, Jeff King wrote:
> On Thu, Feb 07, 2008 at 01:10:43PM +0100, Mike Hommey wrote:
> 
> > > It would be nice if we could generate a minimal test case that
> > > demonstrates the problem, but I can't seem to reproduce it with a
> > > smaller program. If we could, then we could probably get advice from
> > > curl and/or gnutls people.
> > 
> > Did you try to run with the GIT_SSL_NO_VERIFY environment variable set ?
> 
> Yes (I even suggested this earlier in the thread). It returns a
> different error from gnutls (see Anand's earlier response).

Sorry, I've had trouble opening my eyes and actually reading messages I
reply to... anyways, I tried to reproduce with curl-gnutls and...
couldn't... How did you manage that ? Is the server you were trying on
public ? Do you have any http.ssl* variables set in your configuration ?

Mike

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Thu, Feb 07, 2008 at 01:01:09PM -0800, Linus Torvalds wrote:

> > FWIW, this is not about OpenSSL for SHA1; it is about the underlying
> > library used by curl to do SSL (gnutls vs openssl).
> 
> My comment was about claiming "not distributable". That was simply not 
> true. It's perfectly distributable, it's just Debian that has issues with 
> OpenSSL (but then they shouldn't link it against curl either, so there 
> seems to be some _other_ problem there too).

And I happen to agree with you, but...

> > And the problem is that curl linked against gnutls seems _broken_, so 
> > Anand has asked if Debian can ship a binary git linked against a curl 
> > that is linked against openssl (and the answer is probably "no, Debian 
> > people think that is wrong").
> 
> Sure. And you can probably fix it by using NO_OPENSSL, which uses the 
> Mozilla SHA1 library. As I also pointed out.

what I was saying before is that NO_OPENSSL _doesn't_ fix the problem,
because this has nothing whatsoever to do with the mozilla sha1 library
or any decision that git can make.

Debian provides two versions of curl, one that uses openssl and one that
uses gnutls. The question of which is used depends on which Debian
package you happen to have installed. So it is not a git matter at all,
but rather a matter of Debian policy about which version of curl is used
when building the official binary packages.

> In short - I just wanted to make sure that we do not make the insane 
> Debian policies somehow official git ones.

Agreed. There is no fallout from this issue for git; it is purely a
Debian build process issue.

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Fri, Feb 08, 2008 at 12:53:09AM +0300, Dmitry Potapov wrote:

> any derived work) to distribute Git linked against OpenSSL is technically
> illegal unless OpenSSL is the part of the standard OS libraries or Git

FWIW, openssl is in the "important" section in Debian, which makes it a
part of the standard install.

But again, this doesn't matter at all for git itself, but only for
binary distributors who must follow whatever interpretation of the GPL
their distribution decrees.

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Dmitry Potapov]

On Thu, Feb 07, 2008 at 10:15:02AM +0000, Anand Kumria wrote:
> On Wed, 06 Feb 2008 23:23:33 -0500, Jeff King wrote:
> 
> > Googling for your error message turns up only one other instance: a bug
> > in pidgin where the result was "this seems like a bug in gnutls." I hate
> > to say "it's not our bug" without knowing exactly what is causing it,
> > though. And it does seem odd that it works with 1.5.3.8. I wonder if
> > there is some difference in the way we are calling curl that matters.
> 
> It appears that git 1.5.3.8 on Debian links to libcurl3-gnutls whereas, 
> at least for me, git 1.5.4 on Debian links to libcurl4-gnutls 
> (or libcurl4-openssl).

Have you tried Git 1.5.4 with libcurl3-gnutls? It seems the package from
Debian unstable is built with it. I have backported Git 1.5.4 to Etch with
libcurl3-gnutls and I have not noticed any problems with https fetch. So,
I wonder if the problem happens with libcurl4-gnutls only regardless of
your version of Git.

Dmitry

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Jeff King]

On Thu, Feb 07, 2008 at 11:02:43PM +0100, Mike Hommey wrote:

> Sorry, I've had trouble opening my eyes and actually reading messages I
> reply to... anyways, I tried to reproduce with curl-gnutls and...
> couldn't... How did you manage that ? Is the server you were trying on
> public ? Do you have any http.ssl* variables set in your configuration ?

No, my test repo is not public. I have no special ssl configuration
(though I do use GIT_SSL_NO_VERIFY=1 since I just had a test self-signed
cert). The exact recipe on my Debian system is:

1. Build broken git on client

  client$ apt-get install libcurl4-gnutls-dev
  client$ cd ~/compile/git && make install

2. On server with ssl-enabled webserver (I am using lighttpd, but
   I don't think that it matters), make a small repo.

  server$ cd /var/www
  server$ mkdir foo && cd foo && git init
  server$ echo one >file && git add file && git commit -m one
  server$ git update-server-info

3. On client, clone repo, which should work ok

  client$ export GIT_SSL_NO_VERIFY=1 ;# if necessary
  client$ git clone https://yourserver.com/foo/.git

4. make a new commit in parent repo

  server$ echo two >file && git commit -a -m two
  server$ git update-server-info

5. fetch from client (this output is with GIT_SSL_NO_VERIFY=1; you get
   slightly different output if verification is on)

  client$ git fetch
  error: gnutls_handshake() failed: ASN1 parser: Element was not found.
  (curl_result = 35, http_code = 0, sha1 =
  07ac7bd2edd32a5818d719145910119ab72c9dd4)
  Getting pack list for https://peff.net/git/foo/.git
  error: gnutls_handshake() failed: ASN1 parser: Element was not found.
  Getting alternates list for https://peff.net/git/foo/.git
  error: Unable to find 07ac7bd2edd32a5818d719145910119ab72c9dd4 under
  https://peff.net/git/foo/.git
  Cannot obtain needed object 07ac7bd2edd32a5818d719145910119ab72c9dd4
  fatal: Fetch failed.

6. On client, rebuild with libcurl4-openssl-dev

  client$ apt-get install libcurl4-openssl-dev
  client$ cd ~/compile/git && make clean install

7. On client, do the fetch, which now works

  client$ cd ~/foo && git fetch

-Peff

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Anand Kumria]

On Fri, 08 Feb 2008 02:23:37 +0300, Dmitry Potapov wrote:

> On Thu, Feb 07, 2008 at 10:15:02AM +0000, Anand Kumria wrote:
>> On Wed, 06 Feb 2008 23:23:33 -0500, Jeff King wrote:
>> 
>> > Googling for your error message turns up only one other instance: a
>> > bug in pidgin where the result was "this seems like a bug in gnutls."
>> > I hate to say "it's not our bug" without knowing exactly what is
>> > causing it, though. And it does seem odd that it works with 1.5.3.8.
>> > I wonder if there is some difference in the way we are calling curl
>> > that matters.
>> 
>> It appears that git 1.5.3.8 on Debian links to libcurl3-gnutls whereas,
>> at least for me, git 1.5.4 on Debian links to libcurl4-gnutls (or
>> libcurl4-openssl).
> 
> Have you tried Git 1.5.4 with libcurl3-gnutls? It seems the package from
> Debian unstable is built with it. I have backported Git 1.5.4 to Etch
> with libcurl3-gnutls and I have not noticed any problems with https

Yes. I've tried the Debian git 1.5.3.8 and git 1.5.4 with whatever they 
are linked to (libcurl3-gnutls as you point out).

When I decided to build & bisect to see if I could troubleshoot, I ended 
up building with libcurl4-gnutls-dev installed first. When compiled 
against libcurl4-openssl-dev things works. 

So it definately seems specific to how git uses libcurl and how it, in 
turn, uses gnutls.

Anand

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Mike Hommey]

On Thu, Feb 07, 2008 at 07:32:39PM -0500, Jeff King wrote:
> On Thu, Feb 07, 2008 at 11:02:43PM +0100, Mike Hommey wrote:
> 
> > Sorry, I've had trouble opening my eyes and actually reading messages I
> > reply to... anyways, I tried to reproduce with curl-gnutls and...
> > couldn't... How did you manage that ? Is the server you were trying on
> > public ? Do you have any http.ssl* variables set in your configuration ?
> 
> No, my test repo is not public. I have no special ssl configuration
> (though I do use GIT_SSL_NO_VERIFY=1 since I just had a test self-signed
> cert). The exact recipe on my Debian system is:
> 
(...)

Okay, I've been able to reproduce the problem. I don't know what I've
been doing wrong to have it hidden...

Anyways, the interesting thing is to look at what curl has to say in its
verbose mode:

GIT_CURL_VERBOSE=1 git fetch
* Couldn't find host localhost in the .netrc file, using defaults
* About to connect() to localhost port 8443 (#0)
*   Trying 127.0.0.1... * connected
* Connected to localhost (127.0.0.1) port 8443 (#0)
* found 102 certificates in /etc/ssl/certs/ca-certificates.crt
*        server certificate verification FAILED
*        common name: localhost (matched)
*        server certificate expiration date OK
*        server certificate activation date OK
*        certificate public key: RSA
*        certificate version: #1
*        subject: C=GB,ST=Some-State,L=Some-Locality,O=One Organization,OU=One Organization Unit,CN=localhost,EMAIL=webmaster@localhost
*        start date: Thu, 07 Feb 2008 21:27:36 GMT
*        expire date: Sat, 08 Mar 2008 21:27:36 GMT
*        issuer: C=GB,ST=Some-State,L=Some-Locality,O=One Organization,OU=One Organization Unit,CN=localhost,EMAIL=webmaster@localhost
*        compression: DEFLATE
*        cipher: AES 256 CBC
*        MAC: SHA
> GET /foo/.git//info/refs HTTP/1.1
User-Agent: git/1.5.4.7.gd8534-dirty
Host: localhost:8443
Accept: */*

< HTTP/1.1 200 OK
< Date: Fri, 08 Feb 2008 07:10:09 GMT
< Server: Apache/2.2.8 (Debian) DAV/2 mod_ssl/2.2.8 OpenSSL/0.9.8g
< Last-Modified: Fri, 08 Feb 2008 06:52:19 GMT
< ETag: "61d82e-3b-445a0080d0ec0"
< Accept-Ranges: bytes
< Content-Length: 59
< Content-Type: text/plain
< 
* Connection #0 to host localhost left intact
* Couldn't find host localhost in the .netrc file, using defaults
* About to connect() to localhost port 8443 (#0)
*   Trying 127.0.0.1... * connected
* Connected to localhost (127.0.0.1) port 8443 (#0)
* error reading ca cert file /etc/ssl/certs/ca-certificates.crt (ASN1 parser: Element was not found.)
* gnutls_handshake() failed: ASN1 parser: Element was not found.
* Expire cleared
* Closing connection #0
error: gnutls_handshake() failed: ASN1 parser: Element was not found. (curl_result = 35, http_code = 0, sha1 = e0aa43ffb1a1e7052a936b9ed5e0a1462cfc343e)
Getting pack list for https://localhost:8443/foo/.git

So, it looks like either gnutls or curl is doing something wrong and
can't parse /etc/ssl/certs/ca-certificates.crt a second time. This
looks like a bug in either curl or gnutls.

A simplified testcase would probably be to do two requests in a row, but
I don't have time right now to do this testing.

Mike

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Mike Hommey]

On Fri, Feb 08, 2008 at 08:18:35AM +0100, Mike Hommey wrote:
> On Thu, Feb 07, 2008 at 07:32:39PM -0500, Jeff King wrote:
> > On Thu, Feb 07, 2008 at 11:02:43PM +0100, Mike Hommey wrote:
> > 
> > > Sorry, I've had trouble opening my eyes and actually reading messages I
> > > reply to... anyways, I tried to reproduce with curl-gnutls and...
> > > couldn't... How did you manage that ? Is the server you were trying on
> > > public ? Do you have any http.ssl* variables set in your configuration ?
> > 
> > No, my test repo is not public. I have no special ssl configuration
> > (though I do use GIT_SSL_NO_VERIFY=1 since I just had a test self-signed
> > cert). The exact recipe on my Debian system is:
> > 
> (...)
> 
> Okay, I've been able to reproduce the problem. I don't know what I've
> been doing wrong to have it hidden...
> 
> Anyways, the interesting thing is to look at what curl has to say in its
> verbose mode:
(...)
> 
> So, it looks like either gnutls or curl is doing something wrong and
> can't parse /etc/ssl/certs/ca-certificates.crt a second time. This
> looks like a bug in either curl or gnutls.
> 
> A simplified testcase would probably be to do two requests in a row, but
> I don't have time right now to do this testing.

I'm making myself a liar, but I took some few more minutes to test
something like:
diff --git a/http.c b/http.c
index d2c11ae..001b1c4 100644
--- a/http.c
+++ b/http.c
@@ -186,7 +186,7 @@ static CURL* get_curl_handle(void)
        if (ssl_capath != NULL)
                curl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);
 #endif
-       if (ssl_cainfo != NULL)
+//     if (ssl_cainfo != NULL)
                curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
        curl_easy_setopt(result, CURLOPT_FAILONERROR, 1);
 


And the result is interesting:
GIT_CURL_VERBOSE=1 git fetch
* Couldn't find host localhost in the .netrc file, using defaults
* About to connect() to localhost port 8443 (#0)
*   Trying 127.0.0.1... * connected
* Connected to localhost (127.0.0.1) port 8443 (#0)
*        server certificate verification FAILED
*        common name: localhost (matched)
*        server certificate expiration date OK
*        server certificate activation date OK
*        certificate public key: RSA
*        certificate version: #1
*        subject: C=GB,ST=Some-State,L=Some-Locality,O=One Organization,OU=One Organization Unit,CN=localhost,EMAIL=webmaster@localhost
*        start date: Thu, 07 Feb 2008 21:27:36 GMT
*        expire date: Sat, 08 Mar 2008 21:27:36 GMT
*        issuer: C=GB,ST=Some-State,L=Some-Locality,O=One Organization,OU=One Organization Unit,CN=localhost,EMAIL=webmaster@localhost
*        compression: DEFLATE
*        cipher: AES 256 CBC
*        MAC: SHA
> GET /foo/.git//info/refs HTTP/1.1
User-Agent: git/1.5.4.7.gd8534-dirty
Host: localhost:8443
Accept: */*

< HTTP/1.1 200 OK
< Date: Fri, 08 Feb 2008 07:30:10 GMT
< Server: Apache/2.2.8 (Debian) DAV/2 mod_ssl/2.2.8 OpenSSL/0.9.8g
< Last-Modified: Fri, 08 Feb 2008 06:52:19 GMT
< ETag: "61d82e-3b-445a0080d0ec0"
< Accept-Ranges: bytes
< Content-Length: 59
< Content-Type: text/plain
< 
* Connection #0 to host localhost left intact
* Couldn't find host localhost in the .netrc file, using defaults
* About to connect() to localhost port 8443 (#0)
*   Trying 127.0.0.1... * connected
* Connected to localhost (127.0.0.1) port 8443 (#0)
* gnutls_handshake() failed: ASN1 parser: Element was not found.
* Expire cleared
* Closing connection #0
error: gnutls_handshake() failed: ASN1 parser: Element was not found. (curl_result = 35, http_code = 0, sha1 = e0aa43ffb1a1e7052a936b9ed5e0a1462cfc343e)

So, it looks like either gnutls has a problem reinitializing its ASN1
parser or curl is doing something wrong with gnutls when initializing a
new request.

Mike

Re: git-fetch in 1.5.4 fails versus 1.5.3.8

[Dmitry Potapov]

On Fri, Feb 08, 2008 at 02:43:19AM +0000, Anand Kumria wrote:
> 
> Yes. I've tried the Debian git 1.5.3.8 and git 1.5.4 with whatever they 
> are linked to (libcurl3-gnutls as you point out).
> 
> When I decided to build & bisect to see if I could troubleshoot, I ended 
> up building with libcurl4-gnutls-dev installed first. When compiled 
> against libcurl4-openssl-dev things works. 
> 
> So it definately seems specific to how git uses libcurl and how it, in 
> turn, uses gnutls.

I have investigated this issue a bit more... As I mentioned before I
used Git 1.5.4 with libcurl3-gnutls on Debian Etch and did not have
that problem, but when I installed exactly the same package on Debian
unstable it exhibits the above problem. Debian Etch has curl v7.15.5,
while Debian testing uses curl v7.17.1 and Debian unstable uses curl
7.18.0 (both libcurl3 and libcurl4 are built from the same sources).
So the version of libcurl seems to be relevant here. OTOH, git 1.5.3.8
works with both versions of libcurl-gnutls, while git 1.5.4 does not
work with a new one. So, it is also specific to how git uses libcurl.
I will look into it more during the weekend.

Dmitry

[PATCH] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

Either recent curl or gnutls doesn't like initializing again after cleaning
up, and this is happening in some cases such as git fetch.

We work around this by removing the http_cleanup call from get_refs_via_curl,
and allowing http_init to be called several times without initializing http.c
global variables again and leaking old values.

The remaining calls to http_cleanup are either last (http-push.c), or almost
never called (walker.c; the function it lies in is only called from
transport-disconnect, which is called last, and only in builtin-push.c).
These leaks shall be addressed in the http code refactoring.

Signed-off-by: Mike Hommey <mh@glandium.org>
---

 > So, it looks like either gnutls has a problem reinitializing its ASN1
 > parser or curl is doing something wrong with gnutls when initializing a
 > new request.

 In the end, it was a bit of git's fault, but either curl or gnutls is the
 actual culprit. I've not looked into either code to find out who's
 responsible, but a very simplified testcase is as follows:

	#include <curl/curl.h>
	#include <curl/easy.h>

	int main(void) {
	        CURL *easy = curl_easy_init();
	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
	        curl_easy_perform(easy);
	        curl_global_cleanup();
	        easy = curl_easy_init();
	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
	        curl_easy_perform(easy);
	}

	(build with gcc -o test test.c -lcurl)
	(note curl_easy_init does curl_global_init behind the curtains, even the
	second time. You can convince yourself by adding
	curl_global_init(CURL_GLOBAL_ALL);)

 http.c      |    5 +++++
 transport.c |    2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/http.c b/http.c
index d2c11ae..a3aa9e9 100644
--- a/http.c
+++ b/http.c
@@ -215,9 +215,14 @@ static CURL* get_curl_handle(void)
 
 void http_init(void)
 {
+	static int init = 0;
 	char *low_speed_limit;
 	char *low_speed_time;
 
+	if (init)
+		return;
+	init = 1;
+
 	curl_global_init(CURL_GLOBAL_ALL);
 
 	pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
diff --git a/transport.c b/transport.c
index babaa21..1eb6d78 100644
--- a/transport.c
+++ b/transport.c
@@ -473,8 +473,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 		return NULL;
 	}
 
-	http_cleanup();
-
 	data = buffer.buf;
 	start = NULL;
 	mid = data;
-- 
1.5.4.7.gd8534-dirty

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

>  In the end, it was a bit of git's fault, but either curl or gnutls is the
>  actual culprit. I've not looked into either code to find out who's
>  responsible, but a very simplified testcase is as follows:
> 
> 	#include <curl/curl.h>
> 	#include <curl/easy.h>
> 
> 	int main(void) {
> 	        CURL *easy = curl_easy_init();
> 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> 	        curl_easy_perform(easy);
> 	        curl_global_cleanup();
> 	        easy = curl_easy_init();
> 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> 	        curl_easy_perform(easy);
> 	}
> 
> 	(build with gcc -o test test.c -lcurl)
> 	(note curl_easy_init does curl_global_init behind the curtains, even the
> 	second time. You can convince yourself by adding
> 	curl_global_init(CURL_GLOBAL_ALL);)

And the winner is... curl !
The bug was introduced in this commit:
http://cool.haxx.se/cvs.cgi/curl/lib/gtls.c.diff?r1=1.26&r2=1.27
Note how gtls_inited is not set back to FALSE in cleanup.

This ended up released in 7.16.3. I'm filing a bug.

Mike

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

>>  In the end, it was a bit of git's fault, but either curl or gnutls is the
>>  actual culprit. I've not looked into either code to find out who's
>>  responsible, but a very simplified testcase is as follows:
>> ...
>
> And the winner is... curl !
> The bug was introduced in this commit:
> http://cool.haxx.se/cvs.cgi/curl/lib/gtls.c.diff?r1=1.26&r2=1.27
> Note how gtls_inited is not set back to FALSE in cleanup.
>
> This ended up released in 7.16.3. I'm filing a bug.

Good detetive work.  Thanks.

I guess we need to ship with a known leak to work this around.
Sigh...

Perhaps we can convince cURL developers to switch to git while
we are at it? ;-)

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

On Fri, Feb 08, 2008 at 01:46:14PM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> >>  In the end, it was a bit of git's fault, but either curl or gnutls is the
> >>  actual culprit. I've not looked into either code to find out who's
> >>  responsible, but a very simplified testcase is as follows:
> >> ...
> >
> > And the winner is... curl !
> > The bug was introduced in this commit:
> > http://cool.haxx.se/cvs.cgi/curl/lib/gtls.c.diff?r1=1.26&r2=1.27
> > Note how gtls_inited is not set back to FALSE in cleanup.
> >
> > This ended up released in 7.16.3. I'm filing a bug.
> 
> Good detetive work.  Thanks.
> 
> I guess we need to ship with a known leak to work this around.
> Sigh...

We can probably add a test on curl versions to avoid leaking on every
install. Something like #if LIBCURL_VERSION_NUM < 0x071003. And then add
|| LIBCURL_VERSION_NUM > .... whenever this is fixed in curl...
Though, as I said, we are not calling http_cleanup in a lot of cases,
already.

Mike

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Johannes Schindelin]

Hi,

On Fri, 8 Feb 2008, Mike Hommey wrote:

> >  In the end, it was a bit of git's fault, but either curl or gnutls is 
> >  the actual culprit. I've not looked into either code to find out 
> >  who's responsible, but a very simplified testcase is as follows:
> > 
> > 	#include <curl/curl.h>
> > 	#include <curl/easy.h>
> > 
> > 	int main(void) {
> > 	        CURL *easy = curl_easy_init();
> > 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> > 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> > 	        curl_easy_perform(easy);
> > 	        curl_global_cleanup();
> > 	        easy = curl_easy_init();
> > 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> > 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> > 	        curl_easy_perform(easy);
> > 	}
> > 
> > 	(build with gcc -o test test.c -lcurl)
> > 	(note curl_easy_init does curl_global_init behind the curtains, 
> >      even the second time. You can convince yourself by adding
> > 	 curl_global_init(CURL_GLOBAL_ALL);)
> 
> And the winner is... curl !
> The bug was introduced in this commit:
> http://cool.haxx.se/cvs.cgi/curl/lib/gtls.c.diff?r1=1.26&r2=1.27
> Note how gtls_inited is not set back to FALSE in cleanup.

Wow.  I hope you used "git bisect", in order to spare you unnecessary 
work...

> This ended up released in 7.16.3. I'm filing a bug.

Thanks for being so persistent.

Ciao,
Dscho

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

On Fri, Feb 08, 2008 at 09:53:47PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 8 Feb 2008, Mike Hommey wrote:
> 
> > >  In the end, it was a bit of git's fault, but either curl or gnutls is 
> > >  the actual culprit. I've not looked into either code to find out 
> > >  who's responsible, but a very simplified testcase is as follows:
> > > 
> > > 	#include <curl/curl.h>
> > > 	#include <curl/easy.h>
> > > 
> > > 	int main(void) {
> > > 	        CURL *easy = curl_easy_init();
> > > 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> > > 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> > > 	        curl_easy_perform(easy);
> > > 	        curl_global_cleanup();
> > > 	        easy = curl_easy_init();
> > > 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> > > 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> > > 	        curl_easy_perform(easy);
> > > 	}
> > > 
> > > 	(build with gcc -o test test.c -lcurl)
> > > 	(note curl_easy_init does curl_global_init behind the curtains, 
> > >      even the second time. You can convince yourself by adding
> > > 	 curl_global_init(CURL_GLOBAL_ALL);)
> > 
> > And the winner is... curl !
> > The bug was introduced in this commit:
> > http://cool.haxx.se/cvs.cgi/curl/lib/gtls.c.diff?r1=1.26&r2=1.27
> > Note how gtls_inited is not set back to FALSE in cleanup.
> 
> Wow.  I hope you used "git bisect", in order to spare you unnecessary 
> work...

This was actually easy to spot without. I was pretty sure something
fishy was going on with the curl_global_cleanup code, so I followed it
up to Curl_gtls_cleanup and there it was just under my eyes. I only had
to use the annotate thingy in viewcvs (and leave out version 1.35 of the
file, that does whitespace changes only).

Mike

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

On Fri, Feb 08, 2008 at 10:51:40PM +0100, Mike Hommey wrote:
> On Fri, Feb 08, 2008 at 01:46:14PM -0800, Junio C Hamano wrote:
> > Mike Hommey <mh@glandium.org> writes:
> > 
> > >>  In the end, it was a bit of git's fault, but either curl or gnutls is the
> > >>  actual culprit. I've not looked into either code to find out who's
> > >>  responsible, but a very simplified testcase is as follows:
> > >> ...
> > >
> > > And the winner is... curl !
> > > The bug was introduced in this commit:
> > > http://cool.haxx.se/cvs.cgi/curl/lib/gtls.c.diff?r1=1.26&r2=1.27
> > > Note how gtls_inited is not set back to FALSE in cleanup.
> > >
> > > This ended up released in 7.16.3. I'm filing a bug.
> > 
> > Good detetive work.  Thanks.
> > 
> > I guess we need to ship with a known leak to work this around.
> > Sigh...
> 
> We can probably add a test on curl versions to avoid leaking on every
> install. Something like #if LIBCURL_VERSION_NUM < 0x071003. And then add
> || LIBCURL_VERSION_NUM > .... whenever this is fixed in curl...

... and 22 minutes after filing the bug, it's fixed in CVS
http://cool.haxx.se/cvs.cgi/curl/lib/gtls.c.diff?r1=1.36&r2=1.37

which means it will be fixed in version 7.18.1.

Mike

[PATCH v2] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

curl versions 7.16.3 to 7.18.0 included had a regression in which https
requests following curl_global_cleanup/init sequence would fail with ASN1
parser errors with curl-gnutls. Such sequences happen in some cases such
as git fetch.

We work around this by removing the http_cleanup call from get_refs_via_curl
for broken versions of curl, and allowing http_init to be called several
times without initializing http.c global variables again and leaking old
values, which is a safe thing to have unconditionally.

The remaining calls to http_cleanup are either last (http-push.c), or almost
never called (walker.c; the function it lies in is only called from
transport-disconnect, which is called last, and only in builtin-push.c)
These leaks shall be addressed in the http code refactoring.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 http.c      |    5 +++++
 transport.c |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index d2c11ae..a3aa9e9 100644
--- a/http.c
+++ b/http.c
@@ -215,9 +215,14 @@ static CURL* get_curl_handle(void)
 
 void http_init(void)
 {
+	static int init = 0;
 	char *low_speed_limit;
 	char *low_speed_time;
 
+	if (init)
+		return;
+	init = 1;
+
 	curl_global_init(CURL_GLOBAL_ALL);
 
 	pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
diff --git a/transport.c b/transport.c
index babaa21..32ab521 100644
--- a/transport.c
+++ b/transport.c
@@ -473,7 +473,9 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 		return NULL;
 	}
 
+#if (LIBCURL_VERSION_NUM < 0x071003) || (LIBCURL_VERSION_NUM > 0x071200)
 	http_cleanup();
+#endif
 
 	data = buffer.buf;
 	start = NULL;
-- 
1.5.4.7.gd8534-dirty

Re: [PATCH v2] Work around curl-gnutls not liking to be reinitialized

[Johannes Schindelin]

Hi,

On Fri, 8 Feb 2008, Mike Hommey wrote:

> diff --git a/http.c b/http.c
> index d2c11ae..a3aa9e9 100644
> --- a/http.c
> +++ b/http.c
> @@ -215,9 +215,14 @@ static CURL* get_curl_handle(void)
>  
>  void http_init(void)
>  {
> +	static int init = 0;
>  	char *low_speed_limit;
>  	char *low_speed_time;
>  
> +	if (init)
> +		return;
> +	init = 1;
> +

Don't you have to make this conditional on the CURL version as well?  I 
mean, that cleanup:

> diff --git a/transport.c b/transport.c
> index babaa21..32ab521 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -473,7 +473,9 @@ static struct ref *get_refs_via_curl(struct transport *transport)
>  		return NULL;
>  	}
>  
> +#if (LIBCURL_VERSION_NUM < 0x071003) || (LIBCURL_VERSION_NUM > 0x071200)
>  	http_cleanup();
> +#endif

requires us to init again, no?

Ciao,
Dscho

[PATCH] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

curl versions 7.16.3 to 7.18.0 included had a regression in which https
requests following curl_global_cleanup/init sequence would fail with ASN1
parser errors with curl-gnutls. Such sequences happen in some cases such
as git fetch.

We work around this by removing the http_cleanup call from get_refs_via_curl
for the broken versions of curl, and allowing http_init to be called several
times without initializing http.c global variables again and leaking old
values, which is a safe thing to have unconditionally.

The remaining calls to http_cleanup are either last (http-push.c), or almost
never called (walker.c; the function it lies in is only called from
transport-disconnect, which is called last, and only in builtin-push.c)
These leaks shall be addressed in the http code refactoring.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 > Don't you have to make this conditional on the CURL version as well?  I
 > mean, that cleanup:

 > > diff --git a/transport.c b/transport.c
 > > index babaa21..32ab521 100644
 > > --- a/transport.c
 > > +++ b/transport.c
 > > @@ -473,7 +473,9 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 > >              return NULL;
 > >      }
 > >
 > > +#if (LIBCURL_VERSION_NUM < 0x071003) || (LIBCURL_VERSION_NUM > 0x071200)
 > >       http_cleanup();
 > > +#endif
 >
 > requires us to init again, no?

 Damn, you're right. But it would actually be better to just have the init
 variable set to 0 again in http_cleanup, and actually, we already have a
 global variable that is set in http_init and reset in http_cleanup that
 could be used for this test...

 http.c      |    3 +++
 transport.c |    2 ++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index d2c11ae..d69ba90 100644
--- a/http.c
+++ b/http.c
@@ -218,6 +218,9 @@ void http_init(void)
 	char *low_speed_limit;
 	char *low_speed_time;
 
+	if (pragma_header)
+		return;
+
 	curl_global_init(CURL_GLOBAL_ALL);
 
 	pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
diff --git a/transport.c b/transport.c
index babaa21..32ab521 100644
--- a/transport.c
+++ b/transport.c
@@ -473,7 +473,9 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 		return NULL;
 	}
 
+#if (LIBCURL_VERSION_NUM < 0x071003) || (LIBCURL_VERSION_NUM > 0x071200)
 	http_cleanup();
+#endif
 
 	data = buffer.buf;
 	start = NULL;
-- 
1.5.4.8.g95ac

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

And I forgot to add a v3 to the title... *sigh*.

Sorry

Mike

Re: [PATCH] Work around curl-gnutls not liking to be reinitialized

[Jeff King]

On Fri, Feb 08, 2008 at 09:08:55PM +0100, Mike Hommey wrote:

> 	#include <curl/curl.h>
> 	#include <curl/easy.h>
> 
> 	int main(void) {
> 	        CURL *easy = curl_easy_init();
> 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> 	        curl_easy_perform(easy);
> 	        curl_global_cleanup();
> 	        easy = curl_easy_init();
> 	        curl_easy_setopt(easy, CURLOPT_VERBOSE, 1);
> 	        curl_easy_setopt(easy, CURLOPT_URL, "https://www.verisign.com/");
> 	        curl_easy_perform(easy);
> 	}

Hrmph. I had tried to produce a similar minimum test case, but for some
reason I didn't try doing a global_cleanup() between the requests, which
obviously is the culprit.

Thank you for spending the time to track this down. I have confirmed
that your fix works on my test case.

-Peff

[PATCH v4] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

curl versions 7.16.3 to 7.18.0 included had a regression in which https
requests following curl_global_cleanup/init sequence would fail with ASN1
parser errors with curl-gnutls. Such sequences happen in some cases such
as git fetch.

We work around this by removing the http_init and http_cleanup calls from
get_refs_via_curl, replacing them with a transport->data initialization
with the http_walker (which does http_init).

While the http_walker is not currently used in get_refs_via_curl, http
and walker code refactor will make it use it.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 FWIW, the previous patch lacked an initialization for pragma_header. But I
 actually got a better idea ; a more long-term one.

 transport.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/transport.c b/transport.c
index babaa21..497f853 100644
--- a/transport.c
+++ b/transport.c
@@ -441,11 +441,12 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 	struct ref *ref = NULL;
 	struct ref *last_ref = NULL;
 
+	if (!transport->data)
+		transport->data = get_http_walker(transport->url);
+
 	refs_url = xmalloc(strlen(transport->url) + 11);
 	sprintf(refs_url, "%s/info/refs", transport->url);
 
-	http_init();
-
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
@@ -473,8 +474,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)
 		return NULL;
 	}
 
-	http_cleanup();
-
 	data = buffer.buf;
 	start = NULL;
 	mid = data;
-- 
1.5.4.35.gb88c

Re: [PATCH v2] Work around curl-gnutls not liking to be reinitialized

[Florian Weimer]

* Mike Hommey:

> +#if (LIBCURL_VERSION_NUM < 0x071003) || (LIBCURL_VERSION_NUM > 0x071200)
>  	http_cleanup();
> +#endif

Shouldn't you check the version that is used at run time, not the one at
compile time?

Re: [PATCH v2] Work around curl-gnutls not liking to be reinitialized

[Mike Hommey]

On Sat, Feb 09, 2008 at 10:44:55AM +0100, Florian Weimer wrote:
> * Mike Hommey:
> 
> > +#if (LIBCURL_VERSION_NUM < 0x071003) || (LIBCURL_VERSION_NUM > 0x071200)
> >  	http_cleanup();
> > +#endif
> 
> Shouldn't you check the version that is used at run time, not the one at
> compile time?

That is a very good remark, and another good reason to prefer patch v4.

Mike

Re: [PATCH v4] Work around curl-gnutls not liking to be reinitialized

[Daniel Stenberg]

Mike Hommey <mh <at> glandium.org> writes:

> curl versions 7.16.3 to 7.18.0 included had a regression in which https
> requests following curl_global_cleanup/init sequence would fail with ASN1
> parser errors with curl-gnutls. Such sequences happen in some cases such
> as git fetch.

Hi git hackers,

I am the main libcurl author and maintainer.

While I agree that this is a regression and a bug in libcurl, it puzzles me why
you found it in the first place. curl_global_init and curl_global_cleanup should
only be needed to call once per program's lifetime. I can't think of any
practical use to do this re-init at all. You will only waste time on this.

So, your best fix for this problem is to simply to a curl_global_init() in the
beginning and a curl_global_cleanup() in the end.

(I'm not subscribed to this list, just responding via gmane.org)