threads / discuss / 35347

Symbolic refs break ref advertisement on 1.8.4.3+

Subject: Symbolic refs break ref advertisement on 1.8.4.3+

## tl;dr

3 messages between Nov 16, 2013 and Nov 18, 2013.

replies: 2people: 3as markdown or json

Bryan Turner· Nov 16, 2013, 14:39 UTC · lore

According to a git bisect between the v1.8.4 and v1.8.4.3 tags, it appears the changes in 5e7dcad, "upload-pack: send non-HEAD symbolic refs", cause the ref advertisement to fail of the repository has more than a handful of symbolic refs. Here's a simple reproduce case (tested on Bash):

Aphrael:example bturner$ git version
git version 1.8.4.3
Aphrael:symbolic-refs bturner$ git init example
Initialized empty Git repository in /Users/bturner/example/.git/
Aphrael:symbolic-refs bturner$ cd example
Aphrael:example bturner$ echo "Testing..." > file.txt
Aphrael:example bturner$ git add file.txt
Aphrael:example bturner$ git commit -m "Initial commit"
[master (root-commit) b4c4b2a] Initial commit
 1 file changed, 1 insertion(+)
 create mode 100644 file.txt
Aphrael:example bturner$ for ((i=1;i<21;i++)); do git symbolic-ref
refs/heads/syms/$i refs/heads/master; done
Aphrael:example bturner$ git ls-remote .
fatal: protocol error: impossibly long line
fatal: Could not read from remote repository.

A symref= entry is written into the first packet of the ref advertisement, right after the capabilities, for each symbolic ref in the repository. Unfortunately, no splitting is done on that value and so once you have 15-20 symbolic refs (more or less depending on path lengths), you blow the 996 byte limit in format_packet (pkt-line.c) and all further clone/fetch operations fail.

I've verified this same issue exists in all 1.8.5 RCs.

Best regards, Bryan Turner

Jeff King· Nov 17, 2013, 10:02 UTC · re: Bryan Turner · lore

Re: Symbolic refs break ref advertisement on 1.8.4.3+

On Sun, Nov 17, 2013 at 01:39:52AM +1100, Bryan Turner wrote:

Show 12 quoted lines

> Aphrael:example bturner$ for ((i=1;i<21;i++)); do git symbolic-ref
> refs/heads/syms/$i refs/heads/master; done
> Aphrael:example bturner$ git ls-remote .
> fatal: protocol error: impossibly long line
> fatal: Could not read from remote repository.
> 
> A symref= entry is written into the first packet of the ref
> advertisement, right after the capabilities, for each symbolic ref in
> the repository. Unfortunately, no splitting is done on that value and
> so once you have 15-20 symbolic refs (more or less depending on path
> lengths), you blow the 996 byte limit in format_packet (pkt-line.c)
> and all further clone/fetch operations fail.

Ick, yeah. I don't think there is a way around that with the way the information is shoe-horned into the protocol. We should probably just revert 5e7dcad (upload-pack: send non-HEAD symbolic refs, 2013-09-17), and assume the HEAD branch name is short enough to fit.

Another option would be to cap the number of non-HEAD symrefs we'd send (by counting up the bytes and keeping below the limit). That at least makes the "easy" cases work, but it's a bit too flaky for my taste.

-Peff

Junio C Hamano· Nov 18, 2013, 18:16 UTC · re: Jeff King · lore

Re: Symbolic refs break ref advertisement on 1.8.4.3+

Jeff King <peff@peff.net> writes:

Show 23 quoted lines

> On Sun, Nov 17, 2013 at 01:39:52AM +1100, Bryan Turner wrote:
>
>> Aphrael:example bturner$ for ((i=1;i<21;i++)); do git symbolic-ref
>> refs/heads/syms/$i refs/heads/master; done
>> Aphrael:example bturner$ git ls-remote .
>> fatal: protocol error: impossibly long line
>> fatal: Could not read from remote repository.
>> 
>> A symref= entry is written into the first packet of the ref
>> advertisement, right after the capabilities, for each symbolic ref in
>> the repository. Unfortunately, no splitting is done on that value and
>> so once you have 15-20 symbolic refs (more or less depending on path
>> lengths), you blow the 996 byte limit in format_packet (pkt-line.c)
>> and all further clone/fetch operations fail.
>
> Ick, yeah. I don't think there is a way around that with the way the
> information is shoe-horned into the protocol.  We should probably just
> revert 5e7dcad (upload-pack: send non-HEAD symbolic refs, 2013-09-17),
> and assume the HEAD branch name is short enough to fit.
>
> Another option would be to cap the number of non-HEAD symrefs we'd send
> (by counting up the bytes and keeping below the limit). That at least
> makes the "easy" cases work, but it's a bit too flaky for my taste.

Thanks Bryan for an easy reproduction, and thanks Peff for a suggestion. Let's revert that one for now.