upload-pack: only accept packfile-uris if we advertised it

Clients are only supposed to request particular capabilities or features
if the server advertised them. For the "packfile-uris" feature, we only
advertise it if uploadpack.blobpacfileuri is set, but we always accept a
request from the client regardless.

In practice this doesn't really hurt anything, as we'd pass the client's
protocol list on to pack-objects, which ends up ignoring it. But we
should try to follow the protocol spec, and tightening this up may catch
buggy or misbehaving clients more easily.

Thanks to recent refactoring, we can hoist the config check from
upload_pack_advertise() into upload_pack_config(). Note the subtle
handling of a value-less bool (which does not count for triggering an
advertisement).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 6ef4971845fb4407977e98cb62b3468abfca5838..1ef540f73d34756673423ff77fa46567bd34b2ab 100755 (executable)
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -778,6 +778,25 @@ test_expect_success 'archive with custom path does not request v2' '
        ! grep ^GIT_PROTOCOL env.trace
 '
 
+test_expect_success 'reject client packfile-uris if not advertised' '
+       {
+               packetize command=fetch &&
+               packetize object-format=$(test_oid algo) &&
+               printf 0001 &&
+               packetize packfile-uris https &&
+               packetize done &&
+               printf 0000
+       } >input &&
+       test_must_fail env GIT_PROTOCOL=version=2 \
+               git upload-pack client <input &&
+       test_must_fail env GIT_PROTOCOL=version=2 \
+               git -c uploadpack.blobpackfileuri \
+               upload-pack client <input &&
+       GIT_PROTOCOL=version=2 \
+               git -c uploadpack.blobpackfileuri=anything \
+               upload-pack client <input
+'
+
 # Test protocol v2 with 'http://' transport
 #
 . "$TEST_DIRECTORY"/lib-httpd.sh

diff --git a/upload-pack.c b/upload-pack.c
index 491ef51daaf295e90c9f473be755fbddffdd0903..66f4de9d8723a088e02232ee3723fa0758cd6af7 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -113,6 +113,7 @@ struct upload_pack_data {
        unsigned done : 1;                                      /* v2 only */
        unsigned allow_ref_in_want : 1;                         /* v2 only */
        unsigned allow_sideband_all : 1;                        /* v2 only */
+       unsigned allow_packfile_uris : 1;                       /* v2 only */
        unsigned advertise_sid : 1;
        unsigned sent_capabilities : 1;
 };
@@ -1362,6 +1363,9 @@ static int upload_pack_config(const char *var, const char *value,
                data->allow_ref_in_want = git_config_bool(var, value);
        } else if (!strcmp("uploadpack.allowsidebandall", var)) {
                data->allow_sideband_all = git_config_bool(var, value);
+       } else if (!strcmp("uploadpack.blobpackfileuri", var)) {
+               if (value)
+                       data->allow_packfile_uris = 1;
        } else if (!strcmp("core.precomposeunicode", var)) {
                precomposed_unicode = git_config_bool(var, value);
        } else if (!strcmp("transfer.advertisesid", var)) {
@@ -1647,7 +1651,8 @@ static void process_args(struct packet_reader *request,
                        continue;
                }
 
-               if (skip_prefix(arg, "packfile-uris ", &p)) {
+               if (data->allow_packfile_uris &&
+                   skip_prefix(arg, "packfile-uris ", &p)) {
                        string_list_split(&data->uri_protocols, p, ',', -1);
                        continue;
                }
@@ -1847,8 +1852,6 @@ int upload_pack_advertise(struct repository *r,
        get_upload_pack_config(r, &data);
 
        if (value) {
-               char *str = NULL;
-
                strbuf_addstr(value, "shallow wait-for-done");
 
                if (data.allow_filter)
@@ -1860,13 +1863,8 @@ int upload_pack_advertise(struct repository *r,
                if (data.allow_sideband_all)
                        strbuf_addstr(value, " sideband-all");
 
-               if (!repo_config_get_string(r,
-                                           "uploadpack.blobpackfileuri",
-                                           &str) &&
-                   str) {
+               if (data.allow_packfile_uris)
                        strbuf_addstr(value, " packfile-uris");
-                       free(str);
-               }
        }
 
        upload_pack_data_clear(&data);