upload-pack: implement ref-in-want

Currently, while performing packfile negotiation, clients are only allowed to specify their desired objects using object ids. This causes a vulnerability to failure when an object turns non-existent during negotiation, which may happen if, for example, the desired repository is provided by multiple Git servers in a load-balancing arrangement and there exists replication delay. In order to eliminate this vulnerability, implement the ref-in-want feature for the 'fetch' command in protocol version 2. This feature enables the 'fetch' command to support requests in the form of ref names through a new "want-ref <ref>" parameter. At the conclusion of negotiation, the server will send a list of all of the wanted references (as provided by "want-ref" lines) in addition to the generated packfile. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Williams committed Jun 27, 2018 at 15:30 UTC 516e2b76bdcf53e757309481fa0e663217ee8039
4 files changed +260 -1
Documentation/config.txt
+7
@@ -3479,6 +3479,13 @@ Note that this configuration variable is ignored if it is seen in the
3479 repository-level config (this is a safety measure against fetching from
3480 untrusted repositories).
3481
3482 +uploadpack.allowRefInWant::
3483 + If this option is set, `upload-pack` will support the `ref-in-want`
3484 + feature of the protocol version 2 `fetch` command. This feature
3485 + is intended for the benefit of load-balanced servers which may
3486 + not have the same view of what OIDs their refs point to due to
3487 + replication delay.
3488 +
3489 url.<base>.insteadOf::
3490 Any URL that starts with this value will be rewritten to
3491 start, instead, with <base>. In cases where some site serves a
Documentation/technical/protocol-v2.txt
+27 -1
@@ -299,12 +299,21 @@ included in the client's request:
299 for use with partial clone and partial fetch operations. See
300 `rev-list` for possible "filter-spec" values.
301
302 +If the 'ref-in-want' feature is advertised, the following argument can
303 +be included in the client's request as well as the potential addition of
304 +the 'wanted-refs' section in the server's response as explained below.
305 +
306 + want-ref <ref>
307 + Indicates to the server that the client wants to retrieve a
308 + particular ref, where <ref> is the full name of a ref on the
309 + server.
310 +
311 The response of `fetch` is broken into a number of sections separated by
312 delimiter packets (0001), with each section beginning with its section
313 header.
314
315 output = *section
307 - section = (acknowledgments | shallow-info | packfile)
316 + section = (acknowledgments | shallow-info | wanted-refs | packfile)
317 (flush-pkt | delim-pkt)
318
319 acknowledgments = PKT-LINE("acknowledgments" LF)
@@ -319,6 +328,10 @@ header.
328 shallow = "shallow" SP obj-id
329 unshallow = "unshallow" SP obj-id
330
331 + wanted-refs = PKT-LINE("wanted-refs" LF)
332 + *PKT-LINE(wanted-ref LF)
333 + wanted-ref = obj-id SP refname
334 +
335 packfile = PKT-LINE("packfile" LF)
336 *PKT-LINE(%x01-03 *%x00-ff)
337
@@ -379,6 +392,19 @@ header.
392 * This section is only included if a packfile section is also
393 included in the response.
394
395 + wanted-refs section
396 + * This section is only included if the client has requested a
397 + ref using a 'want-ref' line and if a packfile section is also
398 + included in the response.
399 +
400 + * Always begins with the section header "wanted-refs".
401 +
402 + * The server will send a ref listing ("<oid> <refname>") for
403 + each reference requested using 'want-ref' lines.
404 +
405 + * The server MUST NOT send any refs which were not requested
406 + using 'want-ref' lines.
407 +
408 packfile section
409 * This section is only included if the client has sent 'want'
410 lines in its request and either requested that no more
t/t5703-upload-pack-ref-in-want.sh new
+160
@@ -0,0 +1,160 @@
1 +#!/bin/sh
2 +
3 +test_description='upload-pack ref-in-want'
4 +
5 +. ./test-lib.sh
6 +
7 +get_actual_refs () {
8 + sed -n -e '/wanted-refs/,/0001/{
9 + /wanted-refs/d
10 + /0001/d
11 + p
12 + }' <out | test-pkt-line unpack >actual_refs
13 +}
14 +
15 +get_actual_commits () {
16 + sed -n -e '/packfile/,/0000/{
17 + /packfile/d
18 + p
19 + }' <out | test-pkt-line unpack-sideband >o.pack &&
20 + git index-pack o.pack &&
21 + git verify-pack -v o.idx | grep commit | cut -c-40 | sort >actual_commits
22 +}
23 +
24 +check_output () {
25 + get_actual_refs &&
26 + test_cmp expected_refs actual_refs &&
27 + get_actual_commits &&
28 + test_cmp expected_commits actual_commits
29 +}
30 +
31 +# c(o/foo) d(o/bar)
32 +# \ /
33 +# b e(baz) f(master)
34 +# \__ | __/
35 +# \ | /
36 +# a
37 +test_expect_success 'setup repository' '
38 + test_commit a &&
39 + git checkout -b o/foo &&
40 + test_commit b &&
41 + test_commit c &&
42 + git checkout -b o/bar b &&
43 + test_commit d &&
44 + git checkout -b baz a &&
45 + test_commit e &&
46 + git checkout master &&
47 + test_commit f
48 +'
49 +
50 +test_expect_success 'config controls ref-in-want advertisement' '
51 + git serve --advertise-capabilities >out &&
52 + ! grep -a ref-in-want out &&
53 +
54 + git config uploadpack.allowRefInWant false &&
55 + git serve --advertise-capabilities >out &&
56 + ! grep -a ref-in-want out &&
57 +
58 + git config uploadpack.allowRefInWant true &&
59 + git serve --advertise-capabilities >out &&
60 + grep -a ref-in-want out
61 +'
62 +
63 +test_expect_success 'invalid want-ref line' '
64 + test-pkt-line pack >in <<-EOF &&
65 + command=fetch
66 + 0001
67 + no-progress
68 + want-ref refs/heads/non-existent
69 + done
70 + 0000
71 + EOF
72 +
73 + test_must_fail git serve --stateless-rpc 2>out <in &&
74 + grep "unknown ref" out
75 +'
76 +
77 +test_expect_success 'basic want-ref' '
78 + cat >expected_refs <<-EOF &&
79 + $(git rev-parse f) refs/heads/master
80 + EOF
81 + git rev-parse f | sort >expected_commits &&
82 +
83 + test-pkt-line pack >in <<-EOF &&
84 + command=fetch
85 + 0001
86 + no-progress
87 + want-ref refs/heads/master
88 + have $(git rev-parse a)
89 + done
90 + 0000
91 + EOF
92 +
93 + git serve --stateless-rpc >out <in &&
94 + check_output
95 +'
96 +
97 +test_expect_success 'multiple want-ref lines' '
98 + cat >expected_refs <<-EOF &&
99 + $(git rev-parse c) refs/heads/o/foo
100 + $(git rev-parse d) refs/heads/o/bar
101 + EOF
102 + git rev-parse c d | sort >expected_commits &&
103 +
104 + test-pkt-line pack >in <<-EOF &&
105 + command=fetch
106 + 0001
107 + no-progress
108 + want-ref refs/heads/o/foo
109 + want-ref refs/heads/o/bar
110 + have $(git rev-parse b)
111 + done
112 + 0000
113 + EOF
114 +
115 + git serve --stateless-rpc >out <in &&
116 + check_output
117 +'
118 +
119 +test_expect_success 'mix want and want-ref' '
120 + cat >expected_refs <<-EOF &&
121 + $(git rev-parse f) refs/heads/master
122 + EOF
123 + git rev-parse e f | sort >expected_commits &&
124 +
125 + test-pkt-line pack >in <<-EOF &&
126 + command=fetch
127 + 0001
128 + no-progress
129 + want-ref refs/heads/master
130 + want $(git rev-parse e)
131 + have $(git rev-parse a)
132 + done
133 + 0000
134 + EOF
135 +
136 + git serve --stateless-rpc >out <in &&
137 + check_output
138 +'
139 +
140 +test_expect_success 'want-ref with ref we already have commit for' '
141 + cat >expected_refs <<-EOF &&
142 + $(git rev-parse c) refs/heads/o/foo
143 + EOF
144 + >expected_commits &&
145 +
146 + test-pkt-line pack >in <<-EOF &&
147 + command=fetch
148 + 0001
149 + no-progress
150 + want-ref refs/heads/o/foo
151 + have $(git rev-parse c)
152 + done
153 + 0000
154 + EOF
155 +
156 + git serve --stateless-rpc >out <in &&
157 + check_output
158 +'
159 +
160 +test_done
upload-pack.c
+66
@@ -64,6 +64,7 @@ static const char *pack_objects_hook;
64
65 static int filter_capability_requested;
66 static int allow_filter;
67 +static int allow_ref_in_want;
68 static struct list_objects_filter_options filter_options;
69
70 static void reset_timeout(void)
@@ -1075,6 +1076,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
1076 return git_config_string(&pack_objects_hook, var, value);
1077 } else if (!strcmp("uploadpack.allowfilter", var)) {
1078 allow_filter = git_config_bool(var, value);
1079 + } else if (!strcmp("uploadpack.allowrefinwant", var)) {
1080 + allow_ref_in_want = git_config_bool(var, value);
1081 }
1082 return parse_hide_refs_config(var, value, "uploadpack");
1083 }
@@ -1114,6 +1117,7 @@ void upload_pack(struct upload_pack_options *options)
1117
1118 struct upload_pack_data {
1119 struct object_array wants;
1120 + struct string_list wanted_refs;
1121 struct oid_array haves;
1122
1123 struct object_array shallows;
@@ -1135,12 +1139,14 @@ struct upload_pack_data {
1139 static void upload_pack_data_init(struct upload_pack_data *data)
1140 {
1141 struct object_array wants = OBJECT_ARRAY_INIT;
1142 + struct string_list wanted_refs = STRING_LIST_INIT_DUP;
1143 struct oid_array haves = OID_ARRAY_INIT;
1144 struct object_array shallows = OBJECT_ARRAY_INIT;
1145 struct string_list deepen_not = STRING_LIST_INIT_DUP;
1146
1147 memset(data, 0, sizeof(*data));
1148 data->wants = wants;
1149 + data->wanted_refs = wanted_refs;
1150 data->haves = haves;
1151 data->shallows = shallows;
1152 data->deepen_not = deepen_not;
@@ -1149,6 +1155,7 @@ static void upload_pack_data_init(struct upload_pack_data *data)
1155 static void upload_pack_data_clear(struct upload_pack_data *data)
1156 {
1157 object_array_clear(&data->wants);
1158 + string_list_clear(&data->wanted_refs, 1);
1159 oid_array_clear(&data->haves);
1160 object_array_clear(&data->shallows);
1161 string_list_clear(&data->deepen_not, 0);
@@ -1185,6 +1192,34 @@ static int parse_want(const char *line)
1192 return 0;
1193 }
1194
1195 +static int parse_want_ref(const char *line, struct string_list *wanted_refs)
1196 +{
1197 + const char *arg;
1198 + if (skip_prefix(line, "want-ref ", &arg)) {
1199 + struct object_id oid;
1200 + struct string_list_item *item;
1201 + struct object *o;
1202 +
1203 + if (read_ref(arg, &oid)) {
1204 + packet_write_fmt(1, "ERR unknown ref %s", arg);
1205 + die("unknown ref %s", arg);
1206 + }
1207 +
1208 + item = string_list_append(wanted_refs, arg);
1209 + item->util = oiddup(&oid);
1210 +
1211 + o = parse_object_or_die(&oid, arg);
1212 + if (!(o->flags & WANTED)) {
1213 + o->flags |= WANTED;
1214 + add_object_array(o, NULL, &want_obj);
1215 + }
1216 +
1217 + return 1;
1218 + }
1219 +
1220 + return 0;
1221 +}
1222 +
1223 static int parse_have(const char *line, struct oid_array *haves)
1224 {
1225 const char *arg;
@@ -1210,6 +1245,8 @@ static void process_args(struct packet_reader *request,
1245 /* process want */
1246 if (parse_want(arg))
1247 continue;
1248 + if (allow_ref_in_want && parse_want_ref(arg, &data->wanted_refs))
1249 + continue;
1250 /* process have line */
1251 if (parse_have(arg, &data->haves))
1252 continue;
@@ -1352,6 +1389,24 @@ static int process_haves_and_send_acks(struct upload_pack_data *data)
1389 return ret;
1390 }
1391
1392 +static void send_wanted_ref_info(struct upload_pack_data *data)
1393 +{
1394 + const struct string_list_item *item;
1395 +
1396 + if (!data->wanted_refs.nr)
1397 + return;
1398 +
1399 + packet_write_fmt(1, "wanted-refs\n");
1400 +
1401 + for_each_string_list_item(item, &data->wanted_refs) {
1402 + packet_write_fmt(1, "%s %s\n",
1403 + oid_to_hex(item->util),
1404 + item->string);
1405 + }
1406 +
1407 + packet_delim(1);
1408 +}
1409 +
1410 static void send_shallow_info(struct upload_pack_data *data)
1411 {
1412 /* No shallow info needs to be sent */
@@ -1418,6 +1473,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
1473 state = FETCH_DONE;
1474 break;
1475 case FETCH_SEND_PACK:
1476 + send_wanted_ref_info(&data);
1477 send_shallow_info(&data);
1478
1479 packet_write_fmt(1, "packfile\n");
@@ -1438,12 +1494,22 @@ int upload_pack_advertise(struct repository *r,
1494 {
1495 if (value) {
1496 int allow_filter_value;
1497 + int allow_ref_in_want;
1498 +
1499 strbuf_addstr(value, "shallow");
1500 +
1501 if (!repo_config_get_bool(the_repository,
1502 "uploadpack.allowfilter",
1503 &allow_filter_value) &&
1504 allow_filter_value)
1505 strbuf_addstr(value, " filter");
1506 +
1507 + if (!repo_config_get_bool(the_repository,
1508 + "uploadpack.allowrefinwant",
1509 + &allow_ref_in_want) &&
1510 + allow_ref_in_want)
1511 + strbuf_addstr(value, " ref-in-want");
1512 }
1513 +
1514 return 1;
1515 }