upload-pack: use existing config mechanism for advertisement

When serving a v2 capabilities request, we call upload_pack_advertise() to tell us the set of features we can advertise to the client. That involves looking at various config options, all of which need to be kept in sync with the rules we use in upload_pack_config to set flags like allow_filter, allow_sideband_all, and so on. If these two pieces of code get out of sync then we may refuse to respect a capability we advertised, or vice versa accept one that we should not. Instead, let's call the same config helper that we'll use for processing the actual client request, and then just pick the values out of the resulting struct. This is only a little bit shorter than the current code, but we don't repeat any policy logic (e.g., we don't have to worry about the magic sideband-all environment variable here anymore). And this reveals a gap in the existing code: there is no struct flag for the packfile-uris capability (we accept it even if it is not advertised, which we should not). We'll leave the advertisement code for now and deal with it in the next patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 28, 2024 at 17:48 UTC 9a7b22959ad078df1f50d15e86a169aaebfb8c4c
1 file changed +10 -16
upload-pack.c
+10 -16
@@ -1841,31 +1841,23 @@ int upload_pack_v2(struct repository *r, struct packet_reader *request)
1841 int upload_pack_advertise(struct repository *r,
1842 struct strbuf *value)
1843 {
1844 + struct upload_pack_data data;
1845 +
1846 + upload_pack_data_init(&data);
1847 + get_upload_pack_config(r, &data);
1848 +
1849 if (value) {
1845 - int allow_filter_value;
1846 - int allow_ref_in_want;
1847 - int allow_sideband_all_value;
1850 char *str = NULL;
1851
1852 strbuf_addstr(value, "shallow wait-for-done");
1853
1852 - if (!repo_config_get_bool(r,
1853 - "uploadpack.allowfilter",
1854 - &allow_filter_value) &&
1855 - allow_filter_value)
1854 + if (data.allow_filter)
1855 strbuf_addstr(value, " filter");
1856
1858 - if (!repo_config_get_bool(r,
1859 - "uploadpack.allowrefinwant",
1860 - &allow_ref_in_want) &&
1861 - allow_ref_in_want)
1857 + if (data.allow_ref_in_want)
1858 strbuf_addstr(value, " ref-in-want");
1859
1864 - if (git_env_bool("GIT_TEST_SIDEBAND_ALL", 0) ||
1865 - (!repo_config_get_bool(r,
1866 - "uploadpack.allowsidebandall",
1867 - &allow_sideband_all_value) &&
1868 - allow_sideband_all_value))
1860 + if (data.allow_sideband_all)
1861 strbuf_addstr(value, " sideband-all");
1862
1863 if (!repo_config_get_string(r,
@@ -1877,5 +1869,7 @@ int upload_pack_advertise(struct repository *r,
1869 }
1870 }
1871
1872 + upload_pack_data_clear(&data);
1873 +
1874 return 1;
1875 }