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
> )
> '
>
>
