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

[PATCH v4 13/13] repository: stop setting SHA1 as the default object hash

From
Patrick Steinhardt <ps@pks.im>
Date
May 7, 2024, 04:53 UTC
Message-ID
<950b08bc78a76a6f627698372306e2eccc0f3c39.1715057362.git.ps@pks.im>
In-Reply-To
<cover.1715057362.git.ps@pks.im>

During the startup of Git, we call `initialize_the_repository()` to set up `the_repository` as well as `the_index`. Part of this setup is also to set the default object hash of the repository to SHA1. This has the effect that `the_hash_algo` is getting initialized to SHA1, as well. This default hash algorithm eventually gets overridden by most Git commands via `setup_git_directory()`, which also detects the actual hash algorithm used by the repository.

There are some commands though that don't access a repository at all, or at a later point only, and thus retain the default hash function for some amount of time. As some of the the preceding commits demonstrate, this can lead to subtle issues when we access `the_hash_algo` when no repository has been set up.

Address this issue by dropping the set up of the default hash algorithm completely. The effect of this is that `the_hash_algo` will map to a `NULL` pointer and thus cause Git to crash when something tries to access the hash algorithm without it being properly initialized. It thus forces all Git commands to explicitly set up the hash algorithm in case there is no repository.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 repository.c | 20 --------------------
 1 file changed, 20 deletions(-)
diff --git a/repository.c b/repository.c
index 2118f563e3..15c10015b0 100644
--- a/repository.c
+++ b/repository.c
@@ -25,28 +25,8 @@ void initialize_repository(struct repository *repo)
 	repo->remote_state = remote_state_new();
 	repo->parsed_objects = parsed_object_pool_new();
 	ALLOC_ARRAY(repo->index, 1);
 	index_state_init(repo->index, repo);
-
-	/*
-	 * Unfortunately, we need to keep this hack around for the time being:
-	 *
-	 *   - Not setting up the hash algorithm for `the_repository` leads to
-	 *     crashes because `the_hash_algo` is a macro that expands to
-	 *     `the_repository->hash_algo`. So if Git commands try to access
-	 *     `the_hash_algo` without a Git directory we crash.
-	 *
-	 *   - Setting up the hash algorithm to be SHA1 by default breaks other
-	 *     commands when running with SHA256.
-	 *
-	 * This is another point in case why having global state is a bad idea.
-	 * Eventually, we should remove this hack and stop setting the hash
-	 * algorithm in this function altogether. Instead, it should only ever
-	 * be set via our repository setup procedures. But that requires more
-	 * work.
-	 */
-	if (repo == the_repository)
-		repo_set_hash_algo(repo, GIT_HASH_SHA1);
 }
 
 static void expand_base_dir(char **out, const char *in,
 			    const char *base_dir, const char *def_in)
