do not write null sha1s to on-disk index

We should never need to write the null sha1 into an index
entry (short of the 1 in 2^160 chance that somebody actually
has content that hashes to it). If we attempt to do so, it
is much more likely that it is a bug, since we use the null
sha1 as a sentinel value to mean "not valid".

The presence of null sha1s in the index (which can come
from, among other things, "update-index --cacheinfo", or by
reading a corrupted tree) can cause problems for later
readers, because they cannot distinguish the literal null
sha1 from its use a sentinel value.  For example, "git
diff-files" on such an entry would make it appear as if it
is stat-dirty, and until recently, the diff code assumed
such an entry meant that we should be diffing a working tree
file rather than a blob.

Ideally, we would stop such entries from entering even our
in-core index. However, we do sometimes legitimately add
entries with null sha1s in order to represent these sentinel
situations; simply forbidding them in add_index_entry breaks
a lot of the existing code. However, we can at least make
sure that our in-core sentinel representation never makes it
to disk.

To be thorough, we will test an attempt to add both a blob
and a submodule entry. In the former case, we might run into
problems anyway because we will be missing the blob object.
But in the latter case, we do not enforce connectivity
across gitlink entries, making this our only point of
enforcement. The current implementation does not care which
type of entry we are seeing, but testing both cases helps
future-proof the test suite in case that changes.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/read-cache.c b/read-cache.c
index 274e54b4f31da69bf7c0721d4c8ba8e264db5dde..5ae7f2b680d8788f4c806d9c6ec111b6ba339aaa 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -1615,6 +1615,8 @@ int write_index(struct index_state *istate, int newfd)
                        continue;
                if (!ce_uptodate(ce) && is_racy_timestamp(istate, ce))
                        ce_smudge_racily_clean_entry(ce);
                        continue;
                if (!ce_uptodate(ce) && is_racy_timestamp(istate, ce))
                        ce_smudge_racily_clean_entry(ce);

+               if (is_null_sha1(ce->sha1))
+                       return error("cache entry has null sha1: %s", ce->name);

                if (ce_write_entry(&c, newfd, ce) < 0)
                        return -1;
        }
                if (ce_write_entry(&c, newfd, ce) < 0)
                        return -1;
        }

diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 809fafe208cbf68121295922a4c230b2a5e1aee3..0dbbb00d740da8b1e62325c7daea659bb97eb399 100755 (executable)
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -29,4 +29,23 @@ test_expect_success 'update-index -h with corrupt index' '
        grep "[Uu]sage: git update-index" broken/usage
 '
 
        grep "[Uu]sage: git update-index" broken/usage
 '
 

+test_expect_success '--cacheinfo does not accept blob null sha1' '
+       echo content >file &&
+       git add file &&
+       git rev-parse :file >expect &&
+       test_must_fail git update-index --cacheinfo 100644 $_z40 file &&
+       git rev-parse :file >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--cacheinfo does not accept gitlink null sha1' '
+       git init submodule &&
+       (cd submodule && test_commit foo) &&
+       git add submodule &&
+       git rev-parse :submodule >expect &&
+       test_must_fail git update-index --cacheinfo 160000 $_z40 submodule &&
+       git rev-parse :submodule >actual &&
+       test_cmp expect actual
+'
+

 test_done
 test_done