Re: update-index --index-info producing spurious submodule commits
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 18, 2011, 22:49 UTC
- Message-ID
- <7vd3g272tk.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <rmivctuv12s.fsf@fnord.ir.bbn.com>
Greg Troxel <gdt@ir.bbn.com> writes:
> git ls-tree HEAD foo > git ls-tree HEAD foo | git update-index --index-info
This --index-info definitely looks wrong, if "foo" is a directory, as the entries in the index are supposed to be either blobs or commits.
As "update-index --index-info" predates "submodule" by a few years or more, I wouldn't be surprised if the code didn't notice it was fed a wrong input and produced nonsensical result that happened to be a commit.
The command could just instead barf, saying the input is wrong, but the option was so low-level that it was deliberately written to accept and store anything you throw at it --- even when it is nonsensical for the version of plumbing, later updates to the data structure might have made it making sense, which was the way to ease development of the system.
By now, we should start enforcing more sanity on its input.
builtin/update-index.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c index a6a23fa..4b32bfe 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -220,6 +220,12 @@ static int process_path(const char *path) return add_one_path(ce, path, len, &st); } +static int verify_mode(unsigned int mode) +{ + return (mode == 0160000 || mode == 0120000 || + mode == 0100644 || mode == 0100755); +} + static int add_cacheinfo(unsigned int mode, const unsigned char *sha1, const char *path, int stage) { @@ -229,6 +235,9 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1, if (!verify_path(path)) return error("Invalid path '%s'", path); + if (!verify_mode(mode)) + return error("Invalid mode '%o'", mode); + len = strlen(path); size = cache_entry_size(len); ce = xcalloc(1, size);