Re: [PATCH 1.5/8] connect: die when a capability line comes after a ref

[Junio C Hamano <gitster@pobox.com>]

Brandon Williams <bmwill@google.com> writes:

>> Or is this only for v2 clients, and we've changed the protocol but
>> get_remote_heads() just needs to be updated, too?
>
> A client which didn't request protocol v1 (I'm calling the current
> protocol v0, and v1 is just v0 with the initial response from the server
> containing a version string) should not receive a version string in the
> initial response.  The problem is that when introducing the version
> string to protocol version 1, I didn't want to have to do a huge
> refactoring of ALL of the current transport code so I stuck the version
> check in get_remote_heads() since v1 is exactly the same as v0, except
> for the first line from the server.

It is still unclear from your response what other things the server
is now allowed to say before "version".  I have a slight suspicion
that this change makes the input language overly loose.  Before
eb398797 ("connect: advertized capability is not a ref", 2016-09-09)
made the "dummy ref must come before any ref and no refs should be
sent if there is a dummy ref sent", the code before it used to allow
a ".have" or a "shallow" to appear at the beginning, but with the
"anything from the other end whatsoever is not allowed before the
dummy one" check the commit introduced, it made it a protocol error
to send these before dummy ref advertisement.  But with this patch,
you are again allowing them to come before the dummy ref, together
with the "version" line you recently added.  I do not know if it is
a problem in practice or not offhand, though.