git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/4] fsck: silence stderr when parsing .gitmodules

From
Jeff King <peff@peff.net>
Date
Jun 28, 2018, 22:12 UTC
Message-ID
<20180628221244.GA11462@sigill.intra.peff.net>
In-Reply-To
<20180628220603.GD5524@sigill.intra.peff.net>
On Thu, Jun 28, 2018 at 06:06:03PM -0400, Jeff King wrote:
Show 6 quoted lines
> 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
 	)
 '
 
Previous: Jeff KingNext: Ramsay Jones
Message 14 of 31 in “fsck: check skiplist for object in fsck_blob()”
  1. fsck: check skiplist for object in fsck_blob()Ramsay Jones, Jun 27, 2018
  2. Jeff KingJun 28, 2018
  3. Junio C HamanoJun 28, 2018
  4. Jeff KingJun 28, 2018
  5. Ramsay JonesJun 28, 2018
  6. Junio C HamanoJun 28, 2018
  7. Jeff KingJun 28, 2018
  8. Ramsay JonesJun 28, 2018
  9. Jeff KingJun 28, 2018
  10. 1/4 config: turn die_on_error into caller-facing enumJeff King, Jun 28, 2018
  11. 2/4 config: add CONFIG_ERROR_SILENT handlerJeff King, Jun 28, 2018
  12. 3/4 config: add options parameter to git_config_from_memJeff King, Jun 28, 2018
  13. 4/4 fsck: silence stderr when parsing .gitmodulesJeff King, Jun 28, 2018
  14. Jeff KingJun 28, 2018
  15. Ramsay JonesJun 29, 2018
  16. Ramsay JonesJun 29, 2018
  17. Jeff KingJul 3, 2018
  18. Ramsay JonesJul 4, 2018
  19. Jeff KingJul 7, 2018
  20. Ramsay JonesJul 11, 2018
  21. Ramsay JonesJul 13, 2018
  22. Jeff KingJul 13, 2018
  23. Jeff KingJul 13, 2018
  24. Ramsay JonesJul 13, 2018
  25. Jeff KingJul 13, 2018
  26. 1/2 fsck: split ".gitmodules too large" error from parse failureJeff King, Jul 13, 2018
  27. 2/2 fsck: downgrade gitmodulesParse default to "info"Jeff King, Jul 13, 2018
  28. Stefan BellerJul 13, 2018
  29. Junio C HamanoJul 16, 2018
  30. Jeff KingJul 16, 2018
  31. Junio C HamanoJul 16, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.