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

Brandon Williams [off-list ref] writes:

quoted

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.