diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc index 71311b70c8..4147d2cf44 100644 --- a/Documentation/config/promisor.adoc +++ b/Documentation/config/promisor.adoc @@ -46,3 +46,28 @@ 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.checkFields:: + A comma or space separated list of additional remote related + fields that a client will check before accepting a promisor + remote. Currently, only the "partialCloneFilter" and "token" + fields are supported. ++ +When a field is part of this list and a corresponding +"remote.foo." config variable is set locally for remote "foo", +then the value of this config variable will be checked against the +value of the same field passed by the server for the remote "foo". The +remote "foo" will be rejected if the values don't match. ++ +For the "partialCloneFilter" field, this allows the client to ensure +that the server's filter matches what it expects locally, preventing +inconsistencies in filtering behavior. For the "token" field, this can +be used to verify that authentication credentials match expected +values. ++ +The fields should be passed by the server through the +"promisor-remote" capability by using the `promisor.sendFields` config +variable. The fields will be checked only if the +`promisor.acceptFromServer` config variable is not set to "None". If +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 e144290cab..33fd0fc54d 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -377,6 +377,20 @@ static struct string_list *fields_sent(void) return &fields_list; } +static struct string_list *fields_checked(void) +{ + static struct string_list fields_list = STRING_LIST_INIT_NODUP; + static int initialized = 0; + + if (!initialized) { + fields_list.cmp = strcasecmp; + fields_from_config(&fields_list, "promisor.checkFields"); + initialized = 1; + } + + return &fields_list; +} + static void append_fields(struct string_list *fields, struct string_list *field_names, const char *name) @@ -533,15 +547,65 @@ enum accept_promisor { ACCEPT_ALL }; +static int check_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_field(struct string_list_item *item_value, + struct promisor_info *p, int in_list) +{ + if (!in_list) + return check_field_one(item_value, p); + + for (; p; p = p->next) + if (check_field_one(item_value, p)) + return 1; + + return 0; +} + +static int check_all_fields(struct string_list* values, + struct promisor_info *p, + int in_list) +{ + struct string_list* fields = fields_checked(); + struct string_list_item *item_checked; + + string_list_sort(values); + + for_each_string_list_item(item_checked, fields) { + struct string_list_item *item_value; + + item_value = string_list_lookup(values, item_checked->string); + if (!item_value) + return 0; + if (!check_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* values, struct promisor_info *info_list) { struct promisor_info *p; const char *local_url; if (accept == ACCEPT_ALL) - return 1; + return check_all_fields(values, info_list, 1); p = remote_nick_find(info_list, remote_name); @@ -550,7 +614,7 @@ static int should_accept_remote(enum accept_promisor accept, return 0; if (accept == ACCEPT_KNOWN_NAME) - return 1; + return check_all_fields(values, p, 0); if (accept != ACCEPT_KNOWN_URL) BUG("Unhandled 'enum accept_promisor' value '%d'", accept); @@ -568,7 +632,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_fields(values, p, 0); warning(_("known remote named '%s' but with URL '%s' instead of '%s'"), remote_name, local_url, remote_url); @@ -602,9 +666,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); @@ -615,14 +676,29 @@ static void filter_promisor_remote(struct repository *repo, const char *remote_url = NULL; char *decoded_name = NULL; char *decoded_url = NULL; + struct string_list field_values = STRING_LIST_INIT_NODUP; + + field_values.cmp = strcasecmp; 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(&field_values, elems[j]->buf)->util = p + 1; + } else { + warning(_("invalid element '%s' from remote info"), + elems[j]->buf); + } } if (remote_name) @@ -630,9 +706,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, fields_checked()); + if (should_accept_remote(accept, decoded_name, decoded_url, + &field_values, info_list)) + strvec_push(accepted, decoded_name); + } + + string_list_clear(&field_values, 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 ca2ef25415..5e9346cf26 100755 --- a/t/t5710-promisor-remote-capability.sh +++ b/t/t5710-promisor-remote-capability.sh @@ -327,6 +327,41 @@ test_expect_success "clone with promisor.sendFields" ' check_missing_objects server 1 "$oid" ' +test_expect_success "clone with promisor.checkFields" ' + 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.token "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.sendFields "token, partialCloneFilter" && + test_when_finished "git -C server config unset promisor.sendFields" && + 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.checkFields=partialcloneFilter \ + --no-local --filter="blob:limit=5k" server client && + + # Check that 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,token=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 &&