From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
"brian m. carlson" <sandals@crustytoothpaste.net>,
Justin Tobler <jltobler@gmail.com>
Subject: [PATCH v4 00/13] Stop relying on SHA1 fallback for `the_hash_algo`
Date: Tue, 7 May 2024 06:52:42 +0200 [thread overview]
Message-ID: <cover.1715057362.git.ps@pks.im> (raw)
In-Reply-To: <cover.1713519789.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5446 bytes --]
Hi,
this is the third version of my patch series that stops relying on the
SHA1 fallback configured for `the_hash_algo`. Instead, any callers that
access `the_hash_algo` without it being initialized will now crash hard.
This is designed to catch various subtle bugs going forward.
The only change compared to v3 of this patch series is a rebase on top
of the following two topics:
- ps/the-index-is-no-more, which causes a conflict in the final patch.
- jc/no-default-attr-tree-in-bare, which causes a conflict in the
fourth patch.
Interestingly, the range diff does not surface the second resolved
conflict. This is because the context is actually the same, but when
applying the patch in question you would get a conflict. To aid with
this I decided to generate patches with `--unified=4`, which surfaces
the previously-conflicting hunk in `compute_default_attr_source()`.
Patrick
Patrick Steinhardt (13):
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
oss-fuzz/commit-graph: set up hash algorithm
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 +-
oss-fuzz/fuzz-commit-graph.c | 1 +
parse-options-cb.c | 3 +-
path.c | 53 ------------------------------------
path.h | 1 -
remote-curl.c | 19 ++++++++++++-
repository.c | 20 --------------
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 ++++++++++
17 files changed, 168 insertions(+), 92 deletions(-)
Range-diff against v3:
1: 5134f35cda = 1: 5ee372c2d8 path: harden validation of HEAD with non-standard hashes
2: 589b6a99ef = 2: ece0ab94a8 path: move `validate_headref()` to its only user
3: 9a63c445d2 = 3: 1a0859eaf1 parse-options-cb: only abbreviate hashes when hash algo is known
4: 929bacbfce = 4: 1204a34216 attr: don't recompute default attribute source
5: 8f20aec1ee = 5: f098ce9690 attr: fix BUG() when parsing attrs outside of repo
6: 53439067a1 = 6: e8876052ad remote-curl: fix parsing of detached SHA256 heads
7: 1f74960760 = 7: 6116030310 builtin/rev-parse: allow shortening to more than 40 hex characters
8: 2d985abca1 = 8: 872ded113e builtin/blame: don't access potentially unitialized `the_hash_algo`
9: f3b23d28aa = 9: 5b4a21d2ce builtin/bundle: abort "verify" early when there is no repository
10: 7577b6b96c = 10: e7254ae757 builtin/diff: explicitly set hash algo when there is no repo
11: 509c79d1d3 = 11: c21b15ba17 builtin/shortlog: don't set up revisions without repo
12: 660f976129 = 12: f37beb0e89 oss-fuzz/commit-graph: set up hash algorithm
13: 95909c2da5 ! 13: 950b08bc78 repository: stop setting SHA1 as the default object hash
@@ Commit message
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## repository.c ##
-@@ repository.c: void initialize_the_repository(void)
- the_repo.parsed_objects = parsed_object_pool_new();
-
- index_state_init(&the_index, the_repository);
+@@ repository.c: void initialize_repository(struct repository *repo)
+ repo->parsed_objects = parsed_object_pool_new();
+ ALLOC_ARRAY(repo->index, 1);
+ index_state_init(repo->index, repo);
-
-- repo_set_hash_algo(&the_repo, GIT_HASH_SHA1);
+- /*
+- * 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,
--
2.45.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-05-07 4:53 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 9:51 [PATCH 00/11] Stop relying on SHA1 fallback for `the_hash_algo` Patrick Steinhardt
2024-04-19 9:51 ` [PATCH 01/11] path: harden validation of HEAD with non-standard hashes Patrick Steinhardt
2024-04-19 19:03 ` brian m. carlson
2024-04-22 4:56 ` Patrick Steinhardt
2024-04-22 16:15 ` Junio C Hamano
2024-04-23 4:50 ` Patrick Steinhardt
2024-04-23 16:54 ` Junio C Hamano
2024-04-19 9:51 ` [PATCH 02/11] parse-options-cb: only abbreviate hashes when hash algo is known Patrick Steinhardt
2024-04-23 0:30 ` Justin Tobler
2024-04-19 9:51 ` [PATCH 03/11] attr: don't recompute default attribute source Patrick Steinhardt
2024-04-23 0:32 ` Justin Tobler
2024-04-19 9:51 ` [PATCH 04/11] attr: fix BUG() when parsing attrs outside of repo Patrick Steinhardt
2024-04-19 9:51 ` [PATCH 05/11] remote-curl: fix parsing of detached SHA256 heads Patrick Steinhardt
2024-04-19 9:51 ` [PATCH 06/11] builtin/rev-parse: allow shortening to more than 40 hex characters Patrick Steinhardt
2024-04-19 9:51 ` [PATCH 07/11] builtin/blame: don't access potentially unitialized `the_hash_algo` Patrick Steinhardt
2024-04-19 9:51 ` [PATCH 08/11] builtin/bundle: abort "verify" early when there is no repository Patrick Steinhardt
2024-04-19 9:51 ` [PATCH 09/11] builtin/diff: explicitly set hash algo when there is no repo Patrick Steinhardt
2024-04-22 18:41 ` Junio C Hamano
2024-04-19 9:51 ` [PATCH 10/11] builtin/shortlog: don't set up revisions without repo Patrick Steinhardt
2024-04-23 0:35 ` Justin Tobler
2024-04-19 9:51 ` [PATCH 11/11] repository: stop setting SHA1 as the default object hash Patrick Steinhardt
2024-04-19 19:12 ` [PATCH 00/11] Stop relying on SHA1 fallback for `the_hash_algo` brian m. carlson
2024-04-19 19:16 ` Junio C Hamano
2024-04-22 4:56 ` Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 00/12] " Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 01/12] path: harden validation of HEAD with non-standard hashes Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 02/12] path: move `validate_headref()` to its only user Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 03/12] parse-options-cb: only abbreviate hashes when hash algo is known Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 04/12] attr: don't recompute default attribute source Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 05/12] attr: fix BUG() when parsing attrs outside of repo Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 06/12] remote-curl: fix parsing of detached SHA256 heads Patrick Steinhardt
2024-04-23 5:07 ` [PATCH v2 07/12] builtin/rev-parse: allow shortening to more than 40 hex characters Patrick Steinhardt
2024-04-23 5:08 ` [PATCH v2 08/12] builtin/blame: don't access potentially unitialized `the_hash_algo` Patrick Steinhardt
2024-04-23 5:08 ` [PATCH v2 09/12] builtin/bundle: abort "verify" early when there is no repository Patrick Steinhardt
2024-04-23 5:08 ` [PATCH v2 10/12] builtin/diff: explicitly set hash algo when there is no repo Patrick Steinhardt
2024-04-23 5:08 ` [PATCH v2 11/12] builtin/shortlog: don't set up revisions without repo Patrick Steinhardt
2024-04-23 5:08 ` [PATCH v2 12/12] repository: stop setting SHA1 as the default object hash Patrick Steinhardt
2024-04-27 22:09 ` [PATCH v2 00/12] Stop relying on SHA1 fallback for `the_hash_algo` Junio C Hamano
2024-04-29 6:05 ` Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 00/13] " Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 01/13] path: harden validation of HEAD with non-standard hashes Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 02/13] path: move `validate_headref()` to its only user Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 03/13] parse-options-cb: only abbreviate hashes when hash algo is known Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 04/13] attr: don't recompute default attribute source Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 05/13] attr: fix BUG() when parsing attrs outside of repo Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 06/13] remote-curl: fix parsing of detached SHA256 heads Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 07/13] builtin/rev-parse: allow shortening to more than 40 hex characters Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 08/13] builtin/blame: don't access potentially unitialized `the_hash_algo` Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 09/13] builtin/bundle: abort "verify" early when there is no repository Patrick Steinhardt
2024-04-29 6:34 ` [PATCH v3 10/13] builtin/diff: explicitly set hash algo when there is no repo Patrick Steinhardt
2024-04-29 6:35 ` [PATCH v3 11/13] builtin/shortlog: don't set up revisions without repo Patrick Steinhardt
2024-04-29 6:35 ` [PATCH v3 12/13] oss-fuzz/commit-graph: set up hash algorithm Patrick Steinhardt
2024-04-29 6:35 ` [PATCH v3 13/13] repository: stop setting SHA1 as the default object hash Patrick Steinhardt
2024-05-07 4:52 ` Patrick Steinhardt [this message]
2024-05-07 4:52 ` [PATCH v4 01/13] path: harden validation of HEAD with non-standard hashes Patrick Steinhardt
2024-05-07 4:52 ` [PATCH v4 02/13] path: move `validate_headref()` to its only user Patrick Steinhardt
2024-05-07 4:52 ` [PATCH v4 03/13] parse-options-cb: only abbreviate hashes when hash algo is known Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 04/13] attr: don't recompute default attribute source Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 05/13] attr: fix BUG() when parsing attrs outside of repo Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 06/13] remote-curl: fix parsing of detached SHA256 heads Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 07/13] builtin/rev-parse: allow shortening to more than 40 hex characters Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 08/13] builtin/blame: don't access potentially unitialized `the_hash_algo` Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 09/13] builtin/bundle: abort "verify" early when there is no repository Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 10/13] builtin/diff: explicitly set hash algo when there is no repo Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 11/13] builtin/shortlog: don't set up revisions without repo Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 12/13] oss-fuzz/commit-graph: set up hash algorithm Patrick Steinhardt
2024-05-07 4:53 ` [PATCH v4 13/13] repository: stop setting SHA1 as the default object hash Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1715057362.git.ps@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jltobler@gmail.com \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).