Skip to content

Fix use of uninitialized hash algos

Patrick Steinhardt git 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest2 patches 3 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2builtin/patch-id: fix uninitialized hash functionbuiltin/patch-id.c t/t4204-patch-id.sh+47 -00 replies
2/2builtin/hash-object: fix uninitialized hash functionbuiltin/hash-object.c t/t1007-hash-object.sh+13 -01 reply

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/2] Fix use of uninitialized hash algos

Hi,

with c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07), we have stopped setting up the default hash function for the_repository. This change was done so that we stop implicitly using SHA1 in places where we don't really intend to. Instead, code where we try to access the_hash_algo without having the_repository properly initialized will now crash hard.

I have found two more cases where this can now be triggered:

- git-patch-id(1) can read diffs from stdin.

- git-hash-object(1) can hash data from stdin.

Both cases can work without a repository, and if they don't have one they will now crash.

I still consider it a good thing that we did the change regardless of those crashes. In the case of git-patch-id(1) I would claim that using the_hash_algo is wrong in the first place, as patch IDs should be stable and are documented to always use SHA1. Thus, patch IDs in SHA256 repos are essentially broken. And in the case of git-hash-object(1), we should expose a command line option to let the user specify the object hash. So both cases demonstrate that there is room for improvement.

If these cases keep on popping up and we don't feel comfortable with it, then we can still decide to drop c8aed5e8da. The remainder of the topic that this commit was part of should in that case stay though, as those are real bug fixes. We could then re-try in a subsequent release cycle. But for now I don't think this would be warranted yet.

This topic depends on js/ps/undecided-is-not-necessarily-sha1 at c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07).

Thanks!

Patrick

Patrick Steinhardt (2):
builtin/patch-id: fix uninitialized hash function builtin/hash-object: fix uninitialized hash function

 builtin/hash-object.c  |  7 +++++++
 builtin/patch-id.c     | 13 +++++++++++++
 t/t1007-hash-object.sh |  6 ++++++
 t/t4204-patch-id.sh    | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+)
Signature · 1 line
2.45.GIT