Skip to content

Commit 17069c7

Browse files
gitstergitster
committed
Merge branch 'ms/packet-err-check' into jt/fetch-v2-sideband
* ms/packet-err-check: pack-protocol.txt: accept error packets in any context Use packet_reader instead of packet_read_line
2 parents ecbdaf0 + 2d103c3 commit 17069c7

File tree

15 files changed

+174
-133
lines changed

15 files changed

+174
-133
lines changed

‎Documentation/technical/pack-protocol.txt‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,16 @@ protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
2222
otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
2323
include a LF, but the receiver MUST NOT complain if it is not present.
2424

25+
An error packet is a special pkt-line that contains an error string.
26+
27+
----
28+
error-line = PKT-LINE("ERR" SP explanation-text)
29+
----
30+
31+
Throughout the protocol, where `PKT-LINE(...)` is expected, an error packet MAY
32+
be sent. Once this packet is sent by a client or a server, the data transfer
33+
process defined in this protocol is terminated.
34+
2535
Transports
2636
----------
2737
There are three transports over which the packfile protocol is
@@ -89,13 +99,6 @@ process on the server side over the Git protocol is this:
8999
"0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
90100
nc -v example.com 9418
91101

92-
If the server refuses the request for some reasons, it could abort
93-
gracefully with an error message.
94-
95-
----
96-
error-line = PKT-LINE("ERR" SP explanation-text)
97-
----
98-
99102

100103
SSH Transport
101104
-------------
@@ -398,12 +401,11 @@ from the client).
398401
Then the server will start sending its packfile data.
399402

400403
----
401-
server-response = *ack_multi ack / nak / error-line
404+
server-response = *ack_multi ack / nak
402405
ack_multi = PKT-LINE("ACK" SP obj-id ack_status)
403406
ack_status = "continue" / "common" / "ready"
404407
ack = PKT-LINE("ACK" SP obj-id)
405408
nak = PKT-LINE("NAK")
406-
error-line = PKT-LINE("ERR" SP explanation-text)
407409
----
408410

409411
A simple clone may look like this (with no 'have' lines):

