]> git.ipfire.org Git - thirdparty/git.git/commit - connect.c
v0 protocol: fix sha1/sha256 confusion for capabilities^{}
authorJeff King <peff@peff.net>
Fri, 14 Apr 2023 21:25:11 +0000 (17:25 -0400)
committerJunio C Hamano <gitster@pobox.com>
Fri, 14 Apr 2023 22:08:12 +0000 (15:08 -0700)
commit13e67aa39b26b5b391f24f9b4dfb8d5b4647b8cd
tree8572327fbfe7bd3c5806998f6ff8523fa0719db7
parente6c4309748a7663f7ab7e7ec3b93ca4eb4379df4
v0 protocol: fix sha1/sha256 confusion for capabilities^{}

Commit eb398797cd (connect: advertized capability is not a ref,
2016-09-09) added support for an upload-pack server responding with:

  0000000000000000000000000000000000000000        capabilities^{}

followed by a NUL and the actual capabilities. We correctly parse the
oid using the packet_reader's hash_algo field, but then we compare it to
null_oid(), which will instead use our current repo's default algorithm.
If we're defaulting to sha256 locally but the other side is sha1, they
won't match and we'll fail to parse the line (and thus die()).

This can cause a test failure when the suite is run with
GIT_TEST_DEFAULT_HASH=sha256, and we even do so regularly via the
linux-sha256 CI job. But since the test requires JGit to run, it's
usually just skipped, and nobody noticed the problem.

The reason the original patch used JGit is that Git itself does not ever
produce such a line via upload-pack; the feature was added to fix a
real-world problem when interacting with JGit. That was good for
verifying that the incompatibility was fixed, but it's not a good
regression test:

  - hardly anybody runs it, because you have to have jgit installed;
    hence this bug going unnoticed

  - we're depending on jgit's behavior for the test to do anything
    useful. In particular, this behavior is only relevant to the v0
    protocol, but these days we ask for the v2 protocol by default. So
    for modern jgit, this is probably testing nothing.

  - it's complicated and slow. We had to do some fifo trickery to handle
    races, and this one test makes up 40% of the runtime of the total
    script.

Instead, let's just hard-code the response that's of interest to us.
That will test exactly what we want for every run, and reveals the bug
when run in sha256 mode. And of course we'll fix the actual bug by using
the correct hash_algo struct.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
connect.c
t/t5512-ls-remote.sh