Re: [PATCH v5 3/3] promisor-remote: compare remote names case sensitively

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

Christian Couder <christian.couder@gmail.com> writes:

> Because the "[remote "nick"] fetch = ..." configuration variables
> have the nickname in the second part, the nicknames are case
> sensitive, unlike the first and the third component (i.e.
> "remote.origin.fetch" and "Remote.origin.FETCH" are the same thing,
> but "remote.Origin.fetch" and "remote.origin.fetch" are different).

I double-checked what the control flow that passes through
remote.c:handle_config() does, and the above is in line with what
remote_get() does.  

remote.c:read_config() populates the nickname-to-remote hashmap by
using handle_config() callback, which calls make_remote() with the
second level name (e.g. "Origin" and "origin" in the last example of
the above), which is passed to memhash() not memihash() when looking
up or registering the remote.

If we used case insensitive comparison in the new code, a malicious
large-object promisor remote could have told us to use "Origin" as
an extra promisor and in response the new code may noticed that we
have "origin" and tried to equate it with what the other side told
us.  But when the existing code actually interacts with the promisor
remote, it wouldn't have found any configured remote under the name
"Origin", and something funny would start from there.  By using the
right remote consistently throughout the system, we would not get
confused that way, which is good.

> Let's follow the way Git works in general and compare the remote
> names case sensitively when processing advertised remotes.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  Documentation/config/promisor.adoc | 4 ++--
>  promisor-remote.c                  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Looking good.  Thanks.

> diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
> index 9192acfd24..2638b01f83 100644
> --- a/Documentation/config/promisor.adoc
> +++ b/Documentation/config/promisor.adoc
> @@ -26,5 +26,5 @@ promisor.acceptFromServer::
>  	server will be accepted. By accepting a promisor remote, the
>  	client agrees that the server might omit objects that are
>  	lazily fetchable from this promisor remote from its responses
> -	to "fetch" and "clone" requests from the client. See
> -	linkgit:gitprotocol-v2[5].
> +	to "fetch" and "clone" requests from the client. Name and URL
> +	comparisons are case sensitive. See linkgit:gitprotocol-v2[5].
> diff --git a/promisor-remote.c b/promisor-remote.c
> index 0b7b1ec45a..5801ebfd9b 100644
> --- a/promisor-remote.c
> +++ b/promisor-remote.c
> @@ -370,13 +370,13 @@ char *promisor_remote_info(struct repository *repo)
>  
>  /*
>   * Find first index of 'nicks' where there is 'nick'. 'nick' is
> - * compared case insensitively to the strings in 'nicks'. If not found
> + * compared case sensitively to the strings in 'nicks'. If not found
>   * 'nicks->nr' is returned.
>   */
>  static size_t remote_nick_find(struct strvec *nicks, const char *nick)
>  {
>  	for (size_t i = 0; i < nicks->nr; i++)
> -		if (!strcasecmp(nicks->v[i], nick))
> +		if (!strcmp(nicks->v[i], nick))
>  			return i;
>  	return nicks->nr;
>  }