Web lists-archives.com

Re: [PATCH 1/1] protocol: limit max protocol version per service




On Tue, Oct 2, 2018 at 3:00 PM Josh Steadmon <steadmon@xxxxxxxxxx> wrote:
>
> For services other than git-receive-pack, clients currently advertise
> that they support the version set in the protocol.version config,
> regardless of whether or not there is actually an implementation of that
> service for the given protocol version. This causes backwards-
> compatibility problems when a new implementation for the given
> protocol version is added.
>
> This patch sets maximum allowed protocol versions for git-receive-pack,
> git-upload-archive, and git-upload-pack.
>
> Previously, git-receive-pack would downgrade from v2 to v0, but would
> allow v1 if set in protocol.version. Now, it will downgrade from v2 to
> v1.

But does git-receive-pack understand v1?
As to my understanding we have not even defined v1
for push (receive-pack) and archive --remote (upload-archive).
v1 is only known to fetch (upload-pack).

> +enum protocol_version determine_maximum_protocol_version(
> +               const char *service, enum protocol_version default_version)
> +{
> +       if (!strcmp(service, "git-receive-pack"))
> +               return protocol_v1;
> +       else if (!strcmp(service, "git-upload-archive"))
> +               return protocol_v1;

so I would think these two would be _v0.
... goes and checks ...
aa9bab29b8 (upload-pack, receive-pack: introduce protocol version 1,
2017-10-16) seems to actually teach v1 to receive-pack as well,
but upload-archive was completely off radar, so I think returning
(v1, v0, v2 in the order as in the code) would make sense?

Asides from this, I thought there was a deliberate decision
that we'd want to avoid a strict order on the protocol versions,
but I could not find prior discussion on list to back up this claim. :/

For example we'd go with e.g. enums instead of integers
for version numbers, as then some internal setup could
also have things like protocol_v2018-10-02 or protocol_vWhatever;
some protocol version may be advantageous to the client, some to
the server, and we'd need to negotiate the best version that both
are happy with. (e.g. the server may like version 0, 2 and 3, and
the client may like 0,2,4 as 3 is bad security wise for the client,
so both would negotiate to 2 as their best case)

>From a maintenance perspective, do we want to keep
this part of the code central, as it ties protocol (as proxied
by service name) to the max version number?
I would think that we'd rather have the decision local to the
code, i.e. builtin/fetch would need to tell protocol.c that it
can do (0,1,2) and builtin/push can do (0,1), and then the
networking layers of code would figure out by the input
from the caller and the input from the user (configured
protocol.version) what is the best to go forward from
then on.

But I guess having the central place here is not to
bad either. How will it cope with the desire of protocol v2
to have only one end point (c.f. serve.{c,h} via builtin/serve
as "git serve") ?

Stefan