-- 
2.45.0
Previous: Patrick Steinhardt
Message 67 of 67 in “Stop relying on SHA1 fallback for `the_hash_algo`”
  1. 00/11 Stop relying on SHA1 fallback for `the_hash_algo`Patrick Steinhardt, Apr 19, 2024
  2. 01/11 path: harden validation of HEAD with non-standard hashesPatrick Steinhardt, Apr 19, 2024
  3. brian m. carlsonApr 19, 2024
  4. Patrick SteinhardtApr 22, 2024
  5. Junio C HamanoApr 22, 2024
  6. Patrick SteinhardtApr 23, 2024
  7. Junio C HamanoApr 23, 2024
  8. 02/11 parse-options-cb: only abbreviate hashes when hash algo is knownPatrick Steinhardt, Apr 19, 2024
  9. Justin ToblerApr 23, 2024
  10. 03/11 attr: don't recompute default attribute sourcePatrick Steinhardt, Apr 19, 2024
  11. Justin ToblerApr 23, 2024
  12. 04/11 attr: fix BUG() when parsing attrs outside of repoPatrick Steinhardt, Apr 19, 2024
  13. 05/11 remote-curl: fix parsing of detached SHA256 headsPatrick Steinhardt, Apr 19, 2024
  14. 06/11 builtin/rev-parse: allow shortening to more than 40 hex charactersPatrick Steinhardt, Apr 19, 2024
  15. 07/11 builtin/blame: don't access potentially unitialized `the_hash_algo`Patrick Steinhardt, Apr 19, 2024
  16. 08/11 builtin/bundle: abort "verify" early when there is no repositoryPatrick Steinhardt, Apr 19, 2024
  17. 09/11 builtin/diff: explicitly set hash algo when there is no repoPatrick Steinhardt, Apr 19, 2024
  18. Junio C HamanoApr 22, 2024
  19. 10/11 builtin/shortlog: don't set up revisions without repoPatrick Steinhardt, Apr 19, 2024
  20. Justin ToblerApr 23, 2024
  21. 11/11 repository: stop setting SHA1 as the default object hashPatrick Steinhardt, Apr 19, 2024
  22. brian m. carlsonApr 19, 2024
  23. Junio C HamanoApr 19, 2024
  24. Patrick SteinhardtApr 22, 2024
  25. 00/12 Stop relying on SHA1 fallback for `the_hash_algo`Patrick Steinhardt, Apr 23, 2024
  26. 01/12 path: harden validation of HEAD with non-standard hashesPatrick Steinhardt, Apr 23, 2024
  27. 02/12 path: move `validate_headref()` to its only userPatrick Steinhardt, Apr 23, 2024
  28. 03/12 parse-options-cb: only abbreviate hashes when hash algo is knownPatrick Steinhardt, Apr 23, 2024
  29. 04/12 attr: don't recompute default attribute sourcePatrick Steinhardt, Apr 23, 2024
  30. 05/12 attr: fix BUG() when parsing attrs outside of repoPatrick Steinhardt, Apr 23, 2024
  31. 06/12 remote-curl: fix parsing of detached SHA256 headsPatrick Steinhardt, Apr 23, 2024
  32. 07/12 builtin/rev-parse: allow shortening to more than 40 hex charactersPatrick Steinhardt, Apr 23, 2024
  33. 08/12 builtin/blame: don't access potentially unitialized `the_hash_algo`Patrick Steinhardt, Apr 23, 2024
  34. 09/12 builtin/bundle: abort "verify" early when there is no repositoryPatrick Steinhardt, Apr 23, 2024
  35. 10/12 builtin/diff: explicitly set hash algo when there is no repoPatrick Steinhardt, Apr 23, 2024
  36. 11/12 builtin/shortlog: don't set up revisions without repoPatrick Steinhardt, Apr 23, 2024
  37. 12/12 repository: stop setting SHA1 as the default object hashPatrick Steinhardt, Apr 23, 2024
  38. Junio C HamanoApr 27, 2024
  39. Patrick SteinhardtApr 29, 2024
  40. 00/13 Stop relying on SHA1 fallback for `the_hash_algo`Patrick Steinhardt, Apr 29, 2024
  41. 01/13 path: harden validation of HEAD with non-standard hashesPatrick Steinhardt, Apr 29, 2024
  42. 02/13 path: move `validate_headref()` to its only userPatrick Steinhardt, Apr 29, 2024
  43. 03/13 parse-options-cb: only abbreviate hashes when hash algo is knownPatrick Steinhardt, Apr 29, 2024
  44. 04/13 attr: don't recompute default attribute sourcePatrick Steinhardt, Apr 29, 2024
  45. 05/13 attr: fix BUG() when parsing attrs outside of repoPatrick Steinhardt, Apr 29, 2024
  46. 06/13 remote-curl: fix parsing of detached SHA256 headsPatrick Steinhardt, Apr 29, 2024
  47. 07/13 builtin/rev-parse: allow shortening to more than 40 hex charactersPatrick Steinhardt, Apr 29, 2024
  48. 08/13 builtin/blame: don't access potentially unitialized `the_hash_algo`Patrick Steinhardt, Apr 29, 2024
  49. 09/13 builtin/bundle: abort "verify" early when there is no repositoryPatrick Steinhardt, Apr 29, 2024
  50. 10/13 builtin/diff: explicitly set hash algo when there is no repoPatrick Steinhardt, Apr 29, 2024
  51. 11/13 builtin/shortlog: don't set up revisions without repoPatrick Steinhardt, Apr 29, 2024
  52. 12/13 oss-fuzz/commit-graph: set up hash algorithmPatrick Steinhardt, Apr 29, 2024
  53. 13/13 repository: stop setting SHA1 as the default object hashPatrick Steinhardt, Apr 29, 2024
  54. 00/13 Stop relying on SHA1 fallback for `the_hash_algo`Patrick Steinhardt, May 7, 2024
  55. 01/13 path: harden validation of HEAD with non-standard hashesPatrick Steinhardt, May 7, 2024
  56. 02/13 path: move `validate_headref()` to its only userPatrick Steinhardt, May 7, 2024
  57. 03/13 parse-options-cb: only abbreviate hashes when hash algo is knownPatrick Steinhardt, May 7, 2024
  58. 04/13 attr: don't recompute default attribute sourcePatrick Steinhardt, May 7, 2024
  59. 05/13 attr: fix BUG() when parsing attrs outside of repoPatrick Steinhardt, May 7, 2024
  60. 06/13 remote-curl: fix parsing of detached SHA256 headsPatrick Steinhardt, May 7, 2024
  61. 07/13 builtin/rev-parse: allow shortening to more than 40 hex charactersPatrick Steinhardt, May 7, 2024
  62. 08/13 builtin/blame: don't access potentially unitialized `the_hash_algo`Patrick Steinhardt, May 7, 2024
  63. 09/13 builtin/bundle: abort "verify" early when there is no repositoryPatrick Steinhardt, May 7, 2024
  64. 10/13 builtin/diff: explicitly set hash algo when there is no repoPatrick Steinhardt, May 7, 2024
  65. 11/13 builtin/shortlog: don't set up revisions without repoPatrick Steinhardt, May 7, 2024
  66. 12/13 oss-fuzz/commit-graph: set up hash algorithmPatrick Steinhardt, May 7, 2024
  67. 13/13 repository: stop setting SHA1 as the default object hashPatrick Steinhardt, May 7, 2024

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.