Skip to content

Commit c05790a

Browse files
committed
smart_pkt: return parsed length via out-parameter
The `parse_len` function currently directly returns the parsed length of a packet line or an error code in case there was an error. Instead, convert this to our usual style of using the return value as error code only and returning the actual value via an out-parameter. Thus, we can now convert the output parameter to an unsigned type, as the size of a packet cannot ever be negative. While at it, we also move the check whether the input buffer is long enough into `parse_len` itself. We don't really want to pass around potentially non-NUL-terminated buffers to functions without also passing along the length, as this is dangerous in the unlikely case where other callers for that function get added. Note that we need to make sure though to not mess with `GIT_EBUFS` error codes, as these indicate not an error to the caller but that he needs to fetch more data.
1 parent 0b3dfbf commit c05790a

File tree

1 file changed

+34
-29
lines changed

1 file changed

+34
-29
lines changed

src/transports/smart_pkt.c

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -363,13 +363,17 @@ static int unpack_pkt(git_pkt **out, const char *line, size_t len)
363363
return 0;
364364
}
365365

366-
static int32_t parse_len(const char *line)
366+
static int parse_len(size_t *out, const char *line, size_t linelen)
367367
{
368368
char num[PKT_LEN_SIZE + 1];
369369
int i, k, error;
370370
int32_t len;
371371
const char *num_end;
372372

373+
/* Not even enough for the length */
374+
if (linelen < PKT_LEN_SIZE)
375+
return GIT_EBUFS;
376+
373377
memcpy(num, line, PKT_LEN_SIZE);
374378
num[PKT_LEN_SIZE] = '\0';
375379

@@ -390,7 +394,11 @@ static int32_t parse_len(const char *line)
390394
if ((error = git__strtol32(&len, num, &num_end, 16)) < 0)
391395
return error;
392396

393-
return len;
397+
if (len < 0)
398+
return -1;
399+
400+
*out = (size_t) len;
401+
return 0;
394402
}
395403

396404
/*
@@ -409,26 +417,23 @@ static int32_t parse_len(const char *line)
409417
int git_pkt_parse_line(
410418
git_pkt **pkt, const char **endptr, const char *line, size_t linelen)
411419
{
412-
int ret;
413-
int32_t len;
414-
415-
/* Not even enough for the length */
416-
if (linelen > 0 && linelen < PKT_LEN_SIZE)
417-
return GIT_EBUFS;
420+
int error;
421+
size_t len;
418422

419-
len = parse_len(line);
420-
if (len < 0) {
423+
if ((error = parse_len(&len, line, linelen)) < 0) {
421424
/*
422-
* If we fail to parse the length, it might be because the
423-
* server is trying to send us the packfile already.
425+
* If we fail to parse the length, it might be
426+
* because the server is trying to send us the
427+
* packfile already or because we do not yet have
428+
* enough data.
424429
*/
425-
if (linelen >= 4 && !git__prefixcmp(line, "PACK")) {
430+
if (error == GIT_EBUFS)
431+
;
432+
else if (!git__prefixncmp(line, linelen, "PACK"))
426433
giterr_set(GITERR_NET, "unexpected pack file");
427-
} else {
434+
else
428435
giterr_set(GITERR_NET, "bad packet length");
429-
}
430-
431-
return -1;
436+
return error;
432437
}
433438

434439
/*
@@ -465,31 +470,31 @@ int git_pkt_parse_line(
465470
len -= PKT_LEN_SIZE; /* the encoded length includes its own size */
466471

467472
if (*line == GIT_SIDE_BAND_DATA)
468-
ret = data_pkt(pkt, line, len);
473+
error = data_pkt(pkt, line, len);
469474
else if (*line == GIT_SIDE_BAND_PROGRESS)
470-
ret = sideband_progress_pkt(pkt, line, len);
475+
error = sideband_progress_pkt(pkt, line, len);
471476
else if (*line == GIT_SIDE_BAND_ERROR)
472-
ret = sideband_error_pkt(pkt, line, len);
477+
error = sideband_error_pkt(pkt, line, len);
473478
else if (!git__prefixncmp(line, len, "ACK"))
474-
ret = ack_pkt(pkt, line, len);
479+
error = ack_pkt(pkt, line, len);
475480
else if (!git__prefixncmp(line, len, "NAK"))
476-
ret = nak_pkt(pkt);
481+
error = nak_pkt(pkt);
477482
else if (!git__prefixncmp(line, len, "ERR"))
478-
ret = err_pkt(pkt, line, len);
483+
error = err_pkt(pkt, line, len);
479484
else if (*line == '#')
480-
ret = comment_pkt(pkt, line, len);
485+
error = comment_pkt(pkt, line, len);
481486
else if (!git__prefixncmp(line, len, "ok"))
482-
ret = ok_pkt(pkt, line, len);
487+
error = ok_pkt(pkt, line, len);
483488
else if (!git__prefixncmp(line, len, "ng"))
484-
ret = ng_pkt(pkt, line, len);
489+
error = ng_pkt(pkt, line, len);
485490
else if (!git__prefixncmp(line, len, "unpack"))
486-
ret = unpack_pkt(pkt, line, len);
491+
error = unpack_pkt(pkt, line, len);
487492
else
488-
ret = ref_pkt(pkt, line, len);
493+
error = ref_pkt(pkt, line, len);
489494

490495
*endptr = line + len;
491496

492-
return ret;
497+
return error;
493498
}
494499

495500
void git_pkt_free(git_pkt *pkt)

0 commit comments

Comments
 (0)