A previous commit allowed a server to pass extra fields through the
"promisor-remote" protocol capability after the "name" and "url"
fields.
Let's make it possible for a client to check if these extra fields
match what it expects before accepting a promisor remote.
We allow this by introducing a new "promisor.checkExtraFields"
configuration variable. It should contain a comma or space
separated list of fields that will be checked.
Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
---
Documentation/config/promisor.adoc | 17 +++++
promisor-remote.c | 102 +++++++++++++++++++++++---
t/t5710-promisor-remote-capability.sh | 35 +++++++++
3 files changed, 143 insertions(+), 11 deletions(-)
diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
index bc08999733..75207de2ac 100644
--- a/Documentation/config/promisor.adoc
+++ b/Documentation/config/promisor.adoc
@@ -43,3 +43,20 @@ promisor.acceptFromServer::
lazily fetchable from this promisor remote from its responses
to "fetch" and "clone" requests from the client. Name and URL
comparisons are case sensitive. See linkgit:gitprotocol-v2[5].
+
+promisor.checkExtraFields::
+ A comma or space separated list of additional remote related
+ fields that a client will check before accepting a promisor
+ remote. When a field named "bar" is part of this list and a
+ corresponding "remote.foo.bar" config variable is set for
+ locally for remote "foo", then the value of the
+ "remote.foo.bar" config variable will be checked against the
+ value of the "bar" field passed by the server through the
+ "promisor-remote" capability for the remote "foo". The remote
+ "foo" will be rejected if the values don't match. The fields
+ should be passed by the server through the "promisor-remote"
+ capability by using the `promisor.sendExtraFields` config
+ variable, see above. The extra fields will be checked only if
+ the `promisor.acceptFromServer` config variable, see above, is
+ not set to "None". If is set to "None", this config variable
+ will have no effect. See linkgit:gitprotocol-v2[5].
diff --git a/promisor-remote.c b/promisor-remote.c
index 424d88d4a1..3645093f2f 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -358,6 +358,19 @@ static struct string_list *extra_fields_sent(void)
return &fields_list;
}
+static struct string_list *extra_fields_checked(void)
+{
+ static struct string_list fields_list = STRING_LIST_INIT_NODUP;
+ static int initialized = 0;
+
+ if (!initialized) {
+ fields_from_config(&fields_list, "promisor.checkExtraFields");
+ initialized = 1;
+ }
+
+ return &fields_list;
+}
+
static void append_extra_fields(struct string_list *fields,
struct string_list *extra_fields,
const char *name)
@@ -497,15 +510,65 @@ enum accept_promisor {
ACCEPT_ALL
};
+static int check_extra_field_one(struct string_list_item *item_value,
+ struct promisor_info *p)
+{
+ struct string_list_item *item;
+
+ item = unsorted_string_list_lookup(&p->fields, item_value->string);
+ if (!item)
+ return 0;
+
+ return !strcmp(item->util, item_value->util);
+}
+
+
+static int check_extra_field(struct string_list_item *item_value,
+ struct promisor_info *p, int in_list)
+{
+ if (!in_list)
+ return check_extra_field_one(item_value, p);
+
+ for (; p; p = p->next)
+ if (check_extra_field_one(item_value, p))
+ return 1;
+
+ return 0;
+}
+
+static int check_all_extra_fields(struct string_list* extra_values,
+ struct promisor_info *p,
+ int in_list)
+{
+ struct string_list* fields_checked = extra_fields_checked();
+ struct string_list_item *item_checked;
+
+ string_list_sort(extra_values);
+
+ for_each_string_list_item(item_checked, fields_checked) {
+ struct string_list_item *item_value;
+
+ item_value = string_list_lookup(extra_values, item_checked->string);
+ if (!item_value)
+ return 0;
+ if (!check_extra_field(item_value, p, in_list))
+ return 0;
+ }
+
+ return 1;
+}
+
static int should_accept_remote(enum accept_promisor accept,
- const char *remote_name, const char *remote_url,
+ const char *remote_name,
+ const char *remote_url,
+ struct string_list* extras,
struct promisor_info *info_list)
{
struct promisor_info *p;
const char *local_url;
if (accept == ACCEPT_ALL)
- return 1;
+ return check_all_extra_fields(extras, info_list, 1);
p = remote_nick_find(info_list, remote_name);
@@ -514,7 +577,7 @@ static int should_accept_remote(enum accept_promisor accept,
return 0;
if (accept == ACCEPT_KNOWN_NAME)
- return 1;
+ return check_all_extra_fields(extras, p, 0);
if (accept != ACCEPT_KNOWN_URL)
BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
@@ -530,7 +593,7 @@ static int should_accept_remote(enum accept_promisor accept,
local_url = p->fields.items[1].util;
if (!strcmp(local_url, remote_url))
- return 1;
+ return check_all_extra_fields(extras, p, 0);
warning(_("known remote named '%s' but with URL '%s' instead of '%s'"),
remote_name, local_url, remote_url);
@@ -564,9 +627,6 @@ static void filter_promisor_remote(struct repository *repo,
if (accept == ACCEPT_NONE)
return;
- if (accept != ACCEPT_ALL)
- info_list = promisor_info_list(repo, NULL);
-
/* Parse remote info received */
remotes = strbuf_split_str(info, ';', 0);
@@ -577,14 +637,27 @@ static void filter_promisor_remote(struct repository *repo,
const char *remote_url = NULL;
char *decoded_name = NULL;
char *decoded_url = NULL;
+ struct string_list extras = STRING_LIST_INIT_NODUP;
strbuf_strip_suffix(remotes[i], ";");
elems = strbuf_split(remotes[i], ',');
for (size_t j = 0; elems[j]; j++) {
+ char *p;
+
strbuf_strip_suffix(elems[j], ",");
- if (!skip_prefix(elems[j]->buf, "name=", &remote_name))
- skip_prefix(elems[j]->buf, "url=", &remote_url);
+ if (skip_prefix(elems[j]->buf, "name=", &remote_name) ||
+ skip_prefix(elems[j]->buf, "url=", &remote_url))
+ continue;
+
+ p = strchr(elems[j]->buf, '=');
+ if (p) {
+ *p = '\0';
+ string_list_append(&extras, elems[j]->buf)->util = p + 1;
+ } else {
+ warning(_("invalid element '%s' from remote info"),
+ elems[j]->buf);
+ }
}
if (remote_name)
@@ -592,9 +665,16 @@ static void filter_promisor_remote(struct repository *repo,
if (remote_url)
decoded_url = url_percent_decode(remote_url);
- if (decoded_name && should_accept_remote(accept, decoded_name, decoded_url, info_list))
- strvec_push(accepted, decoded_name);
+ if (decoded_name) {
+ if (!info_list)
+ info_list = promisor_info_list(repo, extra_fields_checked());
+
+ if (should_accept_remote(accept, decoded_name, decoded_url,
+ &extras, info_list))
+ strvec_push(accepted, decoded_name);
+ }
+ string_list_clear(&extras, 0);
strbuf_list_free(elems);
free(decoded_name);
free(decoded_url);
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 26f3c63112..858b34587d 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -321,6 +321,41 @@ test_expect_success "clone with promisor.sendExtraFields" '
check_missing_objects server 1 "$oid"
'
+test_expect_success "clone with promisor.checkExtraFields" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ git -C server remote add otherLop "https://invalid.invalid"; &&
+ git -C server config remote.otherLop.id "fooBar" &&
+ git -C server config remote.otherLop.stuff "baz" &&
+ git -C server config remote.otherLop.partialCloneFilter "blob:limit=10k" &&
+ test_when_finished "git -C server remote remove otherLop" &&
+ git -C server config promisor.sendExtraFields "id, partialCloneFilter" &&
+ test_when_finished "git -C server config unset promisor.sendExtraFields" &&
+ test_when_finished "rm trace" &&
+
+ # Clone from server to create a client
+ GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
+ -c remote.lop.promisor=true \
+ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+ -c remote.lop.url="file://$(pwd)/lop" \
+ -c remote.lop.partialCloneFilter="blob:none" \
+ -c promisor.acceptfromserver=All \
+ -c promisor.checkExtraFields=partialCloneFilter \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that extra fields are properly transmitted
+ ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") &&
+ PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" &&
+ PR2="name=otherLop,url=https://invalid.invalid,id=fooBar,partialCloneFilter=blob:limit=10k"; &&
+ test_grep "clone< promisor-remote=$PR1;$PR2" trace &&
+ test_grep "clone> promisor-remote=lop" trace &&
+ test_grep ! "clone> promisor-remote=lop;otherLop" trace &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" '
git -C server config promisor.advertise true &&
--
2.49.0.158.g6ac6832dc3.dirty