‎builtin/archive.c‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ static int run_remote_archiver(int argc, const char **argv,
2727
const char *remote, const char *exec,
2828
const char *name_hint)
2929
{
30-
char *buf;
3130
int fd[2], i, rv;
3231
struct transport *transport;
3332
struct remote *_remote;
33+
struct packet_reader reader;
3434

3535
_remote = remote_get(remote);
3636
if (!_remote->url[0])
@@ -53,18 +53,19 @@ static int run_remote_archiver(int argc, const char **argv,
5353
packet_write_fmt(fd[1], "argument %s\n", argv[i]);
5454
packet_flush(fd[1]);
5555

56-
buf = packet_read_line(fd[0], NULL);
57-
if (!buf)
56+
packet_reader_init(&reader, fd[0], NULL, 0,
57+
PACKET_READ_CHOMP_NEWLINE |
58+
PACKET_READ_DIE_ON_ERR_PACKET);
59+
60+
if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
5861
die(_("git archive: expected ACK/NAK, got a flush packet"));
59-
if (strcmp(buf, "ACK")) {
60-
if (starts_with(buf, "NACK "))
61-
die(_("git archive: NACK %s"), buf + 5);
62-
if (starts_with(buf, "ERR "))
63-
die(_("remote error: %s"), buf + 4);
62+
if (strcmp(reader.line, "ACK")) {
63+
if (starts_with(reader.line, "NACK "))
64+
die(_("git archive: NACK %s"), reader.line + 5);
6465
die(_("git archive: protocol error"));
6566
}
6667

67-
if (packet_read_line(fd[0], NULL))
68+
if (packet_reader_read(&reader) != PACKET_READ_FLUSH)
6869
die(_("git archive: expected a flush"));
6970

7071
/* Now, start reading from fd[0] and spit it out to stdout */

‎builtin/fetch-pack.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
217217

218218
packet_reader_init(&reader, fd[0], NULL, 0,
219219
PACKET_READ_CHOMP_NEWLINE |
220-
PACKET_READ_GENTLE_ON_EOF);
220+
PACKET_READ_GENTLE_ON_EOF |
221+
PACKET_READ_DIE_ON_ERR_PACKET);
221222

222223
switch (discover_version(&reader)) {
223224
case protocol_v2:

‎builtin/receive-pack.c‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1569,30 +1569,29 @@ static void queue_commands_from_cert(struct command **tail,
15691569
}
15701570
}
15711571

1572-
static struct command *read_head_info(struct oid_array *shallow)
1572+
static struct command *read_head_info(struct packet_reader *reader,
1573+
struct oid_array *shallow)
15731574
{
15741575
struct command *commands = NULL;
15751576
struct command **p = &commands;
15761577
for (;;) {
1577-
char *line;
1578-
int len, linelen;
1578+
int linelen;
15791579

1580-
line = packet_read_line(0, &len);
1581-
if (!line)
1580+
if (packet_reader_read(reader) != PACKET_READ_NORMAL)
15821581
break;
15831582

1584-
if (len > 8 && starts_with(line, "shallow ")) {
1583+
if (reader->pktlen > 8 && starts_with(reader->line, "shallow ")) {
15851584
struct object_id oid;
1586-
if (get_oid_hex(line + 8, &oid))
1585+
if (get_oid_hex(reader->line + 8, &oid))
15871586
die("protocol error: expected shallow sha, got '%s'",
1588-
line + 8);
1587+
reader->line + 8);
15891588
oid_array_append(shallow, &oid);
15901589
continue;
15911590
}
15921591

1593-
linelen = strlen(line);
1594-
if (linelen < len) {
1595-
const char *feature_list = line + linelen + 1;
1592+
linelen = strlen(reader->line);
1593+
if (linelen < reader->pktlen) {
1594+
const char *feature_list = reader->line + linelen + 1;
15961595
if (parse_feature_request(feature_list, "report-status"))
15971596
report_status = 1;
15981597
if (parse_feature_request(feature_list, "side-band-64k"))
@@ -1607,28 +1606,32 @@ static struct command *read_head_info(struct oid_array *shallow)
16071606
use_push_options = 1;
16081607
}
16091608

1610-
if (!strcmp(line, "push-cert")) {
1609+
if (!strcmp(reader->line, "push-cert")) {
16111610
int true_flush = 0;
1612-
char certbuf[1024];
1611+
int saved_options = reader->options;
1612+
reader->options &= ~PACKET_READ_CHOMP_NEWLINE;
16131613

16141614
for (;;) {
1615-
len = packet_read(0, NULL, NULL,
1616-
certbuf, sizeof(certbuf), 0);
1617-
if (!len) {
1615+
packet_reader_read(reader);
1616+
if (reader->status == PACKET_READ_FLUSH) {
16181617
true_flush = 1;
16191618
break;
16201619
}
1621-
if (!strcmp(certbuf, "push-cert-end\n"))
1620+
if (reader->status != PACKET_READ_NORMAL) {
1621+
die("protocol error: got an unexpected packet");
1622+
}
1623+
if (!strcmp(reader->line, "push-cert-end\n"))
16221624
break; /* end of cert */
1623-
strbuf_addstr(&push_cert, certbuf);
1625+
strbuf_addstr(&push_cert, reader->line);
16241626
}
1627+
reader->options = saved_options;
16251628

16261629
if (true_flush)
16271630
break;
16281631
continue;
16291632
}
16301633

1631-
p = queue_command(p, line, linelen);
1634+
p = queue_command(p, reader->line, linelen);
16321635
}
16331636

16341637
if (push_cert.len)
@@ -1637,18 +1640,14 @@ static struct command *read_head_info(struct oid_array *shallow)
16371640
return commands;
16381641
}
16391642

1640-
static void read_push_options(struct string_list *options)
1643+
static void read_push_options(struct packet_reader *reader,
1644+
struct string_list *options)
16411645
{
16421646
while (1) {
1643-
char *line;
1644-
int len;
1645-
1646-
line = packet_read_line(0, &len);
1647-
1648-
if (!line)
1647+
if (packet_reader_read(reader) != PACKET_READ_NORMAL)
16491648
break;
16501649

1651-
string_list_append(options, line);
1650+
string_list_append(options, reader->line);
16521651
}
16531652
}
16541653

@@ -1924,6 +1923,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
19241923
struct oid_array shallow = OID_ARRAY_INIT;
19251924
struct oid_array ref = OID_ARRAY_INIT;
19261925
struct shallow_info si;
1926+
struct packet_reader reader;
19271927

19281928
struct option options[] = {
19291929
OPT__QUIET(&quiet, N_("quiet")),
@@ -1986,12 +1986,16 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
19861986
if (advertise_refs)
19871987
return 0;
19881988

1989-
if ((commands = read_head_info(&shallow)) != NULL) {
1989+
packet_reader_init(&reader, 0, NULL, 0,
1990+
PACKET_READ_CHOMP_NEWLINE |
1991+
PACKET_READ_DIE_ON_ERR_PACKET);
1992+
1993+
if ((commands = read_head_info(&reader, &shallow)) != NULL) {
19901994
const char *unpack_status = NULL;
19911995
struct string_list push_options = STRING_LIST_INIT_DUP;
19921996

19931997
if (use_push_options)
1994-
read_push_options(&push_options);
1998+
read_push_options(&reader, &push_options);
19951999
if (!check_cert_push_options(&push_options)) {
19962000
struct command *cmd;
19972001
for (cmd = commands; cmd; cmd = cmd->next)

‎builtin/send-pack.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
250250

251251
packet_reader_init(&reader, fd[0], NULL, 0,
252252
PACKET_READ_CHOMP_NEWLINE |
253-
PACKET_READ_GENTLE_ON_EOF);
253+
PACKET_READ_GENTLE_ON_EOF |
254+
PACKET_READ_DIE_ON_ERR_PACKET);
254255

255256
switch (discover_version(&reader)) {
256257
case protocol_v2:

‎connect.c‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,6 @@ struct ref **get_remote_heads(struct packet_reader *reader,
296296
struct ref **orig_list = list;
297297
int len = 0;
298298
enum get_remote_heads_state state = EXPECTING_FIRST_REF;
299-
const char *arg;
300299

301300
*list = NULL;
302301

@@ -306,8 +305,6 @@ struct ref **get_remote_heads(struct packet_reader *reader,
306305
die_initial_contact(1);
307306
case PACKET_READ_NORMAL:
308307
len = reader->pktlen;
309-
if (len > 4 && skip_prefix(reader->line, "ERR ", &arg))
310-
die(_("remote error: %s"), arg);
311308
break;
312309
case PACKET_READ_FLUSH:
313310
state = EXPECTING_DONE;

0 commit comments

Comments
 (0)