config: fix segfault when parsing "core.abbrev" without repo

The "core.abbrev" config allows the user to specify the minimum length
when abbreviating object hashes. Next to the values "auto" and "no",
this config also accepts a concrete length that needs to be bigger or
equal to the minimum length and smaller or equal to the hash algorithm's
hex length. While the former condition is trivial, the latter depends on
the object format used by the current repository. It is thus a variable
upper boundary that may either be 40 (SHA-1) or 64 (SHA-256).

This has two major downsides. First, the user that specifies this config
must be aware of the object hashes that its repository use. If they want
to configure the value globally, then they cannot pick any value in the
range `[41, 64]` if they have any repository that uses SHA-1. If they
did, Git would error out when parsing the config.

Second, and more importantly, parsing "core.abbrev" crashes when outside
of a Git repository because we dereference `the_hash_algo` to figure out
its hex length. Starting with c8aed5e8da (repository: stop setting SHA1
as the default object hash, 2024-05-07) though, we stopped initializing
`the_hash_algo` outside of Git repositories.

Fix both of these issues by not making it an error anymore when the
given length exceeds the hash length. Instead, leave the abbreviated
length intact. `repo_find_unique_abbrev_r()` handles this just fine
except for a performance penalty which we will fix in a subsequent
commit.

Reported-by: Kyle Lippincott <spectral@google.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/config.c b/config.c
index 14461312b336dce8761334fe819c320cc9e5436d..d0e9396bda969566b97730e89505c3a197a86c19 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1456,10 +1456,10 @@ static int git_default_core_config(const char *var, const char *value,
                if (!strcasecmp(value, "auto"))
                        default_abbrev = -1;
                else if (!git_parse_maybe_bool_text(value))
-                       default_abbrev = the_hash_algo->hexsz;
+                       default_abbrev = GIT_MAX_HEXSZ;
                else {
                        int abbrev = git_config_int(var, value, ctx->kvi);
-                       if (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)
+                       if (abbrev < minimum_abbrev)
                                return error(_("abbrev length out of range: %d"), abbrev);
                        default_abbrev = abbrev;
                }

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 86c695eb0aef8d211053f549917666f838c3b5e1..e97826458c21cfa2ea072eccb1709230b6a0c5ce 100755 (executable)
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1237,6 +1237,18 @@ test_expect_success 'log.abbrevCommit configuration' '
        test_cmp expect.whatchanged.full actual
 '
 
+test_expect_success '--abbrev-commit with core.abbrev=false' '
+       git log --no-abbrev >expect &&
+       git -c core.abbrev=false log --abbrev-commit >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--abbrev-commit with core.abbrev=9000' '
+       git log --no-abbrev >expect &&
+       git -c core.abbrev=9000 log --abbrev-commit >actual &&
+       test_cmp expect actual
+'
+
 test_expect_success 'show added path under "--follow -M"' '
        # This tests for a regression introduced in v1.7.2-rc0~103^2~2
        test_create_repo regression &&

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index cc0b953f149f44aaac84a1465f156df34a7834f2..5d7ea147f1abc5ffae19ad093f8e12c3427a970a 100755 (executable)
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -46,6 +46,13 @@ test_expect_success 'output from clone' '
        test $(grep Clon output | wc -l) = 1
 '
 
+test_expect_success 'output from clone with core.abbrev does not crash' '
+       rm -fr dst &&
+       echo "Cloning into ${SQ}dst${SQ}..." >expect &&
+       git -c core.abbrev=12 clone -n "file://$(pwd)/src" dst >actual 2>&1 &&
+       test_cmp expect actual
+'
+
 test_expect_success 'clone does not keep pack' '
 
        rm -fr dst &&