@matheusbsbs / matheus-bagliano / commits / 8cd4249c4c

interpret_branch_name: always respect "namelen" parameter

interpret_branch_name gets passed a "name" buffer to parse, along with a "namelen" parameter representing its length. If "namelen" is zero, we fallback to the NUL-terminated string-length of "name". However, it does not necessarily follow that if we have gotten a non-zero "namelen", it is the NUL-terminated string-length of "name". E.g., when get_sha1() is parsing "foo:bar", we will be asked to operate only on the first three characters. Yet in interpret_branch_name and its helpers, we use string functions like strchr() to operate on "name", looking past the length we were given. This can result in us mis-parsing object names. We should instead be limiting our search to "namelen" bytes. There are three distinct types of object names this patch addresses: - The intrepret_empty_at helper uses strchr to find the next @-expression after our potential empty-at. In an expression like "@:foo@bar", it erroneously thinks that the second "@" is relevant, even if we were asked only to look at the first character. This case is easy to trigger (and we test it in this patch). - When finding the initial @-mark for @{upstream}, we use strchr. This means we might treat "foo:@{upstream}" as the upstream for "foo:", even though we were asked only to look at "foo". We cannot test this one in practice, because it is masked by another bug (which is fixed in the next patch). - The interpret_nth_prior_checkout helper did not receive the name length at all. This turns out not to be a problem in practice, though, because its parsing is so limited: it always starts from the far-left of the string, and will not tolerate a colon (which is currently the only way to get a smaller-than-strlen "namelen"). However, it's still worth fixing to make the code more obviously correct, and to future-proof us against callers with more exotic buffers. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 15, 2014 at 03:31 UTC 8cd4249c4cdb19fce489a3f6ba31f499f4331341

2 files changed +24 -8

sha1_name.c

+10 -7

@@ -430,7 +430,7 @@ static inline int upstream_mark(const char *string, int len)

430 }

431

432 static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);

433 -static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);

433 +static int interpret_nth_prior_checkout(const char *name, int namelen, struct strbuf *buf);

434

435 static int get_sha1_basic(const char *str, int len, unsigned char *sha1)

436 {

@@ -492,7 +492,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)

492 struct strbuf buf = STRBUF_INIT;

493 int detached;

494

495 - if (interpret_nth_prior_checkout(str, &buf) > 0) {

495 + if (interpret_nth_prior_checkout(str, len, &buf) > 0) {

496 detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));

497 strbuf_release(&buf);

498 if (detached)

@@ -931,7 +931,8 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,

931 * Parse @{-N} syntax, return the number of characters parsed

932 * if successful; otherwise signal an error with negative value.

933 */

934 -static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)

934 +static int interpret_nth_prior_checkout(const char *name, int namelen,

935 + struct strbuf *buf)

936 {

937 long nth;

938 int retval;

@@ -939,9 +940,11 @@ static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)

940 const char *brace;

941 char *num_end;

942

943 + if (namelen < 4)

944 + return -1;

945 if (name[0] != '@' || name[1] != '{' || name[2] != '-')

946 return -1;

944 - brace = strchr(name, '}');

947 + brace = memchr(name, '}', namelen);

948 if (!brace)

949 return -1;

950 nth = strtol(name + 3, &num_end, 10);

@@ -1014,7 +1017,7 @@ static int interpret_empty_at(const char *name, int namelen, int len, struct str

1017 return -1;

1018

1019 /* make sure it's a single @, or @@{.*}, not @foo */

1017 - next = strchr(name + len + 1, '@');

1020 + next = memchr(name + len + 1, '@', namelen - len - 1);

1021 if (next && next[1] != '{')

1022 return -1;

1023 if (!next)

@@ -1120,7 +1123,7 @@ static int interpret_upstream_mark(const char *name, int namelen,

1123 int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)

1124 {

1125 char *at;

1123 - int len = interpret_nth_prior_checkout(name, buf);

1126 + int len = interpret_nth_prior_checkout(name, namelen, buf);

1127

1128 if (!namelen)

1129 namelen = strlen(name);

@@ -1134,7 +1137,7 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)

1137 return reinterpret(name, namelen, len, buf);

1138 }

1139

1137 - at = strchr(name, '@');

1140 + at = memchr(name, '@', namelen);

1141 if (!at)

1142 return -1;

1143

t/t1508-at-combinations.sh

+14 -1

@@ -9,8 +9,11 @@ check() {

9 if test '$2' = 'commit'

10 then

11 git log -1 --format=%s '$1' >actual

12 - else

12 + elif test '$2' = 'ref'

13 + then

14 git rev-parse --symbolic-full-name '$1' >actual

15 + else

16 + git cat-file -p '$1' >actual

17 fi &&

18 test_cmp expect actual

19 "

@@ -82,4 +85,14 @@ check HEAD ref refs/heads/old-branch

85 check "HEAD@{1}" commit new-two

86 check "@{1}" commit old-one

88 +test_expect_success 'create path with @' '

89 + echo content >normal &&

90 + echo content >fun@ny &&

91 + git add normal fun@ny &&

92 + git commit -m "funny path"

93 +'

94 +

95 +check "@:normal" blob content

96 +check "@:fun@ny" blob content

97 +

98 test_done