resolve_ref: use strbufs for internal buffers
resolve_ref already uses a strbuf internally when generating pathnames, but it uses fixed-size buffers for storing the refname and symbolic refs. This means that you cannot actually point HEAD to a ref that is larger than 256 bytes. We can lift this limit by using strbufs here, too. Like sb_path, we pass the the buffers into our helper function, so that we can easily clean up all output paths. We can also drop the "unsafe" name from our helper function, as it no longer uses a single static buffer (but of course resolve_ref_unsafe is still unsafe, because the static buffers moved there). As a bonus, we also get to drop some strcpy calls between the two fixed buffers (that cannot currently overflow because the two buffers are sized identically). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Sep 24, 2015 at 17:07 UTC
495127dbcbd53e89d7edee8db42bfa7e57c8a120
2 files changed
+59
-27
refs.c
+30
-27
@@ -1579,16 +1579,15 @@ static int resolve_missing_loose_ref(const char *refname,
1579
}
1580
1581
/* This function needs to return a meaningful errno on failure */
1582
-static const char *resolve_ref_unsafe_1(const char *refname,
1583
- int resolve_flags,
1584
- unsigned char *sha1,
1585
- int *flags,
1586
- struct strbuf *sb_path)
1582
+static const char *resolve_ref_1(const char *refname,
1583
+ int resolve_flags,
1584
+ unsigned char *sha1,
1585
+ int *flags,
1586
+ struct strbuf *sb_refname,
1587
+ struct strbuf *sb_path,
1588
+ struct strbuf *sb_contents)
1589
{
1590
int depth = MAXDEPTH;
1589
- ssize_t len;
1590
- char buffer[256];
1591
- static char refname_buffer[256];
1591
int bad_name = 0;
1592
1593
if (flags)
@@ -1654,19 +1653,18 @@ static const char *resolve_ref_unsafe_1(const char *refname,
1653
1654
/* Follow "normalized" - ie "refs/.." symlinks by hand */
1655
if (S_ISLNK(st.st_mode)) {
1657
- len = readlink(path, buffer, sizeof(buffer)-1);
1658
- if (len < 0) {
1656
+ strbuf_reset(sb_contents);
1657
+ if (strbuf_readlink(sb_contents, path, 0) < 0) {
1658
if (errno == ENOENT || errno == EINVAL)
1659
/* inconsistent with lstat; retry */
1660
goto stat_ref;
1661
else
1662
return NULL;
1663
}
1665
- buffer[len] = 0;
1666
- if (starts_with(buffer, "refs/") &&
1667
- !check_refname_format(buffer, 0)) {
1668
- strcpy(refname_buffer, buffer);
1669
- refname = refname_buffer;
1664
+ if (starts_with(sb_contents->buf, "refs/") &&
1665
+ !check_refname_format(sb_contents->buf, 0)) {
1666
+ strbuf_swap(sb_refname, sb_contents);
1667
+ refname = sb_refname->buf;
1668
if (flags)
1669
*flags |= REF_ISSYMREF;
1670
if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
@@ -1695,28 +1693,26 @@ static const char *resolve_ref_unsafe_1(const char *refname,
1693
else
1694
return NULL;
1695
}
1698
- len = read_in_full(fd, buffer, sizeof(buffer)-1);
1699
- if (len < 0) {
1696
+ strbuf_reset(sb_contents);
1697
+ if (strbuf_read(sb_contents, fd, 256) < 0) {
1698
int save_errno = errno;
1699
close(fd);
1700
errno = save_errno;
1701
return NULL;
1702
}
1703
close(fd);
1706
- while (len && isspace(buffer[len-1]))
1707
- len--;
1708
- buffer[len] = '\0';
1704
+ strbuf_rtrim(sb_contents);
1705
1706
/*
1707
* Is it a symbolic ref?
1708
*/
1713
- if (!starts_with(buffer, "ref:")) {
1709
+ if (!starts_with(sb_contents->buf, "ref:")) {
1710
/*
1711
* Please note that FETCH_HEAD has a second
1712
* line containing other data.
1713
*/
1718
- if (get_sha1_hex(buffer, sha1) ||
1719
- (buffer[40] != '\0' && !isspace(buffer[40]))) {
1714
+ if (get_sha1_hex(sb_contents->buf, sha1) ||
1715
+ (sb_contents->buf[40] != '\0' && !isspace(sb_contents->buf[40]))) {
1716
if (flags)
1717
*flags |= REF_ISBROKEN;
1718
errno = EINVAL;
@@ -1731,10 +1727,12 @@ static const char *resolve_ref_unsafe_1(const char *refname,
1727
}
1728
if (flags)
1729
*flags |= REF_ISSYMREF;
1734
- buf = buffer + 4;
1730
+ buf = sb_contents->buf + 4;
1731
while (isspace(*buf))
1732
buf++;
1737
- refname = strcpy(refname_buffer, buf);
1733
+ strbuf_reset(sb_refname);
1734
+ strbuf_addstr(sb_refname, buf);
1735
+ refname = sb_refname->buf;
1736
if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
1737
hashclr(sha1);
1738
return refname;
@@ -1756,10 +1754,15 @@ static const char *resolve_ref_unsafe_1(const char *refname,
1754
const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
1755
unsigned char *sha1, int *flags)
1756
{
1757
+ static struct strbuf sb_refname = STRBUF_INIT;
1758
+ struct strbuf sb_contents = STRBUF_INIT;
1759
struct strbuf sb_path = STRBUF_INIT;
1760
- const char *ret = resolve_ref_unsafe_1(refname, resolve_flags,
1761
- sha1, flags, &sb_path);
1760
+ const char *ret;
1761
+
1762
+ ret = resolve_ref_1(refname, resolve_flags, sha1, flags,
1763
+ &sb_refname, &sb_path, &sb_contents);
1764
strbuf_release(&sb_path);
1765
+ strbuf_release(&sb_contents);
1766
return ret;
1767
}
1768
t/t1401-symbolic-ref.sh
+29
@@ -63,4 +63,33 @@ test_expect_success 'symbolic-ref fails to delete real ref' '
63
'
64
reset_to_sane
65
66
+test_expect_success 'create large ref name' '
67
+ # make 256+ character ref; some systems may not handle that,
68
+ # so be gentle
69
+ long=0123456789abcdef &&
70
+ long=$long/$long/$long/$long &&
71
+ long=$long/$long/$long/$long &&
72
+ long_ref=refs/heads/$long &&
73
+ tree=$(git write-tree) &&
74
+ commit=$(echo foo | git commit-tree $tree) &&
75
+ if git update-ref $long_ref $commit; then
76
+ test_set_prereq LONG_REF
77
+ else
78
+ echo >&2 "long refs not supported"
79
+ fi
80
+'
81
+
82
+test_expect_success LONG_REF 'symbolic-ref can point to large ref name' '
83
+ git symbolic-ref HEAD $long_ref &&
84
+ echo $long_ref >expect &&
85
+ git symbolic-ref HEAD >actual &&
86
+ test_cmp expect actual
87
+'
88
+
89
+test_expect_success LONG_REF 'we can parse long symbolic ref' '
90
+ echo $commit >expect &&
91
+ git rev-parse --verify HEAD >actual &&
92
+ test_cmp expect actual
93
+'
94
+
95
test_done