Re: [PATCH 31/44] connect: parse v2 refs with correct hash algorithm

From: Martin Ågren <martin.agren@gmail.com>
Date: May 16, 2020, 11:14 UTC
Message-ID: <CAN0heSpq-t1E6CBsLiVDUE8NYB2ES916MR=d99eug9U=vdOLHQ@mail.gmail.com>
In-Reply-To: <20200513005424.81369-32-sandals@crustytoothpaste.net>

On Wed, 13 May 2020 at 02:58, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 12 quoted lines

>
> When using protocol v2, we need to know what hash algorithm is used by
> the remote end.  See if the server has sent us an object-format
> capability, and if so, use it to determine the hash algorithm in use and
> set that value in the packet reader.  Parse the refs using this
> algorithm.
>
> Note that we use memcpy instead of oidcpy for copying values, since
> oidcpy is intentionally limited to the current hash algorithm length,
> and the copy will be too short if the server side uses SHA-256 but the
> client side has not had a repository set up (and therefore defaults to
> SHA-1).

> -       oidcpy(&ref->old_oid, &old_oid);
> +       memcpy(ref->old_oid.hash, old_oid.hash, reader->hash_algo->rawsz);

Might an `oidcpy_algop()` prove useful over time?

  oidcpy_algop(&ref->old_oid, &old_oid, reader->hash_algo);

Show 20 quoted lines

> @@ -442,6 +444,7 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
>                              const struct string_list *server_options)
>  {
>         int i;
> +       const char *hash_name;
>         *list = NULL;
>
>         if (server_supports_v2("ls-refs", 1))
> @@ -450,6 +453,14 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
>         if (server_supports_v2("agent", 0))
>                 packet_write_fmt(fd_out, "agent=%s", git_user_agent_sanitized());
>
> +       if (server_feature_v2("object-format", &hash_name)) {
> +               int hash_algo = hash_algo_by_name(hash_name);
> +               if (hash_algo == GIT_HASH_UNKNOWN)
> +                       die(_("unknown object format '%s' specified by server"), hash_name);
> +               reader->hash_algo = &hash_algos[hash_algo];
> +               packet_write_fmt(fd_out, "object-format=%s", reader->hash_algo->name);
> +       }
> +

(Similar to an earlier comment I made, if we don't see any "object-format", we rely on `reader->hash_algo` to have been properly set up (which it has) and to not have been modified since (which we could probably rely on, hmm?).)

Martin