Skip to content

Commit 3c98008

Browse files
gitstergitster
committed
Merge branch 'jt/push-options-doc'
The receive-pack program now makes sure that the push certificate records the same set of push options used for pushing. * jt/push-options-doc: receive-pack: verify push options in cert docs: correct receive.advertisePushOptions default
2 parents e4b6ccd + cbaf82c commit 3c98008

File tree

4 files changed

+114
-11
lines changed

4 files changed

+114
-11
lines changed

‎Documentation/config.txt‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,9 +2623,8 @@ receive.advertiseAtomic::
26232623
capability, set this variable to false.
26242624

26252625
receive.advertisePushOptions::
2626-
By default, git-receive-pack will advertise the push options
2627-
capability to its clients. If you don't want to advertise this
2628-
capability, set this variable to false.
2626+
When set to true, git-receive-pack will advertise the push options
2627+
capability to its clients. False by default.
26292628

26302629
receive.autogc::
26312630
By default, git-receive-pack will run "git-gc --auto" after

‎Documentation/technical/pack-protocol.txt‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -473,13 +473,10 @@ that it wants to update, it sends a line listing the obj-id currently on
473473
the server, the obj-id the client would like to update it to and the name
474474
of the reference.
475475

476-
This list is followed by a flush-pkt. Then the push options are transmitted
477-
one per packet followed by another flush-pkt. After that the packfile that
478-
should contain all the objects that the server will need to complete the new
479-
references will be sent.
476+
This list is followed by a flush-pkt.
480477

481478
----
482-
update-request = *shallow ( command-list | push-cert ) [packfile]
479+
update-requests = *shallow ( command-list | push-cert )
483480

484481
shallow = PKT-LINE("shallow" SP obj-id)
485482

@@ -500,12 +497,35 @@ references will be sent.
500497
PKT-LINE("pusher" SP ident LF)
501498
PKT-LINE("pushee" SP url LF)
502499
PKT-LINE("nonce" SP nonce LF)
500+
*PKT-LINE("push-option" SP push-option LF)
503501
PKT-LINE(LF)
504502
*PKT-LINE(command LF)
505503
*PKT-LINE(gpg-signature-lines LF)
506504
PKT-LINE("push-cert-end" LF)
507505

508-
packfile = "PACK" 28*(OCTET)
506+
push-option = 1*( VCHAR | SP )
507+
----
508+
509+
If the server has advertised the 'push-options' capability and the client has
510+
specified 'push-options' as part of the capability list above, the client then
511+
sends its push options followed by a flush-pkt.
512+
513+
----
514+
push-options = *PKT-LINE(push-option) flush-pkt
515+
----
516+
517+
For backwards compatibility with older Git servers, if the client sends a push
518+
cert and push options, it MUST send its push options both embedded within the
519+
push cert and after the push cert. (Note that the push options within the cert
520+
are prefixed, but the push options after the cert are not.) Both these lists
521+
MUST be the same, modulo the prefix.
522+
523+
After that the packfile that
524+
should contain all the objects that the server will need to complete the new
525+
references will be sent.
526+
527+
----
528+
packfile = "PACK" 28*(OCTET)
509529
----
510530

511531
If the receiving end does not support delete-refs, the sending end MUST

