@matheusbsbs / matheus-bagliano / commits / a625b092cc

branch: correctly reject refs/heads/{-dash,HEAD}

strbuf_check_branch_ref() is the central place where many codepaths see if a proposed name is suitable for the name of a branch. It was designed to allow us to get stricter than the check_refname_format() check used for refnames in general, and we already use it to reject a branch whose name begins with a '-'. The function gets a strbuf and a string "name", and returns non-zero if the name is not appropriate as the name for a branch. When the name is good, it places the full refname for the branch with the proposed name in the strbuf before it returns. However, it turns out that one caller looks at what is in the strbuf even when the function returns an error. Make the function populate the strbuf even when it returns an error. That way, when "-dash" is given as name, "refs/heads/-dash" is placed in the strbuf when returning an error to copy_or_rename_branch(), which notices that the user is trying to recover with "git branch -m -- -dash dash" to rename "-dash" to "dash". While at it, use the same mechanism to also reject "HEAD" as a branch name. Helped-by: Jeff King <peff@peff.net> Helped-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Nov 14, 2017 at 17:12 UTC a625b092cc59940521789fe8a3ff69c8d6b14eb2
2 files changed +55 -2
sha1_name.c
+12 -2
@@ -1332,9 +1332,19 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
1332 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
1333 {
1334 strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
1335 - if (name[0] == '-')
1336 - return -1;
1335 +
1336 + /*
1337 + * This splice must be done even if we end up rejecting the
1338 + * name; builtin/branch.c::copy_or_rename_branch() still wants
1339 + * to see what the name expanded to so that "branch -m" can be
1340 + * used as a tool to correct earlier mistakes.
1341 + */
1342 strbuf_splice(sb, 0, 0, "refs/heads/", 11);
1343 +
1344 + if (*name == '-' ||
1345 + !strcmp(sb->buf, "refs/heads/HEAD"))
1346 + return -1;
1347 +
1348 return check_refname_format(sb->buf, 0);
1349 }
1350
t/t1430-bad-ref-name.sh
+43
@@ -331,4 +331,47 @@ test_expect_success 'update-ref --stdin -z fails delete with bad ref name' '
331 grep "fatal: invalid ref format: ~a" err
332 '
333
334 +test_expect_success 'branch rejects HEAD as a branch name' '
335 + test_must_fail git branch HEAD HEAD^ &&
336 + test_must_fail git show-ref refs/heads/HEAD
337 +'
338 +
339 +test_expect_success 'checkout -b rejects HEAD as a branch name' '
340 + test_must_fail git checkout -B HEAD HEAD^ &&
341 + test_must_fail git show-ref refs/heads/HEAD
342 +'
343 +
344 +test_expect_success 'update-ref can operate on refs/heads/HEAD' '
345 + git update-ref refs/heads/HEAD HEAD^ &&
346 + git show-ref refs/heads/HEAD &&
347 + git update-ref -d refs/heads/HEAD &&
348 + test_must_fail git show-ref refs/heads/HEAD
349 +'
350 +
351 +test_expect_success 'branch -d can remove refs/heads/HEAD' '
352 + git update-ref refs/heads/HEAD HEAD^ &&
353 + git branch -d HEAD &&
354 + test_must_fail git show-ref refs/heads/HEAD
355 +'
356 +
357 +test_expect_success 'branch -m can rename refs/heads/HEAD' '
358 + git update-ref refs/heads/HEAD HEAD^ &&
359 + git branch -m HEAD tail &&
360 + test_must_fail git show-ref refs/heads/HEAD &&
361 + git show-ref refs/heads/tail
362 +'
363 +
364 +test_expect_success 'branch -d can remove refs/heads/-dash' '
365 + git update-ref refs/heads/-dash HEAD^ &&
366 + git branch -d -- -dash &&
367 + test_must_fail git show-ref refs/heads/-dash
368 +'
369 +
370 +test_expect_success 'branch -m can rename refs/heads/-dash' '
371 + git update-ref refs/heads/-dash HEAD^ &&
372 + git branch -m -- -dash dash &&
373 + test_must_fail git show-ref refs/heads/-dash &&
374 + git show-ref refs/heads/dash
375 +'
376 +
377 test_done