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

[PATCH v2 00/12] Stop relying on SHA1 fallback for `the_hash_algo`

From
Patrick Steinhardt <ps@pks.im>
Date
Apr 23, 2024, 05:07 UTC
Message-ID
<cover.1713848619.git.ps@pks.im>
In-Reply-To
<cover.1713519789.git.ps@pks.im>
Hi,

this is the second version of my patch series that causes us to stop relying on the SHA1 default hash.

Changes compared to v1:
    - Various typo fixes in commit messages.
    - Added another patch that moves `validate_headref()` into "setup.c"
      to clarify that it is only used during repository discovery.
    - Indented a diff in a commit message so that git-am(1) is happy.
Thanks!
Patrick
Patrick Steinhardt (12):
  path: harden validation of HEAD with non-standard hashes
  path: move `validate_headref()` to its only user
  parse-options-cb: only abbreviate hashes when hash algo is known
  attr: don't recompute default attribute source
  attr: fix BUG() when parsing attrs outside of repo
  remote-curl: fix parsing of detached SHA256 heads
  builtin/rev-parse: allow shortening to more than 40 hex characters
  builtin/blame: don't access potentially unitialized `the_hash_algo`
  builtin/bundle: abort "verify" early when there is no repository
  builtin/diff: explicitly set hash algo when there is no repo
  builtin/shortlog: don't set up revisions without repo
  repository: stop setting SHA1 as the default object hash
 attr.c                     | 31 +++++++++++++++-------
 builtin/blame.c            |  5 ++--
 builtin/bundle.c           |  5 ++++
 builtin/diff.c             |  9 +++++++
 builtin/rev-parse.c        |  5 ++--
 builtin/shortlog.c         |  2 +-
 parse-options-cb.c         |  3 ++-
 path.c                     | 53 --------------------------------------
 path.h                     |  1 -
 remote-curl.c              | 19 +++++++++++++-
 repository.c               |  2 --
 setup.c                    | 53 ++++++++++++++++++++++++++++++++++++++
 t/t0003-attributes.sh      | 15 +++++++++++
 t/t0040-parse-options.sh   | 17 ++++++++++++
 t/t1500-rev-parse.sh       |  6 +++++
 t/t5550-http-fetch-dumb.sh | 15 +++++++++++
 16 files changed, 167 insertions(+), 74 deletions(-)
Range-diff against v1:
 1:  aa4d6f508b !  1:  a986b464d3 path: harden validation of HEAD with non-standard hashes
    @@ Commit message
         current version of Git doesn't understand yet. We'd still want to detect
         the repository as proper Git repository in that case, and we will fail
         eventually with a proper error message that the hash isn't understood
    -    when trying to set up the repostiory format.
    +    when trying to set up the repository format.
     
         It follows that we could just leave the current code intact, as in
         practice the code change doesn't have any user visible impact. But it
         also prepares us for `the_hash_algo` being unset when there is no
    -    repositroy.
    +    repository.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
 -:  ---------- >  2:  a347c7e6ca path: move `validate_headref()` to its only user
 2:  5daaaed2b9 !  3:  c0a15b2fa6 parse-options-cb: only abbreviate hashes when hash algo is known
    @@ Commit message
         parse-options-cb: only abbreviate hashes when hash algo is known
     
         The `OPT__ABBREV()` option can be used to add an option that abbreviates
    -    object IDs. When given an length longer than `the_hash_algo->hexsz`,
    -    then it will instead set the length to that maximum length.
    +    object IDs. When given a length longer than `the_hash_algo->hexsz`, then
    +    it will instead set the length to that maximum length.
     
         It may not always be guaranteed that we have `the_hash_algo` initialized
    -    properly as the hash algortihm can only be set up after we have set up
    +    properly as the hash algorithm can only be set up after we have set up
         `the_repository`. In that case, the hash would always be truncated to
         the hex length of SHA1, which may not be what the user desires.
     
 3:  ae91a27ffc !  4:  1b5f904eed attr: don't recompute default attribute source
    @@ Commit message
         variable is the null object ID then we try to look up the attr source,
         otherwise we skip over it.
     
    -    This has approach is flawed though: the variable will never be set to
    +    This approach is flawed though: the variable will never be set to
         anything else but the null object ID in case there is no attr source.
         Consequently, we re-compute the information on every call. And in the
         worst case, when we silently ignore bad trees, this will cause us to try
 4:  53c8e1cd7c =  5:  26909daca4 attr: fix BUG() when parsing attrs outside of repo
 5:  32a429fb60 =  6:  0b99184f50 remote-curl: fix parsing of detached SHA256 heads
 6:  9cb7baa50c =  7:  ccfda3c2d2 builtin/rev-parse: allow shortening to more than 40 hex characters
 7:  e189a4ad15 =  8:  1813e7eb5c builtin/blame: don't access potentially unitialized `the_hash_algo`
 8:  bc4bda3508 =  9:  31182a1fc6 builtin/bundle: abort "verify" early when there is no repository
 9:  39e56dab62 ! 10:  78e19d0a1b builtin/diff: explicitly set hash algo when there is no repo
    @@ Commit message
         hashing the files that we are diffing so that we can print the "index"
         line:
     
    -    ```
    -    diff --git a/a b/b
    -    index 7898192..6178079 100644
    -    --- a/a
    -    +++ b/b
    -    @@ -1 +1 @@
    -    -a
    -    +b
    -    ```
    +        ```
    +        diff --git a/a b/b
    +        index 7898192..6178079 100644
    +        --- a/a
    +        +++ b/b
    +        @@ -1 +1 @@
    +        -a
    +        +b
    +        ```
     
         We implicitly use SHA1 to calculate the hash here, which is because
         `the_repository` gets initialized with SHA1 during the startup routine.
10:  508e28ed1e ! 11:  51bcddbc31 builtin/shortlog: don't set up revisions without repo
    @@ Commit message
         repository in that context, it is thus unsupported to pass any revisions
         as arguments.
     
    -    Reghardless of that we still end up calling `setup_revisions()`. While
    +    Regardless of that we still end up calling `setup_revisions()`. While
         that works alright, it is somewhat strange. Furthermore, this is about
         to cause problems when we unset the default object hash.
     
11:  f86a6ff3ba = 12:  e8126371e1 repository: stop setting SHA1 as the default object hash
base-commit: 21306a098c3f174ad4c2a5cddb9069ee27a548b0
-- 
2.45.0-rc0
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 25 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.