‎builtin/receive-pack.c‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,8 @@ static char *prepare_push_cert_nonce(const char *path, timestamp_t stamp)
473473
* after dropping "_commit" from its name and possibly moving it out
474474
* of commit.c
475475
*/
476-
static char *find_header(const char *msg, size_t len, const char *key)
476+
static char *find_header(const char *msg, size_t len, const char *key,
477+
const char **next_line)
477478
{
478479
int key_len = strlen(key);
479480
const char *line = msg;
@@ -486,6 +487,8 @@ static char *find_header(const char *msg, size_t len, const char *key)
486487
if (line + key_len < eol &&
487488
!memcmp(line, key, key_len) && line[key_len] == ' ') {
488489
int offset = key_len + 1;
490+
if (next_line)
491+
*next_line = *eol ? eol + 1 : eol;
489492
return xmemdupz(line + offset, (eol - line) - offset);
490493
}
491494
line = *eol ? eol + 1 : NULL;
@@ -495,7 +498,7 @@ static char *find_header(const char *msg, size_t len, const char *key)
495498

496499
static const char *check_nonce(const char *buf, size_t len)
497500
{
498-
char *nonce = find_header(buf, len, "nonce");
501+
char *nonce = find_header(buf, len, "nonce", NULL);
499502
timestamp_t stamp, ostamp;
500503
char *bohmac, *expect = NULL;
501504
const char *retval = NONCE_BAD;
@@ -575,6 +578,45 @@ static const char *check_nonce(const char *buf, size_t len)
575578
return retval;
576579
}
577580

581+
/*
582+
* Return 1 if there is no push_cert or if the push options in push_cert are
583+
* the same as those in the argument; 0 otherwise.
584+
*/
585+
static int check_cert_push_options(const struct string_list *push_options)
586+
{
587+
const char *buf = push_cert.buf;
588+
int len = push_cert.len;
589+
590+
char *option;
591+
const char *next_line;
592+
int options_seen = 0;
593+
594+
int retval = 1;
595+
596+
if (!len)
597+
return 1;
598+
599+
while ((option = find_header(buf, len, "push-option", &next_line))) {
600+
len -= (next_line - buf);
601+
buf = next_line;
602+
options_seen++;
603+
if (options_seen > push_options->nr
604+
|| strcmp(option,
605+
push_options->items[options_seen - 1].string)) {
606+
retval = 0;
607+
goto leave;
608+
}
609+
free(option);
610+
}
611+
612+
if (options_seen != push_options->nr)
613+
retval = 0;
614+
615+
leave:
616+
free(option);
617+
return retval;
618+
}
619+
578620
static void prepare_push_cert_sha1(struct child_process *proc)
579621
{
580622
static int already_done;
@@ -1929,6 +1971,11 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
19291971

19301972
if (use_push_options)
19311973
read_push_options(&push_options);
1974+
if (!check_cert_push_options(&push_options)) {
1975+
struct command *cmd;
1976+
for (cmd = commands; cmd; cmd = cmd->next)
1977+
cmd->error_string = "inconsistent push options";
1978+
}
19321979

19331980
prepare_shallow_info(&si, &shallow);
19341981
if (!si.nr_ours && !si.nr_theirs)

‎t/t5534-push-signed.sh‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,43 @@ test_expect_success GPG 'signed push sends push certificate' '
124124
test_cmp expect dst/push-cert-status
125125
'
126126

127+
test_expect_success GPG 'inconsistent push options in signed push not allowed' '
128+
# First, invoke receive-pack with dummy input to obtain its preamble.
129+
prepare_dst &&
130+
git -C dst config receive.certnonceseed sekrit &&
131+
git -C dst config receive.advertisepushoptions 1 &&
132+
printf xxxx | test_might_fail git receive-pack dst >preamble &&
133+
134+
# Then, invoke push. Simulate a receive-pack that sends the preamble we
135+
# obtained, followed by a dummy packet.
136+
write_script myscript <<-\EOF &&
137+
cat preamble &&
138+
printf xxxx &&
139+
cat >push
140+
EOF
141+
test_might_fail git push --push-option="foo" --push-option="bar" \
142+
--receive-pack="\"$(pwd)/myscript\"" --signed dst --delete ff &&
143+
144+
# Replay the push output on a fresh dst, checking that ff is truly
145+
# deleted.
146+
prepare_dst &&
147+
git -C dst config receive.certnonceseed sekrit &&
148+
git -C dst config receive.advertisepushoptions 1 &&
149+
git receive-pack dst <push &&
150+
test_must_fail git -C dst rev-parse ff &&
151+
152+
# Tweak the push output to make the push option outside the cert
153+
# different, then replay it on a fresh dst, checking that ff is not
154+
# deleted.
155+
perl -pe "s/([^ ])bar/\$1baz/" push >push.tweak &&
156+
prepare_dst &&
157+
git -C dst config receive.certnonceseed sekrit &&
158+
git -C dst config receive.advertisepushoptions 1 &&
159+
git receive-pack dst <push.tweak >out &&
160+
git -C dst rev-parse ff &&
161+
grep "inconsistent push options" out
162+
'
163+
127164
test_expect_success GPG 'fail without key and heed user.signingkey' '
128165
prepare_dst &&
129166
mkdir -p dst/.git/hooks &&

0 commit comments

Comments
 (0)