Skip to content

Commit 6677c46

Browse files
author
Junio C Hamano
committed
get_sha1_basic(): corner case ambiguity fix
When .git/refs/heads/frotz and .git/refs/tags/frotz existed, and the object name stored in .git/refs/heads/frotz were corrupt, we ended up picking tags/frotz without complaining. Worse yet, if the corrupt .git/refs/heads/frotz was more than 40 bytes and began with hexadecimal characters, it silently overwritten the initial part of the returned result. This commit adds a couple of tests to demonstrate these cases, with a fix. Signed-off-by: Junio C Hamano <junkio@cox.net>
1 parent 8431c4e commit 6677c46

File tree

3 files changed

+75
-9
lines changed

3 files changed

+75
-9
lines changed

‎sha1_name.c‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,12 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
203203
return NULL;
204204
}
205205

206-
static int ambiguous_path(const char *path)
206+
static int ambiguous_path(const char *path, int len)
207207
{
208208
int slash = 1;
209+
int cnt;
209210

210-
for (;;) {
211+
for (cnt = 0; cnt < len; cnt++) {
211212
switch (*path++) {
212213
case '\0':
213214
break;
@@ -224,6 +225,7 @@ static int ambiguous_path(const char *path)
224225
}
225226
return slash;
226227
}
228+
return slash;
227229
}
228230

229231
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
@@ -242,26 +244,41 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
242244
return 0;
243245

244246
/* Accept only unambiguous ref paths. */
245-
if (ambiguous_path(str))
247+
if (ambiguous_path(str, len))
246248
return -1;
247249

248250
for (p = prefix; *p; p++) {
249251
char *pathname = git_path("%s/%.*s", *p, len, str);
252+
250253
if (!read_ref(pathname, sha1)) {
251254
/* Must be unique; i.e. when heads/foo and
252255
* tags/foo are both present, reject "foo".
253-
* Note that read_ref() eventually calls
254-
* get_sha1_hex() which can smudge initial
255-
* part of the buffer even if what is read
256-
* is found to be invalid halfway.
257256
*/
258257
if (1 < found++)
259258
return -1;
260259
}
260+
261+
/* We want to allow .git/description file and
262+
* "description" branch to exist at the same time.
263+
* "git-rev-parse description" should silently skip
264+
* .git/description file as a candidate for
265+
* get_sha1(). However, having garbage file anywhere
266+
* under refs/ is not OK, and we would not have caught
267+
* ambiguous heads and tags with the above test.
268+
*/
269+
else if (**p && !access(pathname, F_OK)) {
270+
/* Garbage exists under .git/refs */
271+
return error("garbage ref found '%s'", pathname);
272+
}
261273
}
262-
if (found == 1)
274+
switch (found) {
275+
case 0:
276+
return -1;
277+
case 1:
263278
return 0;
264-
return -1;
279+
default:
280+
return error("ambiguous refname '%.*s'", len, str);
281+
}
265282
}
266283

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

‎t/t0000-basic.sh‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,4 +205,52 @@ test_expect_success \
205205
'no diff after checkout and git-update-index --refresh.' \
206206
'git-diff-files >current && cmp -s current /dev/null'
207207

208+
209+
# extended sha1 parsing and ambiguity resolution
210+
211+
GIT_AUTHOR_DATE='1995-01-29T16:00:00 -0800'
212+
GIT_AUTHOR_EMAIL=a.u.thor@example.com
213+
GIT_AUTHOR_NAME='A U Thor'
214+
GIT_COMMITTER_DATE='1995-01-29T16:00:00 -0800'
215+
GIT_COMMITTER_EMAIL=c.o.mmitter@example.com
216+
GIT_COMMITTER_NAME='C O Mmitter'
217+
export GIT_AUTHOR_DATE
218+
export GIT_AUTHOR_EMAIL
219+
export GIT_AUTHOR_NAME
220+
export GIT_COMMITTER_DATE
221+
export GIT_COMMITTER_EMAIL
222+
export GIT_COMMITTER_NAME
223+
224+
test_expect_success \
225+
'initial commit.' \
226+
'commit=$(echo Initial commit | git-commit-tree $tree) &&
227+
echo "$commit" >.git/refs/heads/master &&
228+
git-ls-tree HEAD &&
229+
test "$commit" = 51a092e9ef6cbbe66d258acd17599d3f80be6162'
230+
231+
test_expect_success \
232+
'Ambiguous' \
233+
'echo "$commit" >.git/refs/heads/nasty &&
234+
echo "$commit" >.git/refs/tags/nasty &&
235+
if git-rev-parse --verify nasty
236+
then
237+
echo "should have barfed"
238+
false
239+
else
240+
:
241+
fi &&
242+
# names directly underneath .git/ should not interfere
243+
echo "$commit" >.git/refs/heads/description &&
244+
git-rev-parse --verify description &&
245+
# broken object name
246+
echo fffffffffffffffffffffffffffffffffffffffg \
247+
>.git/refs/heads/nasty &&
248+
if git-rev-parse --verify nasty
249+
then
250+
echo "should have barfed"
251+
false
252+
else
253+
:
254+
fi'
255+
208256
test_done

‎t/test-lib.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ unset GIT_ALTERNATE_OBJECT_DIRECTORIES
1818
unset GIT_AUTHOR_DATE
1919
unset GIT_AUTHOR_EMAIL
2020
unset GIT_AUTHOR_NAME
21+
unset GIT_COMMITTER_DATE
2122
unset GIT_COMMITTER_EMAIL
2223
unset GIT_COMMITTER_NAME
2324
unset GIT_DIFF_OPTS

0 commit comments

Comments
 (0)