On Thu, Jun 28, 2018 at 06:06:03PM -0400, Jeff King wrote: > Note that we didn't test this case at all, so I've added > coverage in t7415. We may end up toning down or removing > this fsck check in the future. So take this test as checking > what happens now with a focus on stderr, and not any > ironclad guarantee that we must detect and report parse > failures in the future. And such a patch _could_ look something like this. Though we could also perhaps leave it in place and tone it down to "ignore" by default. There's another case that triggers gitmodulesParse, too, which is a blob so gigantic that we try not to hold it in memory. Technically that could also happen due to somebody using .gitmodules for something unrelated. But that seems even more far-fetched. And it _is_ dangerous to leave, because I think existing vulnerable clients will try to load a 500MB .gitmodules file in memory and parse it. --- diff --git a/fsck.c b/fsck.c index 87b0e228bd..296e8a8a7c 100644 --- a/fsck.c +++ b/fsck.c @@ -1013,11 +1013,9 @@ static int fsck_blob(struct blob *blob, const char *buf, data.options = options; data.ret = 0; config_opts.error_action = CONFIG_ERROR_SILENT; - if (git_config_from_mem(fsck_gitmodules_fn, CONFIG_ORIGIN_BLOB, - ".gitmodules", buf, size, &data, &config_opts)) - data.ret |= report(options, &blob->object, - FSCK_MSG_GITMODULES_PARSE, - "could not parse gitmodules blob"); + /* ignore errors */ + git_config_from_mem(fsck_gitmodules_fn, CONFIG_ORIGIN_BLOB, + ".gitmodules", buf, size, &data, &config_opts); return data.ret; } diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh index ba8af785a5..9a7dae88a5 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7415-submodule-names.sh @@ -176,7 +176,7 @@ test_expect_success 'fsck detects non-blob .gitmodules' ' ) ' -test_expect_success 'fsck detects corrupt .gitmodules' ' +test_expect_success 'fsck does not complain about corrupt .gitmodules' ' git init corrupt && ( cd corrupt && @@ -185,9 +185,8 @@ test_expect_success 'fsck detects corrupt .gitmodules' ' git add .gitmodules && git commit -m "broken gitmodules" && - test_must_fail git fsck 2>output && - grep gitmodulesParse output && - test_i18ngrep ! "bad config" output + git fsck 2>output && + ! test -s output ) '
