On 28/06/18 23:12, Jeff King wrote: > 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. I also applied and tested the patch below. I think this patch must be included in the series. ATB, Ramsay Jones > --- > 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 > ) > ' > >
