Re: [PATCH v5 3/3] promisor-remote: compare remote names case sensitively
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 14, 2025, 17:28 UTC
- Message-ID
- <xmqqo6y3h67u.fsf@gitster.g>
- In-Reply-To
- <20250314141203.2548803-4-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
Show 5 quoted lines
> 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.
Show 8 quoted lines
> 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.
Show 32 quoted lines
> 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;
> }