Be more careful about updating refs

This makes write_ref_sha1() more careful: it actually checks the SHA1 of
the ref it is updating, and refuses to update a ref with an object that it
cannot find.

Perhaps more importantly, it also refuses to update a branch head with a
non-commit object. I don't quite know *how* the stable series maintainers
were able to corrupt their repository to have a HEAD that pointed to a tag
rather than a commit object, but they did. Which results in a totally
broken repository that cannot be cloned or committed on.

So make it harder for people to shoot themselves in the foot like that.

The test t1400-update-ref.sh is fixed at the same time, as it
assumed that the commands involved in the particular test would
not care about corrupted repositories whose refs point at
nonexistant bogus objects.  That assumption does not hold true
anymore.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs.c b/refs.c
index 58f6d173970bcd3c1e540b7cff04d1613fce3692..c3ffe0393b3c5b884422d3f9389b6c89ab6fd0df 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1119,10 +1119,16 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
        return 0;
 }
 
        return 0;
 }
 

+static int is_branch(const char *refname)
+{
+       return !strcmp(refname, "HEAD") || !prefixcmp(refname, "refs/heads/");
+}
+

 int write_ref_sha1(struct ref_lock *lock,
        const unsigned char *sha1, const char *logmsg)
 {
        static char term = '\n';
 int write_ref_sha1(struct ref_lock *lock,
        const unsigned char *sha1, const char *logmsg)
 {
        static char term = '\n';

+       struct object *o;

 
        if (!lock)
                return -1;
 
        if (!lock)
                return -1;


@@ -1130,6 +1136,19 @@ int write_ref_sha1(struct ref_lock *lock,
                unlock_ref(lock);
                return 0;
        }
                unlock_ref(lock);
                return 0;
        }

+       o = parse_object(sha1);
+       if (!o) {
+               error("Trying to write ref %s with nonexistant object %s",
+                       lock->ref_name, sha1_to_hex(sha1));
+               unlock_ref(lock);
+               return -1;
+       }
+       if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {
+               error("Trying to write non-commit object %s to branch %s",
+                       sha1_to_hex(sha1), lock->ref_name);
+               unlock_ref(lock);
+               return -1;
+       }

        if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
            write_in_full(lock->lock_fd, &term, 1) != 1
                || close(lock->lock_fd) < 0) {
        if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
            write_in_full(lock->lock_fd, &term, 1) != 1
                || close(lock->lock_fd) < 0) {

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index a90824ba8ab0e6df5c4afad528e30d118e23455c..71ab2dd0eedd5fa79300fe84fbd168b538dfd36c 100755 (executable)
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -7,12 +7,19 @@ test_description='Test git update-ref and basic ref logging'
 . ./test-lib.sh
 
 Z=0000000000000000000000000000000000000000
 . ./test-lib.sh
 
 Z=0000000000000000000000000000000000000000

-A=1111111111111111111111111111111111111111
-B=2222222222222222222222222222222222222222
-C=3333333333333333333333333333333333333333
-D=4444444444444444444444444444444444444444
-E=5555555555555555555555555555555555555555
-F=6666666666666666666666666666666666666666
+
+test_expect_success setup '
+
+       for name in A B C D E F
+       do
+               test_tick &&
+               T=$(git write-tree) &&
+               sha1=$(echo $name | git commit-tree $T) &&
+               eval $name=$sha1
+       done
+
+'
+

 m=refs/heads/master
 n_dir=refs/heads/gu
 n=$n_dir/fixes
 m=refs/heads/master
 n_dir=refs/heads/gu
 n=$n_dir/fixes