Skip to content

Commit 516e2b7

Browse files
bmwillgitster
bmwill
authored and
gitster
committed
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>
1 parent 4bcd37d commit 516e2b7

File tree

4 files changed

+260
-1
lines changed

4 files changed

+260
-1
lines changed

‎Documentation/config.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3479,6 +3479,13 @@ Note that this configuration variable is ignored if it is seen in the
34793479
repository-level config (this is a safety measure against fetching from
34803480
untrusted repositories).
34813481

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+
34823489
url.<base>.insteadOf::
34833490
Any URL that starts with this value will be rewritten to
34843491
start, instead, with <base>. In cases where some site serves a

‎Documentation/technical/protocol-v2.txt‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,21 @@ included in the client's request:
299299
for use with partial clone and partial fetch operations. See
300300
`rev-list` for possible "filter-spec" values.
301301

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+
302311
The response of `fetch` is broken into a number of sections separated by
303312
delimiter packets (0001), with each section beginning with its section
304313
header.
305314

306315
output = *section
307-
section = (acknowledgments | shallow-info | packfile)
316+
section = (acknowledgments | shallow-info | wanted-refs | packfile)
308317
(flush-pkt | delim-pkt)
309318

310319
acknowledgments = PKT-LINE("acknowledgments" LF)
@@ -319,6 +328,10 @@ header.
319328
shallow = "shallow" SP obj-id
320329
unshallow = "unshallow" SP obj-id
321330

331+
wanted-refs = PKT-LINE("wanted-refs" LF)
332+
*PKT-LINE(wanted-ref LF)
333+
wanted-ref = obj-id SP refname
334+
322335
packfile = PKT-LINE("packfile" LF)
323336
*PKT-LINE(%x01-03 *%x00-ff)
324337

@@ -379,6 +392,19 @@ header.
379392
* This section is only included if a packfile section is also
380393
included in the response.
381394

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+
382408
packfile section
383409
* This section is only included if the client has sent 'want'
384410
lines in its request and either requested that no more

‎t/t5703-upload-pack-ref-in-want.sh‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ static const char *pack_objects_hook;
6464

6565
static int filter_capability_requested;
6666
static int allow_filter;
67+
static int allow_ref_in_want;
6768
static struct list_objects_filter_options filter_options;
6869

6970
static void reset_timeout(void)
@@ -1075,6 +1076,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
10751076
return git_config_string(&pack_objects_hook, var, value);
10761077
} else if (!strcmp("uploadpack.allowfilter", var)) {
10771078
allow_filter = git_config_bool(var, value);
1079+
} else if (!strcmp("uploadpack.allowrefinwant", var)) {
1080+
allow_ref_in_want = git_config_bool(var, value);
10781081
}
10791082
return parse_hide_refs_config(var, value, "uploadpack");
10801083
}
@@ -1114,6 +1117,7 @@ void upload_pack(struct upload_pack_options *options)
11141117

11151118
struct upload_pack_data {
11161119
struct object_array wants;
1120+
struct string_list wanted_refs;
11171121
struct oid_array haves;
11181122

11191123
struct object_array shallows;
@@ -1135,12 +1139,14 @@ struct upload_pack_data {
11351139
static void upload_pack_data_init(struct upload_pack_data *data)
11361140
{
11371141
struct object_array wants = OBJECT_ARRAY_INIT;
1142+
struct string_list wanted_refs = STRING_LIST_INIT_DUP;
11381143
struct oid_array haves = OID_ARRAY_INIT;
11391144
struct object_array shallows = OBJECT_ARRAY_INIT;
11401145
struct string_list deepen_not = STRING_LIST_INIT_DUP;
11411146

11421147
memset(data, 0, sizeof(*data));
11431148
data->wants = wants;
1149+
data->wanted_refs = wanted_refs;
11441150
data->haves = haves;
11451151
data->shallows = shallows;
11461152
data->deepen_not = deepen_not;
@@ -1149,6 +1155,7 @@ static void upload_pack_data_init(struct upload_pack_data *data)
11491155
static void upload_pack_data_clear(struct upload_pack_data *data)
11501156
{
11511157
object_array_clear(&data->wants);
1158+
string_list_clear(&data->wanted_refs, 1);
11521159
oid_array_clear(&data->haves);
11531160
object_array_clear(&data->shallows);
11541161
string_list_clear(&data->deepen_not, 0);
@@ -1185,6 +1192,34 @@ static int parse_want(const char *line)
11851192
return 0;
11861193
}
11871194

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+
11881223
static int parse_have(const char *line, struct oid_array *haves)
11891224
{
11901225
const char *arg;
@@ -1210,6 +1245,8 @@ static void process_args(struct packet_reader *request,
12101245
/* process want */
12111246
if (parse_want(arg))
12121247
continue;
1248+
if (allow_ref_in_want && parse_want_ref(arg, &data->wanted_refs))
1249+
continue;
12131250
/* process have line */
12141251
if (parse_have(arg, &data->haves))
12151252
continue;
@@ -1352,6 +1389,24 @@ static int process_haves_and_send_acks(struct upload_pack_data *data)
13521389
return ret;
13531390
}
13541391

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+
13551410
static void send_shallow_info(struct upload_pack_data *data)
13561411
{
13571412
/* No shallow info needs to be sent */
@@ -1418,6 +1473,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
14181473
state = FETCH_DONE;
14191474
break;
14201475
case FETCH_SEND_PACK:
1476+
send_wanted_ref_info(&data);
14211477
send_shallow_info(&data);
14221478

14231479
packet_write_fmt(1, "packfile\n");
@@ -1438,12 +1494,22 @@ int upload_pack_advertise(struct repository *r,
14381494
{
14391495
if (value) {
14401496
int allow_filter_value;
1497+
int allow_ref_in_want;
1498+
14411499
strbuf_addstr(value, "shallow");
1500+
14421501
if (!repo_config_get_bool(the_repository,
14431502
"uploadpack.allowfilter",
14441503
&allow_filter_value) &&
14451504
allow_filter_value)
14461505
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");
14471512
}
1513+
14481514
return 1;
14491515
}

0 commit comments

Comments
 (0)