From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
"Jonathan Tan" <jonathantanmy@google.com>,
"Andrei Rybak" <rybak.a.v@gmail.com>,
"Taylor Blau" <me@ttaylorr.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v7 00/17] fsck: lib-ify object-file.c & better fsck "invalid object" error reporting
Date: Mon, 20 Sep 2021 21:04:04 +0200 [thread overview]
Message-ID: <cover-v7-00.17-00000000000-20210920T190304Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v6-00.22-00000000000-20210907T104558Z-avarab@gmail.com>
This improves fsck error reporting, see the examples in the commit
messages of 16/17 and 17/17. To get there I've lib-ified more things
in object-file.c and the general object APIs, i.e. now we'll return
error codes instead of calling die() in these cases.
v6 of this got a very detailed review from Taylor Blau (thanks a
lot!), for the v6 see:
https://lore.kernel.org/git/cover-v6-00.22-00000000000-20210907T104558Z-avarab@gmail.com/
This should address all of the things brought up, and more. After
leaving this series for a while I came up with ways to simplify it
even more, so now it's 17 instead of 22 patches!
So things like:
* The move of functions from cache.h to object-store.h is gone, that
still makes sense to do, but can be left for later.
* A large part of the mid-series is squashed together and
re-arranged, e.g. I moved the migration of unpack_loose_header() to
to an enum earlier, which simplified later steps. The 15/17 rewrite
of much of parse_loose_header() is now much simpler.
* I attempted to address the comments about the tests with some
for-loops and boilerplate-generated testing, maybe some of it's a
bit too ugly, but it's both less copy/pasting now, and more cases
(e.g. the "cat-file -p" case) are tested.
* We now test for what happens/error reporting when we append garbage
to a loose object.
* Many more small changes / improvements / simplifications, see the
range-diff below, but given its size perhaps a re-read is easier...
Ævar Arnfjörð Bjarmason (17):
fsck tests: add test for fsck-ing an unknown type
fsck tests: refactor one test to use a sub-repo
fsck tests: test current hash/type mismatch behavior
fsck tests: test for garbage appended to a loose object
cat-file tests: move bogus_* variable declarations earlier
cat-file tests: test for missing/bogus object with -t, -s and -p
cat-file tests: add corrupt loose object test
cat-file tests: test for current --allow-unknown-type behavior
object-file.c: don't set "typep" when returning non-zero
object-file.c: return -1, not "status" from unpack_loose_header()
object-file.c: make parse_loose_header_extended() public
object-file.c: simplify unpack_loose_short_header()
object-file.c: use "enum" return type for unpack_loose_header()
object-file.c: return ULHR_TOO_LONG on "header too long"
object-file.c: stop dying in parse_loose_header()
fsck: don't hard die on invalid object types
fsck: report invalid object type-path combinations
builtin/fast-export.c | 2 +-
builtin/fsck.c | 28 +++++-
builtin/index-pack.c | 2 +-
builtin/mktag.c | 3 +-
cache.h | 45 ++++++++-
object-file.c | 176 +++++++++++++++------------------
object-store.h | 7 +-
object.c | 4 +-
pack-check.c | 3 +-
streaming.c | 27 +++--
t/oid-info/oid | 2 +
t/t1006-cat-file.sh | 223 +++++++++++++++++++++++++++++++++++++++---
t/t1450-fsck.sh | 99 +++++++++++++++----
13 files changed, 463 insertions(+), 158 deletions(-)
Range-diff against v6:
2: 9072eef3be3 ! 1: 752cef556c2 fsck tests: add test for fsck-ing an unknown type
@@ t/t1450-fsck.sh: test_expect_success 'detect corrupt index file in fsck' '
+test_expect_success 'fsck hard errors on an invalid object type' '
+ git init --bare garbage-type &&
-+ empty_blob=$(git -C garbage-type hash-object --stdin -w -t blob </dev/null) &&
-+ garbage_blob=$(git -C garbage-type hash-object --stdin -w -t garbage --literally </dev/null) &&
-+ cat >err.expect <<-\EOF &&
-+ fatal: invalid object type
-+ EOF
-+ test_must_fail git -C garbage-type fsck >out.actual 2>err.actual &&
-+ test_cmp err.expect err.actual &&
-+ test_must_be_empty out.actual
++ (
++ cd garbage-type &&
++
++ empty=$(git hash-object --stdin -w -t blob </dev/null) &&
++ garbage=$(git hash-object --stdin -w -t garbage --literally </dev/null) &&
++
++ cat >err.expect <<-\EOF &&
++ fatal: invalid object type
++ EOF
++ test_must_fail git fsck >out 2>err &&
++ test_cmp err.expect err &&
++ test_must_be_empty out
++ )
+'
+
test_done
1: ebe89f65354 ! 2: 612003bdd2c fsck tests: refactor one test to use a sub-repo
@@ Commit message
teardown of a tests so we're not leaving corrupt content for the next
test.
- We should instead simply use something like this test_create_repo
- pattern. It's both less verbose, and makes things easier to debug as a
- failing test can have their state left behind under -d without
- damaging the state for other tests.
+ We can instead use the pattern of creating a named sub-repository,
+ then we don't have to worry about cleaning up after ourselves, nobody
+ will care what state the broken "hash-mismatch" repository is after
+ this test runs.
- But let's punt on that general refactoring and just change this one
- test, I'm going to change it further in subsequent commits.
+ See [1] for related discussion on various "modern" test patterns that
+ can be used to avoid verbosity and increase reliability.
+
+ 1. https://lore.kernel.org/git/87y27veeyj.fsf@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ t/t1450-fsck.sh: remove_object () {
- test_when_finished "remove_object $cmt" &&
- git update-ref refs/heads/bogus $cmt &&
- test_when_finished "git update-ref -d refs/heads/bogus" &&
--
-- test_must_fail git fsck 2>out &&
-- test_i18ngrep "$sha.*corrupt" out
+test_expect_success 'object with hash mismatch' '
+ git init --bare hash-mismatch &&
+ (
+ cd hash-mismatch &&
+
+- test_must_fail git fsck 2>out &&
+- test_i18ngrep "$sha.*corrupt" out
+ oid=$(echo blob | git hash-object -w --stdin) &&
+ old=$(test_oid_to_path "$oid") &&
+ new=$(dirname $old)/$(test_oid ff_2) &&
+ oid="$(dirname $new)$(basename $new)" &&
++
+ mv objects/$old objects/$new &&
+ git update-index --add --cacheinfo 100644 $oid foo &&
+ tree=$(git write-tree) &&
+ cmt=$(echo bogus | git commit-tree $tree) &&
+ git update-ref refs/heads/bogus $cmt &&
++
+ test_must_fail git fsck 2>out &&
-+ test_i18ngrep "$oid.*corrupt" out
++ grep "$oid.*corrupt" out
+ )
'
3: d442a309178 ! 3: 1e40a4235e9 cat-file tests: test for missing object with -t and -s
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- cat-file tests: test for missing object with -t and -s
+ fsck tests: test current hash/type mismatch behavior
- Test for what happens when the -t and -s flags are asked to operate on
- a missing object, this extends tests added in 3e370f9faf0 (t1006: add
- tests for git cat-file --allow-unknown-type, 2015-05-03). The -t and
- -s flags are the only ones that can be combined with
- --allow-unknown-type, so let's test with and without that flag.
+ If fsck we move an object around between .git/objects/?? directories
+ to simulate a hash mismatch "git fsck" will currently hard die() in
+ object-file.c. This behavior will be fixed in subsequent commits, but
+ let's test for it as-is for now.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- ## t/t1006-cat-file.sh ##
-@@ t/t1006-cat-file.sh: test_expect_success '%(deltabase) reports packed delta bases' '
- }
+ ## t/t1450-fsck.sh ##
+@@ t/t1450-fsck.sh: test_expect_success 'object with hash mismatch' '
+ )
'
-+missing_oid=$(test_oid deadbeef)
-+test_expect_success 'error on type of missing object' '
-+ cat >expect.err <<-\EOF &&
-+ fatal: git cat-file: could not get object info
-+ EOF
-+ test_must_fail git cat-file -t $missing_oid >out 2>err &&
-+ test_must_be_empty out &&
-+ test_cmp expect.err err &&
++test_expect_success 'object with hash and type mismatch' '
++ git init --bare hash-type-mismatch &&
++ (
++ cd hash-type-mismatch &&
+
-+ test_must_fail git cat-file -t --allow-unknown-type $missing_oid >out 2>err &&
-+ test_must_be_empty out &&
-+ test_cmp expect.err err
-+'
++ oid=$(echo blob | git hash-object -w --stdin -t garbage --literally) &&
++ old=$(test_oid_to_path "$oid") &&
++ new=$(dirname $old)/$(test_oid ff_2) &&
++ oid="$(dirname $new)$(basename $new)" &&
+
-+test_expect_success 'error on size of missing object' '
-+ cat >expect.err <<-\EOF &&
-+ fatal: git cat-file: could not get object info
-+ EOF
-+ test_must_fail git cat-file -s $missing_oid >out 2>err &&
-+ test_must_be_empty out &&
-+ test_cmp expect.err err &&
++ mv objects/$old objects/$new &&
++ git update-index --add --cacheinfo 100644 $oid foo &&
++ tree=$(git write-tree) &&
++ cmt=$(echo bogus | git commit-tree $tree) &&
++ git update-ref refs/heads/bogus $cmt &&
+
-+ test_must_fail git cat-file -s --allow-unknown-type $missing_oid >out 2>err &&
-+ test_must_be_empty out &&
-+ test_cmp expect.err err
++ cat >expect <<-\EOF &&
++ fatal: invalid object type
++ EOF
++ test_must_fail git fsck 2>actual &&
++ test_cmp expect actual
++ )
+'
+
- bogus_type="bogus"
- bogus_content="bogus"
- bogus_size=$(strlen "$bogus_content")
+ test_expect_success 'branch pointing to non-commit' '
+ git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
+ test_when_finished "git update-ref -d refs/heads/invalid" &&
5: 82db40ebf8a ! 4: 854991c1543 rev-list tests: test for behavior with invalid object types
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- rev-list tests: test for behavior with invalid object types
+ fsck tests: test for garbage appended to a loose object
- Fix a blindspot in the tests for the "rev-list --disk-usage" feature
- added in 16950f8384a (rev-list: add --disk-usage option for
- calculating disk usage, 2021-02-09) to test for what happens when it's
- asked to calculate the disk usage of invalid object types.
+ There wasn't any output tests for this scenario, let's ensure that we
+ don't regress on it in the changes that come after this.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- ## t/t6115-rev-list-du.sh ##
-@@ t/t6115-rev-list-du.sh: check_du HEAD
- check_du --objects HEAD
- check_du --objects HEAD^..HEAD
+ ## t/t1450-fsck.sh ##
+@@ t/t1450-fsck.sh: test_expect_success 'object with hash and type mismatch' '
+ )
+ '
-+test_expect_success 'setup garbage repository' '
-+ git clone --bare . garbage.git &&
-+ garbage_oid=$(git -C garbage.git hash-object -t garbage -w --stdin --literally <one.t) &&
-+ git -C garbage.git rev-list --objects --all --disk-usage &&
++test_expect_success POSIXPERM 'zlib corrupt loose object output ' '
++ git init --bare corrupt-loose-output &&
++ (
++ cd corrupt-loose-output &&
++ oid=$(git hash-object -w --stdin --literally </dev/null) &&
++ oidf=objects/$(test_oid_to_path "$oid") &&
++ chmod 755 $oidf &&
++ echo extra garbage >>$oidf &&
+
-+ # Manually create a ref because "update-ref", "tag" etc. have
-+ # no corresponding --literally option.
-+ echo $garbage_oid >garbage.git/refs/tags/garbage-tag &&
-+ test_must_fail git -C garbage.git rev-list --objects --all --disk-usage
++ cat >expect.error <<-EOF &&
++ error: garbage at end of loose object '\''$oid'\''
++ error: unable to unpack contents of ./$oidf
++ error: $oid: object corrupt or missing: ./$oidf
++ EOF
++ test_must_fail git fsck 2>actual &&
++ grep ^error: actual >error &&
++ test_cmp expect.error error
++ )
+'
+
- test_done
+ test_expect_success 'branch pointing to non-commit' '
+ git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
+ test_when_finished "git update-ref -d refs/heads/invalid" &&
19: ad1614dbb8d ! 5: fc93c2c2530 fsck: don't hard die on invalid object types
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- fsck: don't hard die on invalid object types
+ cat-file tests: move bogus_* variable declarations earlier
- Change the error fsck emits on invalid object types, such as:
-
- $ git hash-object --stdin -w -t garbage --literally </dev/null
- <OID>
-
- From the very ungraceful error of:
-
- $ git fsck
- fatal: invalid object type
- $
-
- To:
-
- $ git fsck
- error: hash mismatch for <OID_PATH> (expected <OID>)
- error: <OID>: object corrupt or missing: <OID_PATH>
- [ the rest of the fsck output here, i.e. it didn't hard die ]
-
- We'll still exit with non-zero, but now we'll finish the rest of the
- traversal. The tests that's being added here asserts that we'll still
- complain about other fsck issues (e.g. an unrelated dangling blob).
-
- To do this we need to pass down the "OBJECT_INFO_ALLOW_UNKNOWN_TYPE"
- flag from read_loose_object() through to parse_loose_header(). Since
- the read_loose_object() function is only used in builtin/fsck.c we can
- simply change it. See f6371f92104 (sha1_file: add read_loose_object()
- function, 2017-01-13) for the introduction of read_loose_object().
-
- Why are we complaining about a "hash mismatch" for an object of a type
- we don't know about? We shouldn't. This is the bare minimal change
- needed to not make fsck hard die on a repository that's been corrupted
- in this manner. In subsequent commits we'll teach fsck to recognize
- this particular type of corruption and emit a better error message.
+ Change the short/long bogus bogus object type variables into a form
+ where the two sets can be used concurrently. This'll be used by
+ subsequently added tests.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- ## builtin/fsck.c ##
-@@ builtin/fsck.c: static int fsck_loose(const struct object_id *oid, const char *path, void *data)
- void *contents;
- int eaten;
+ ## t/t1006-cat-file.sh ##
+@@ t/t1006-cat-file.sh: test_expect_success '%(deltabase) reports packed delta bases' '
+ }
+ '
-- if (read_loose_object(path, oid, &type, &size, &contents) < 0) {
-+ if (read_loose_object(path, oid, &type, &size, &contents,
-+ OBJECT_INFO_ALLOW_UNKNOWN_TYPE) < 0) {
- errors_found |= ERROR_OBJECT;
- error(_("%s: object corrupt or missing: %s"),
- oid_to_hex(oid), path);
-
- ## object-file.c ##
-@@ object-file.c: int read_loose_object(const char *path,
- const struct object_id *expected_oid,
- enum object_type *type,
- unsigned long *size,
-- void **contents)
-+ void **contents,
-+ unsigned int oi_flags)
- {
- int ret = -1;
- void *map = NULL;
-@@ object-file.c: int read_loose_object(const char *path,
- git_zstream stream;
- char hdr[MAX_HEADER_LEN];
- struct object_info oi = OBJECT_INFO_INIT;
-+ int allow_unknown = oi_flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
- oi.typep = type;
- oi.sizep = size;
+-bogus_type="bogus"
+-bogus_content="bogus"
+-bogus_size=$(strlen "$bogus_content")
+-bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
++test_expect_success 'setup bogus data' '
++ bogus_short_type="bogus" &&
++ bogus_short_content="bogus" &&
++ bogus_short_size=$(strlen "$bogus_short_content") &&
++ bogus_short_sha1=$(echo_without_newline "$bogus_short_content" | git hash-object -t $bogus_short_type --literally -w --stdin) &&
++
++ bogus_long_type="abcdefghijklmnopqrstuvwxyz1234679" &&
++ bogus_long_content="bogus" &&
++ bogus_long_size=$(strlen "$bogus_long_content") &&
++ bogus_long_sha1=$(echo_without_newline "$bogus_long_content" | git hash-object -t $bogus_long_type --literally -w --stdin)
++'
-@@ object-file.c: int read_loose_object(const char *path,
- git_inflate_end(&stream);
- goto out;
- }
-- if (*type < 0)
-- die(_("invalid object type"));
-+ if (!allow_unknown && *type < 0) {
-+ error(_("header for %s declares an unknown type"), path);
-+ git_inflate_end(&stream);
-+ goto out;
-+ }
+ test_expect_success "Type of broken object is correct" '
+- echo $bogus_type >expect &&
+- git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
++ echo $bogus_short_type >expect &&
++ git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
+ test_cmp expect actual
+ '
- if (*type == OBJ_BLOB && *size > big_file_threshold) {
- if (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)
-
- ## object-store.h ##
-@@ object-store.h: int read_loose_object(const char *path,
- const struct object_id *expected_oid,
- enum object_type *type,
- unsigned long *size,
-- void **contents);
-+ void **contents,
-+ unsigned int oi_flags);
+ test_expect_success "Size of broken object is correct" '
+- echo $bogus_size >expect &&
+- git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
++ echo $bogus_short_size >expect &&
++ git cat-file -s --allow-unknown-type $bogus_short_sha1 >actual &&
+ test_cmp expect actual
+ '
+-bogus_type="abcdefghijklmnopqrstuvwxyz1234679"
+-bogus_content="bogus"
+-bogus_size=$(strlen "$bogus_content")
+-bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
- /* Retry packed storage after checking packed and loose storage */
- #define HAS_OBJECT_RECHECK_PACKED 1
-
- ## t/t1450-fsck.sh ##
-@@ t/t1450-fsck.sh: test_expect_success 'detect corrupt index file in fsck' '
- test_i18ngrep "bad index file" errors
+ test_expect_success "Type of broken object is correct when type is large" '
+- echo $bogus_type >expect &&
+- git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
++ echo $bogus_long_type >expect &&
++ git cat-file -t --allow-unknown-type $bogus_long_sha1 >actual &&
+ test_cmp expect actual
'
--test_expect_success 'fsck hard errors on an invalid object type' '
-+test_expect_success 'fsck error and recovery on invalid object type' '
- git init --bare garbage-type &&
- empty_blob=$(git -C garbage-type hash-object --stdin -w -t blob </dev/null) &&
- garbage_blob=$(git -C garbage-type hash-object --stdin -w -t garbage --literally </dev/null) &&
-- cat >err.expect <<-\EOF &&
-- fatal: invalid object type
-- EOF
-- test_must_fail git -C garbage-type fsck >out.actual 2>err.actual &&
-- test_cmp err.expect err.actual &&
-- test_must_be_empty out.actual
-+ test_must_fail git -C garbage-type fsck >out 2>err &&
-+ grep -e "^error" -e "^fatal" err >errors &&
-+ test_line_count = 2 errors &&
-+ grep "error: hash mismatch for" err &&
-+ grep "$garbage_blob: object corrupt or missing:" err &&
-+ grep "dangling blob $empty_blob" out
+ test_expect_success "Size of large broken object is correct when type is large" '
+- echo $bogus_size >expect &&
+- git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
++ echo $bogus_long_size >expect &&
++ git cat-file -s --allow-unknown-type $bogus_long_sha1 >actual &&
+ test_cmp expect actual
'
- test_done
4: 0358273022f ! 6: 051088aa114 cat-file tests: test that --allow-unknown-type isn't on by default
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- cat-file tests: test that --allow-unknown-type isn't on by default
+ cat-file tests: test for missing/bogus object with -t, -s and -p
- Fix a blindspot in the tests for the --allow-unknown-type feature
- added in 39e4ae38804 (cat-file: teach cat-file a
- '--allow-unknown-type' option, 2015-05-03). We should check that
- --allow-unknown-type isn't on by default.
+ When we look up a missing object with cat_one_file() what error we
+ print out currently depends on whether we'll error out early in
+ get_oid_with_context(), or if we'll get an error later from
+ oid_object_info_extended().
- Before this change all the tests would succeed if --allow-unknown-type
- was on by default, let's fix that by asserting that -t and -s die on a
- "garbage" type without --allow-unknown-type.
+ The --allow-unknown-type flag then changes whether we pass the
+ "OBJECT_INFO_ALLOW_UNKNOWN_TYPE" flag to get_oid_with_context() or
+ not.
+
+ The "-p" flag is yet another special-case in printing the same output
+ on the deadbeef OID as we'd emit on the deadbeef_short OID for the
+ "-s" and "-t" options, it also doesn't support the
+ "--allow-unknown-type" flag at all.
+
+ Let's test the combination of the two sets of [-t, -s, -p] and
+ [--{no-}allow-unknown-type] (the --no-allow-unknown-type is implicit
+ in not supplying it), as well as a [missing,bogus] object pair.
+
+ This extends tests added in 3e370f9faf0 (t1006: add tests for git
+ cat-file --allow-unknown-type, 2015-05-03).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## t/oid-info/oid ##
+@@ t/oid-info/oid: numeric sha1:0123456789012345678901234567890123456789
+ numeric sha256:0123456789012345678901234567890123456789012345678901234567890123
+ deadbeef sha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbeef
+ deadbeef sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
++deadbeef_short sha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbee
++deadbee_short sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbee
+
## t/t1006-cat-file.sh ##
-@@ t/t1006-cat-file.sh: bogus_content="bogus"
- bogus_size=$(strlen "$bogus_content")
- bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
+@@ t/t1006-cat-file.sh: test_expect_success 'setup bogus data' '
+ bogus_long_sha1=$(echo_without_newline "$bogus_long_content" | git hash-object -t $bogus_long_type --literally -w --stdin)
+ '
-+test_expect_success 'die on broken object under -t and -s without --allow-unknown-type' '
-+ cat >err.expect <<-\EOF &&
-+ fatal: invalid object type
-+ EOF
++for arg1 in '' --allow-unknown-type
++do
++ for arg2 in -s -t -p
++ do
++ if test $arg1 = "--allow-unknown-type" && test "$arg2" = "-p"
++ then
++ continue
++ fi
++
+
-+ test_must_fail git cat-file -t $bogus_sha1 >out.actual 2>err.actual &&
-+ test_cmp err.expect err.actual &&
-+ test_must_be_empty out.actual &&
++ test_expect_success "cat-file $arg1 $arg2 error on bogus short OID" '
++ cat >expect <<-\EOF &&
++ fatal: invalid object type
++ EOF
+
-+ test_must_fail git cat-file -s $bogus_sha1 >out.actual 2>err.actual &&
-+ test_cmp err.expect err.actual &&
-+ test_must_be_empty out.actual
-+'
++ if test "$arg1" = "--allow-unknown-type"
++ then
++ git cat-file $arg1 $arg2 $bogus_short_sha1
++ else
++ test_must_fail git cat-file $arg1 $arg2 $bogus_short_sha1 >out 2>actual &&
++ test_must_be_empty out &&
++ test_cmp expect actual
++ fi
++ '
++
++ test_expect_success "cat-file $arg1 $arg2 error on bogus full OID" '
++ if test "$arg2" = "-p"
++ then
++ cat >expect <<-EOF
++ error: unable to unpack $bogus_long_sha1 header
++ fatal: Not a valid object name $bogus_long_sha1
++ EOF
++ else
++ cat >expect <<-EOF
++ error: unable to unpack $bogus_long_sha1 header
++ fatal: git cat-file: could not get object info
++ EOF
++ fi &&
++
++ if test "$arg1" = "--allow-unknown-type"
++ then
++ git cat-file $arg1 $arg2 $bogus_short_sha1
++ else
++ test_must_fail git cat-file $arg1 $arg2 $bogus_long_sha1 >out 2>actual &&
++ test_must_be_empty out &&
++ test_cmp expect actual
++ fi
++ '
++
++ test_expect_success "cat-file $arg1 $arg2 error on missing short OID" '
++ cat >expect.err <<-EOF &&
++ fatal: Not a valid object name $(test_oid deadbeef_short)
++ EOF
++ test_must_fail git cat-file $arg1 $arg2 $(test_oid deadbeef_short) >out 2>err.actual &&
++ test_must_be_empty out
++ '
++
++ test_expect_success "cat-file $arg1 $arg2 error on missing full OID" '
++ if test "$arg2" = "-p"
++ then
++ cat >expect.err <<-EOF
++ fatal: Not a valid object name $(test_oid deadbeef)
++ EOF
++ else
++ cat >expect.err <<-\EOF
++ fatal: git cat-file: could not get object info
++ EOF
++ fi &&
++ test_must_fail git cat-file $arg1 $arg2 $(test_oid deadbeef) >out 2>err.actual &&
++ test_must_be_empty out &&
++ test_cmp expect.err err.actual
++ '
++ done
++done
+
test_expect_success "Type of broken object is correct" '
- echo $bogus_type >expect &&
- git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
-@@ t/t1006-cat-file.sh: bogus_content="bogus"
- bogus_size=$(strlen "$bogus_content")
- bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
-
-+test_expect_success 'die on broken object with large type under -t and -s without --allow-unknown-type' '
-+ cat >err.expect <<-EOF &&
-+ error: unable to unpack $bogus_sha1 header
-+ fatal: git cat-file: could not get object info
-+ EOF
-+
-+ test_must_fail git cat-file -t $bogus_sha1 >out.actual 2>err.actual &&
-+ test_cmp err.expect err.actual &&
-+ test_must_be_empty out.actual &&
-+
-+ test_must_fail git cat-file -s $bogus_sha1 >out.actual 2>err.actual &&
-+ test_cmp err.expect err.actual &&
-+ test_must_be_empty out.actual
-+'
-+
- test_expect_success "Type of broken object is correct when type is large" '
- echo $bogus_type >expect &&
- git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
+ echo $bogus_short_type >expect &&
+ git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
6: d1ffd21acc5 = 7: 20bd81c1af0 cat-file tests: add corrupt loose object test
7: 22ab12c2282 ! 8: cd1d52b8a07 cat-file tests: test for current --allow-unknown-type behavior
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1006-cat-file.sh ##
-@@ t/t1006-cat-file.sh: test_expect_success 'die on broken object under -t and -s without --allow-unknow
- test_must_be_empty out.actual
- '
+@@ t/t1006-cat-file.sh: do
+ done
+ done
+test_expect_success '-e is OK with a broken object without --allow-unknown-type' '
-+ git cat-file -e $bogus_sha1
++ git cat-file -e $bogus_short_sha1
+'
+
+test_expect_success '-e can not be combined with --allow-unknown-type' '
-+ test_expect_code 128 git cat-file -e --allow-unknown-type $bogus_sha1
++ test_expect_code 128 git cat-file -e --allow-unknown-type $bogus_short_sha1
+'
+
+test_expect_success '-p cannot print a broken object even with --allow-unknown-type' '
-+ test_must_fail git cat-file -p $bogus_sha1 &&
-+ test_expect_code 128 git cat-file -p --allow-unknown-type $bogus_sha1
++ test_must_fail git cat-file -p $bogus_short_sha1 &&
++ test_expect_code 128 git cat-file -p --allow-unknown-type $bogus_short_sha1
+'
+
+test_expect_success '<type> <hash> does not work with objects of broken types' '
+ cat >err.expect <<-\EOF &&
+ fatal: invalid object type "bogus"
+ EOF
-+ test_must_fail git cat-file $bogus_type $bogus_sha1 2>err.actual &&
++ test_must_fail git cat-file $bogus_short_type $bogus_short_sha1 2>err.actual &&
+ test_cmp err.expect err.actual
+'
+
+test_expect_success 'broken types combined with --batch and --batch-check' '
-+ echo $bogus_sha1 >bogus-oid &&
++ echo $bogus_short_sha1 >bogus-oid &&
+
+ cat >err.expect <<-\EOF &&
+ fatal: invalid object type
@@ t/t1006-cat-file.sh: test_expect_success 'die on broken object under -t and -s w
+ test_expect_code 128 git cat-file --batch --allow-unknown-type <bogus-oid &&
+ test_expect_code 128 git cat-file --batch-check --allow-unknown-type <bogus-oid
+'
-+
- test_expect_success "Type of broken object is correct" '
- echo $bogus_type >expect &&
- git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
-@@ t/t1006-cat-file.sh: test_expect_success "Size of broken object is correct" '
- git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
- test_cmp expect actual
- '
+
+test_expect_success 'the --allow-unknown-type option does not consider replacement refs' '
+ cat >expect <<-EOF &&
-+ $bogus_type
++ $bogus_short_type
+ EOF
-+ git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
++ git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
+ test_cmp expect actual &&
+
+ # Create it manually, as "git replace" will die on bogus
+ # types.
+ head=$(git rev-parse --verify HEAD) &&
++ test_when_finished "rm -rf .git/refs/replace" &&
+ mkdir -p .git/refs/replace &&
-+ echo $head >.git/refs/replace/$bogus_sha1 &&
++ echo $head >.git/refs/replace/$bogus_short_sha1 &&
+
+ cat >expect <<-EOF &&
+ commit
+ EOF
-+ git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
++ git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
+ test_cmp expect actual
+'
+
- bogus_type="abcdefghijklmnopqrstuvwxyz1234679"
- bogus_content="bogus"
- bogus_size=$(strlen "$bogus_content")
+ test_expect_success "Type of broken object is correct" '
+ echo $bogus_short_type >expect &&
+ git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
8: 38e4266772d = 9: d9f5adfc74b object-file.c: don't set "typep" when returning non-zero
9: 5b9278e7bb4 < -: ----------- cache.h: move object functions to object-store.h
16: 9e7dbfb4aa3 ! 10: 51d14bc9274 object-file.c: return -1, not "status" from unpack_loose_header()
@@ Commit message
## object-file.c ##
@@ object-file.c: int unpack_loose_header(git_zstream *stream,
- status = git_inflate(stream, 0);
- obj_read_lock();
+ buffer, bufsiz);
+
if (status < Z_OK)
- return status;
+ return -1;
- /*
- * Check if entire header is unpacked in the first iteration.
+ /* Make sure we have the terminating NUL */
+ if (!memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
10: b15ad53414b ! 11: f43cfd8a5ed object-file.c: make parse_loose_header_extended() public
@@ Commit message
could simply use parse_loose_header_extended() there, but will leave
the API in a better end state.
+ It would be a better end-state to have already moved the declaration
+ of these functions to object-store.h to avoid the forward declaration
+ of "struct object_info" in cache.h, but let's leave that cleanup for
+ some other time.
+
+ 1. https://lore.kernel.org/git/patch-v6-09.22-5b9278e7bb4-20210907T104559Z-avarab@gmail.com/
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## cache.h ##
+@@ cache.h: char *xdg_cache_home(const char *filename);
+ int git_open_cloexec(const char *name, int flags);
+ #define git_open(name) git_open_cloexec(name, O_RDONLY)
+ int unpack_loose_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
+-int parse_loose_header(const char *hdr, unsigned long *sizep);
++struct object_info;
++int parse_loose_header(const char *hdr, struct object_info *oi,
++ unsigned int flags);
+
+ int check_object_signature(struct repository *r, const struct object_id *oid,
+ void *buf, unsigned long size, const char *type);
+
## object-file.c ##
@@ object-file.c: static void *unpack_loose_rest(git_zstream *stream,
* too permissive for what we want to check. So do an anal
@@ object-file.c: static void *unpack_loose_rest(git_zstream *stream,
*/
-static int parse_loose_header_extended(const char *hdr, struct object_info *oi,
- unsigned int flags)
-+int parse_loose_header(const char *hdr,
-+ struct object_info *oi,
++int parse_loose_header(const char *hdr, struct object_info *oi,
+ unsigned int flags)
{
const char *type_buf = hdr;
@@ object-file.c: int read_loose_object(const char *path,
error(_("unable to parse header of %s"), path);
git_inflate_end(&stream);
- ## object-store.h ##
-@@ object-store.h: int for_each_packed_object(each_packed_object_fn, void *,
- int unpack_loose_header(git_zstream *stream, unsigned char *map,
- unsigned long mapsize, void *buffer,
- unsigned long bufsiz);
--int parse_loose_header(const char *hdr, unsigned long *sizep);
-+int parse_loose_header(const char *hdr, struct object_info *oi,
-+ unsigned int flags);
- int check_object_signature(struct repository *r, const struct object_id *oid,
- void *buf, unsigned long size, const char *type);
- int finalize_object_file(const char *tmpfile, const char *filename);
-
## streaming.c ##
@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct repository *r,
const struct object_id *oid,
11: 326eb74545d < -: ----------- object-file.c: add missing braces to loose_object_info()
12: 4f829e9b727 ! 12: 50d938f7f3c object-file.c: simplify unpack_loose_short_header()
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## cache.h ##
+@@ cache.h: char *xdg_cache_home(const char *filename);
+
+ int git_open_cloexec(const char *name, int flags);
+ #define git_open(name) git_open_cloexec(name, O_RDONLY)
+-int unpack_loose_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
++
++/**
++ * unpack_loose_header() initializes the data stream needed to unpack
++ * a loose object header.
++ *
++ * Returns 0 on success. Returns negative values on error.
++ *
++ * It will only parse up to MAX_HEADER_LEN bytes unless an optional
++ * "hdrbuf" argument is non-NULL. This is intended for use with
++ * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
++ * reporting. The full header will be extracted to "hdrbuf" for use
++ * with parse_loose_header().
++ */
++int unpack_loose_header(git_zstream *stream, unsigned char *map,
++ unsigned long mapsize, void *buffer,
++ unsigned long bufsiz, struct strbuf *hdrbuf);
+ struct object_info;
+ int parse_loose_header(const char *hdr, struct object_info *oi,
+ unsigned int flags);
+
## object-file.c ##
@@ object-file.c: void *map_loose_object(struct repository *r,
return map_loose_object_1(r, NULL, oid, size);
@@ object-file.c: static int unpack_loose_short_header(git_zstream *stream,
- int status = unpack_loose_short_header(stream, map, mapsize,
- buffer, bufsiz);
-
- if (status < Z_OK)
- return status;
-
+- if (status < Z_OK)
+- return -1;
+-
- /* Make sure we have the terminating NUL */
- if (!memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
- return -1;
@@ object-file.c: static int unpack_loose_short_header(git_zstream *stream,
- int status;
-
- status = unpack_loose_short_header(stream, map, mapsize, buffer, bufsiz);
-- if (status < Z_OK)
-- return -1;
--
- /*
- * Check if entire header is unpacked in the first iteration.
- */
+ if (status < Z_OK)
+ return -1;
+
+@@ object-file.c: static int unpack_loose_header_to_strbuf(git_zstream *stream, unsigned char *map
if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
return 0;
@@ object-file.c: static int unpack_loose_short_header(git_zstream *stream,
* buffer[0..bufsiz] was not large enough. Copy the partial
* result out to header, and then append the result of further
@@ object-file.c: static int loose_object_info(struct repository *r,
- unsigned long mapsize;
- void *map;
- git_zstream stream;
-+ int hdr_ret;
char hdr[MAX_HEADER_LEN];
struct strbuf hdrbuf = STRBUF_INIT;
unsigned long size_scratch;
@@ object-file.c: static int loose_object_info(struct repository *r,
- if (unpack_loose_header_to_strbuf(&stream, map, mapsize, hdr, sizeof(hdr), &hdrbuf) < 0)
- status = error(_("unable to unpack %s header with --allow-unknown-type"),
- oid_to_hex(oid));
-- } else if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0) {
+- } else if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
+
-+ hdr_ret = unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
-+ allow_unknown ? &hdrbuf : NULL);
-+ if (hdr_ret < 0) {
++ if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
++ allow_unknown ? &hdrbuf : NULL) < 0)
status = error(_("unable to unpack %s header"),
oid_to_hex(oid));
- }
+ if (status < 0)
@@ object-file.c: int read_loose_object(const char *path,
goto out;
}
@@ object-file.c: int read_loose_object(const char *path,
goto out;
}
- ## object-store.h ##
-@@ object-store.h: int for_each_object_in_pack(struct packed_git *p,
- int for_each_packed_object(each_packed_object_fn, void *,
- enum for_each_object_flags flags);
-
-+/**
-+ * unpack_loose_header() initializes the data stream needed to unpack
-+ * a loose object header.
-+ *
-+ * Returns 0 on success. Returns negative values on error.
-+ *
-+ * It will only parse up to MAX_HEADER_LEN bytes unless an optional
-+ * "hdrbuf" argument is non-NULL. This is intended for use with
-+ * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
-+ * reporting. The full header will be extracted to "hdrbuf" for use
-+ * with parse_loose_header().
-+ */
- int unpack_loose_header(git_zstream *stream, unsigned char *map,
- unsigned long mapsize, void *buffer,
-- unsigned long bufsiz);
-+ unsigned long bufsiz, struct strbuf *hdrbuf);
- int parse_loose_header(const char *hdr, struct object_info *oi,
- unsigned int flags);
- int check_object_signature(struct repository *r, const struct object_id *oid,
-
## streaming.c ##
@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct repository *r,
st->u.loose.mapped,
13: 90489d9e6ec < -: ----------- object-file.c: split up ternary in parse_loose_header()
18: 1b7173a5b5b ! 13: 755fde00b46 object-file.c: use "enum" return type for unpack_loose_header()
@@ Metadata
## Commit message ##
object-file.c: use "enum" return type for unpack_loose_header()
- In the preceding commits we changed and documented
- unpack_loose_header() from return any negative value or zero, to only
- -2, -1 or 0. Let's instead add an "enum unpack_loose_header_result"
- type and use it, and have the compiler assert that we're exhaustively
- covering all return values. This gets rid of the need for having a
- "default" BUG() case in loose_object_info().
+ In a preceding commit we changed and documented unpack_loose_header()
+ from its previous behavior of returning any negative value or zero, to
+ only -1 or 0.
- I'm on the fence about whether this is more readable or worth it, but
- since it was suggested in [1] to do this let's go for it.
-
- 1. https://lore.kernel.org/git/20210527175433.2673306-1-jonathantanmy@google.com/
+ Let's add an "enum unpack_loose_header_result" type and use it for
+ these return values, and have the compiler assert that we're
+ exhaustively covering all of them.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## cache.h ##
+@@ cache.h: int git_open_cloexec(const char *name, int flags);
+ * unpack_loose_header() initializes the data stream needed to unpack
+ * a loose object header.
+ *
+- * Returns 0 on success. Returns negative values on error.
++ * Returns:
++ *
++ * - ULHR_OK on success
++ * - ULHR_BAD on error
+ *
+ * It will only parse up to MAX_HEADER_LEN bytes unless an optional
+ * "hdrbuf" argument is non-NULL. This is intended for use with
+@@ cache.h: int git_open_cloexec(const char *name, int flags);
+ * reporting. The full header will be extracted to "hdrbuf" for use
+ * with parse_loose_header().
+ */
+-int unpack_loose_header(git_zstream *stream, unsigned char *map,
+- unsigned long mapsize, void *buffer,
+- unsigned long bufsiz, struct strbuf *hdrbuf);
++enum unpack_loose_header_result {
++ ULHR_OK,
++ ULHR_BAD,
++};
++enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
++ unsigned char *map,
++ unsigned long mapsize,
++ void *buffer,
++ unsigned long bufsiz,
++ struct strbuf *hdrbuf);
++
+ struct object_info;
+ int parse_loose_header(const char *hdr, struct object_info *oi,
+ unsigned int flags);
+
## object-file.c ##
@@ object-file.c: void *map_loose_object(struct repository *r,
return map_loose_object_1(r, NULL, oid, size);
@@ object-file.c: void *map_loose_object(struct repository *r,
{
int status;
+@@ object-file.c: int unpack_loose_header(git_zstream *stream,
+ status = git_inflate(stream, 0);
+ obj_read_lock();
+ if (status < Z_OK)
+- return -1;
++ return ULHR_BAD;
+
+ /*
+ * Check if entire header is unpacked in the first iteration.
+ */
+ if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
+- return 0;
++ return ULHR_OK;
+
+ /*
+ * We have a header longer than MAX_HEADER_LEN. The "header"
+@@ object-file.c: int unpack_loose_header(git_zstream *stream,
+ * --allow-unknown-type".
+ */
+ if (!header)
+- return -1;
++ return ULHR_BAD;
+
+ /*
+ * buffer[0..bufsiz] was not large enough. Copy the partial
+@@ object-file.c: int unpack_loose_header(git_zstream *stream,
+ stream->next_out = buffer;
+ stream->avail_out = bufsiz;
+ } while (status != Z_STREAM_END);
+- return -1;
++ return ULHR_BAD;
+ }
+
+ static void *unpack_loose_rest(git_zstream *stream,
@@ object-file.c: static int loose_object_info(struct repository *r,
- unsigned long mapsize;
- void *map;
- git_zstream stream;
-- int hdr_ret;
-+ enum unpack_loose_header_result hdr_ret;
- char hdr[MAX_HEADER_LEN];
- struct strbuf hdrbuf = STRBUF_INIT;
- unsigned long size_scratch;
-@@ object-file.c: static int loose_object_info(struct repository *r,
- hdr_ret = unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
- allow_unknown ? &hdrbuf : NULL);
- switch (hdr_ret) {
-- case 0:
-+ case UNPACK_LOOSE_HEADER_RESULT_OK:
- break;
-- case -1:
-+ case UNPACK_LOOSE_HEADER_RESULT_BAD:
+ if (oi->disk_sizep)
+ *oi->disk_sizep = mapsize;
+
+- if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
+- allow_unknown ? &hdrbuf : NULL) < 0)
++ switch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
++ allow_unknown ? &hdrbuf : NULL)) {
++ case ULHR_OK:
++ break;
++ case ULHR_BAD:
status = error(_("unable to unpack %s header"),
oid_to_hex(oid));
- break;
-- case -2:
-+ case UNPACK_LOOSE_HEADER_RESULT_BAD_TOO_LONG:
- status = error(_("header for %s too long, exceeds %d bytes"),
- oid_to_hex(oid), MAX_HEADER_LEN);
- break;
-- default:
-- BUG("unknown hdr_ret value %d", hdr_ret);
- }
- if (!status) {
- if (!parse_loose_header(hdrbuf.len ? hdrbuf.buf : hdr, oi))
-
- ## object-store.h ##
-@@ object-store.h: int for_each_object_in_pack(struct packed_git *p,
- int for_each_packed_object(each_packed_object_fn, void *,
- enum for_each_object_flags flags);
-
-+enum unpack_loose_header_result {
-+ UNPACK_LOOSE_HEADER_RESULT_BAD_TOO_LONG = -2,
-+ UNPACK_LOOSE_HEADER_RESULT_BAD = -1,
-+ UNPACK_LOOSE_HEADER_RESULT_OK,
-+
-+};
+- if (status < 0)
+- ; /* Do nothing */
+- else if (hdrbuf.len) {
++ break;
++ }
+
- /**
- * unpack_loose_header() initializes the data stream needed to unpack
- * a loose object header.
- *
-- * Returns 0 on success. Returns negative values on error. If the
-- * header exceeds MAX_HEADER_LEN -2 will be returned.
-+ * Returns UNPACK_LOOSE_HEADER_RESULT_OK on success. Returns
-+ * UNPACK_LOOSE_HEADER_RESULT_BAD values on error, or if the header
-+ * exceeds MAX_HEADER_LEN UNPACK_LOOSE_HEADER_RESULT_BAD_TOO_LONG will
-+ * be returned.
- *
- * It will only parse up to MAX_HEADER_LEN bytes unless an optional
- * "hdrbuf" argument is non-NULL. This is intended for use with
- * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
- * reporting. The full header will be extracted to "hdrbuf" for use
-- * with parse_loose_header(), -2 will still be returned from this
-- * function to indicate that the header was too long.
-+ * with parse_loose_header(), UNPACK_LOOSE_HEADER_RESULT_BAD_TOO_LONG
-+ * will still be returned from this function to indicate that the
-+ * header was too long.
- */
--int unpack_loose_header(git_zstream *stream, unsigned char *map,
-- unsigned long mapsize, void *buffer,
-- unsigned long bufsiz, struct strbuf *hdrbuf);
-+enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
-+ unsigned char *map,
-+ unsigned long mapsize,
-+ void *buffer,
-+ unsigned long bufsiz,
-+ struct strbuf *hdrbuf);
-
- /**
- * parse_loose_header() parses the starting "<type> <len>\0" of an
++ if (status < 0) {
++ /* Do nothing */
++ } else if (hdrbuf.len) {
+ if ((status = parse_loose_header(hdrbuf.buf, oi, flags)) < 0)
+ status = error(_("unable to parse %s header with --allow-unknown-type"),
+ oid_to_hex(oid));
## streaming.c ##
@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct repository *r,
- enum object_type *type)
- {
- struct object_info oi = OBJECT_INFO_INIT;
-+ enum unpack_loose_header_result hdr_ret;
- oi.sizep = &st->size;
- oi.typep = type;
-
st->u.loose.mapped = map_loose_object(r, oid, &st->u.loose.mapsize);
if (!st->u.loose.mapped)
return -1;
@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct reposi
- st->u.loose.hdr,
- sizeof(st->u.loose.hdr),
- NULL) < 0) ||
-- (parse_loose_header(st->u.loose.hdr, &oi) < 0) ||
-- *type < 0) {
+- (parse_loose_header(st->u.loose.hdr, &oi, 0) < 0)) {
- git_inflate_end(&st->z);
- munmap(st->u.loose.mapped, st->u.loose.mapsize);
- return -1;
-+ hdr_ret = unpack_loose_header(&st->z, st->u.loose.mapped,
-+ st->u.loose.mapsize, st->u.loose.hdr,
-+ sizeof(st->u.loose.hdr), NULL);
-+ switch (hdr_ret) {
-+ case UNPACK_LOOSE_HEADER_RESULT_OK:
++ switch (unpack_loose_header(&st->z, st->u.loose.mapped,
++ st->u.loose.mapsize, st->u.loose.hdr,
++ sizeof(st->u.loose.hdr), NULL)) {
++ case ULHR_OK:
+ break;
-+ case UNPACK_LOOSE_HEADER_RESULT_BAD:
-+ case UNPACK_LOOSE_HEADER_RESULT_BAD_TOO_LONG:
++ case ULHR_BAD:
+ goto error;
}
-+ if (parse_loose_header(st->u.loose.hdr, &oi) < 0 || *type < 0)
++ if (parse_loose_header(st->u.loose.hdr, &oi, 0) < 0)
+ goto error;
st->u.loose.hdr_used = strlen(st->u.loose.hdr) + 1;
17: f28c4f0dfb4 ! 14: 522d71eb19d object-file.c: return -2 on "header too long" in unpack_loose_header()
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- object-file.c: return -2 on "header too long" in unpack_loose_header()
+ object-file.c: return ULHR_TOO_LONG on "header too long"
Split up the return code for "header too long" from the generic
negative return value unpack_loose_header() returns, and report via
@@ Commit message
As a test added earlier in this series in t1006-cat-file.sh shows
we'll correctly emit zlib errors from zlib.c already in this case, so
we have no need to carry those return codes further down the
- stack. Let's instead just return -2 saying we ran into the
+ stack. Let's instead just return ULHR_TOO_LONG saying we ran into the
MAX_HEADER_LEN limit, or other negative values for "unable to unpack
<OID> header".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## cache.h ##
+@@ cache.h: int git_open_cloexec(const char *name, int flags);
+ *
+ * - ULHR_OK on success
+ * - ULHR_BAD on error
++ * - ULHR_TOO_LONG if the header was too long
+ *
+ * It will only parse up to MAX_HEADER_LEN bytes unless an optional
+ * "hdrbuf" argument is non-NULL. This is intended for use with
+ * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
+ * reporting. The full header will be extracted to "hdrbuf" for use
+- * with parse_loose_header().
++ * with parse_loose_header(), ULHR_TOO_LONG will still be returned
++ * from this function to indicate that the header was too long.
+ */
+ enum unpack_loose_header_result {
+ ULHR_OK,
+ ULHR_BAD,
++ ULHR_TOO_LONG,
+ };
+ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
+ unsigned char *map,
+
## object-file.c ##
-@@ object-file.c: int unpack_loose_header(git_zstream *stream,
+@@ object-file.c: enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
* --allow-unknown-type".
*/
if (!header)
-- return -1;
-+ return -2;
+- return ULHR_BAD;
++ return ULHR_TOO_LONG;
/*
* buffer[0..bufsiz] was not large enough. Copy the partial
-@@ object-file.c: int unpack_loose_header(git_zstream *stream,
+@@ object-file.c: enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
stream->next_out = buffer;
stream->avail_out = bufsiz;
} while (status != Z_STREAM_END);
-- return -1;
-+ return -2;
+- return ULHR_BAD;
++ return ULHR_TOO_LONG;
}
static void *unpack_loose_rest(git_zstream *stream,
@@ object-file.c: static int loose_object_info(struct repository *r,
-
- hdr_ret = unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
- allow_unknown ? &hdrbuf : NULL);
-- if (hdr_ret < 0) {
-+ switch (hdr_ret) {
-+ case 0:
-+ break;
-+ case -1:
status = error(_("unable to unpack %s header"),
oid_to_hex(oid));
-+ break;
-+ case -2:
+ break;
++ case ULHR_TOO_LONG:
+ status = error(_("header for %s too long, exceeds %d bytes"),
+ oid_to_hex(oid), MAX_HEADER_LEN);
+ break;
-+ default:
-+ BUG("unknown hdr_ret value %d", hdr_ret);
}
- if (!status) {
- if (!parse_loose_header(hdrbuf.len ? hdrbuf.buf : hdr, oi))
+
+ if (status < 0) {
- ## object-store.h ##
-@@ object-store.h: int for_each_packed_object(each_packed_object_fn, void *,
- * unpack_loose_header() initializes the data stream needed to unpack
- * a loose object header.
- *
-- * Returns 0 on success. Returns negative values on error.
-+ * Returns 0 on success. Returns negative values on error. If the
-+ * header exceeds MAX_HEADER_LEN -2 will be returned.
- *
- * It will only parse up to MAX_HEADER_LEN bytes unless an optional
- * "hdrbuf" argument is non-NULL. This is intended for use with
- * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
- * reporting. The full header will be extracted to "hdrbuf" for use
-- * with parse_loose_header().
-+ * with parse_loose_header(), -2 will still be returned from this
-+ * function to indicate that the header was too long.
- */
- int unpack_loose_header(git_zstream *stream, unsigned char *map,
- unsigned long mapsize, void *buffer,
+ ## streaming.c ##
+@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct repository *r,
+ case ULHR_OK:
+ break;
+ case ULHR_BAD:
++ case ULHR_TOO_LONG:
+ goto error;
+ }
+ if (parse_loose_header(st->u.loose.hdr, &oi, 0) < 0)
## t/t1006-cat-file.sh ##
-@@ t/t1006-cat-file.sh: bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_t
-
- test_expect_success 'die on broken object with large type under -t and -s without --allow-unknown-type' '
- cat >err.expect <<-EOF &&
-- error: unable to unpack $bogus_sha1 header
-+ error: header for $bogus_sha1 too long, exceeds 32 bytes
- fatal: git cat-file: could not get object info
- EOF
-
+@@ t/t1006-cat-file.sh: do
+ if test "$arg2" = "-p"
+ then
+ cat >expect <<-EOF
+- error: unable to unpack $bogus_long_sha1 header
++ error: header for $bogus_long_sha1 too long, exceeds 32 bytes
+ fatal: Not a valid object name $bogus_long_sha1
+ EOF
+ else
+ cat >expect <<-EOF
+- error: unable to unpack $bogus_long_sha1 header
++ error: header for $bogus_long_sha1 too long, exceeds 32 bytes
+ fatal: git cat-file: could not get object info
+ EOF
+ fi &&
14: 7c9819d37c5 ! 15: 1ca875395c1 object-file.c: stop dying in parse_loose_header()
@@ Metadata
## Commit message ##
object-file.c: stop dying in parse_loose_header()
- Start the libification of parse_loose_header() by making it return
- error codes and data instead of invoking die() by itself. For now
- we'll move the relevant die() call to loose_object_info() and
- read_loose_object() to keep this change smaller, but in subsequent
- commits we'll also libify those.
+ Make parse_loose_header() return error codes and data instead of
+ invoking die() by itself.
- Since the refactoring of parse_loose_header_extended() into
- parse_loose_header() in an earlier commit, its interface accepts a
- "unsigned long *sizep". Rather it accepts a "struct object_info *",
- that structure will be populated with information about the object.
+ For now we'll move the relevant die() call to loose_object_info() and
+ read_loose_object() to keep this change smaller. In a subsequent
+ commit we'll make read_loose_object() return an error code instead of
+ dying. We should also address the "allow_unknown" case (should be
+ moved to builtin/cat-file.c), but for now I'll be leaving it.
- It thus makes sense to further libify the interface so that it stops
- calling die() when it encounters OBJ_BAD, and instead rely on its
- callers to check the populated "oi->typep".
+ For making parse_loose_header() not die() change its prototype to
+ accept a "struct object_info *" instead of the "unsigned long *sizep"
+ it accepted before. Its callers can now check the populated populated
+ "oi->typep".
Because of this we don't need to pass in the "unsigned int flags"
which we used for OBJECT_INFO_ALLOW_UNKNOWN_TYPE, we can instead do
@@ Commit message
variable. In some cases we set it to the return value of "error()",
i.e. -1, and later checked if "status < 0" was true.
- In another case added in c84a1f3ed4d (sha1_file: refactor read_object,
- 2017-06-21) (but the behavior pre-dated that) we did checks of "status
- >= 0", because at that point "status" had become the return value of
- parse_loose_header(). I.e. a non-negative "enum object_type" (unless
- we -1, aka. OBJ_BAD).
+ Since 93cff9a978e (sha1_loose_object_info: return error for corrupted
+ objects, 2017-04-01) the return value of loose_object_info() (then
+ named sha1_loose_object_info()) had been a "status" variable that be
+ any negative value, as we were expecting to return the "enum
+ object_type".
- Now that parse_loose_header() will return 0 on success instead of the
+ The only negative type happens to be OBJ_BAD, but the code still
+ assumed that more might be added. This was then used later in
+ e.g. c84a1f3ed4d (sha1_file: refactor read_object, 2017-06-21). Now
+ that parse_loose_header() will return 0 on success instead of the
type (which it'll stick into the "struct object_info") we don't need
to conflate these two cases in its callers.
+ Since parse_loose_header() doesn't need to return an arbitrary
+ "status" we only need to treat its "ret < 0" specially, but can
+ idiomatically overwrite it with our own error() return. This along
+ with having made unpack_loose_header() return an "enum
+ unpack_loose_header_result" in an earlier commit means that we can
+ move the previously nested if/else cases mostly into the "ULHR_OK"
+ branch of the "switch" statement.
+
+ We should be less silent if we reach that "status = -1" branch, which
+ happens if we've got trailing garbage in loose objects, see
+ f6371f92104 (sha1_file: add read_loose_object() function, 2017-01-13)
+ for a better way to handle it. For now let's punt on it, a subsequent
+ commit will address that edge case.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ ## cache.h ##
+@@ cache.h: enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
+ unsigned long bufsiz,
+ struct strbuf *hdrbuf);
+
++/**
++ * parse_loose_header() parses the starting "<type> <len>\0" of an
++ * object. If it doesn't follow that format -1 is returned. To check
++ * the validity of the <type> populate the "typep" in the "struct
++ * object_info". It will be OBJ_BAD if the object type is unknown. The
++ * parsed <len> can be retrieved via "oi->sizep", and from there
++ * passed to unpack_loose_rest().
++ */
+ struct object_info;
+-int parse_loose_header(const char *hdr, struct object_info *oi,
+- unsigned int flags);
++int parse_loose_header(const char *hdr, struct object_info *oi);
+
+ int check_object_signature(struct repository *r, const struct object_id *oid,
+ void *buf, unsigned long size, const char *type);
+
## object-file.c ##
@@ object-file.c: static void *unpack_loose_rest(git_zstream *stream,
* too permissive for what we want to check. So do an anal
* object header parse by hand.
*/
--int parse_loose_header(const char *hdr,
-- struct object_info *oi,
+-int parse_loose_header(const char *hdr, struct object_info *oi,
- unsigned int flags)
+int parse_loose_header(const char *hdr, struct object_info *oi)
{
const char *type_buf = hdr;
unsigned long size;
-@@ object-file.c: int parse_loose_header(const char *hdr,
+@@ object-file.c: int parse_loose_header(const char *hdr, struct object_info *oi,
type = type_from_string_gently(type_buf, type_len, 1);
if (oi->type_name)
strbuf_add(oi->type_name, type_buf, type_len);
@@ object-file.c: int parse_loose_header(const char *hdr,
if (oi->typep)
*oi->typep = type;
-@@ object-file.c: int parse_loose_header(const char *hdr,
- if (*hdr)
- return -1;
-
-- return type;
+@@ object-file.c: int parse_loose_header(const char *hdr, struct object_info *oi,
+ /*
+ * The length must be followed by a zero byte
+ */
+- return *hdr ? -1 : type;
++ if (*hdr)
++ return -1;
++
+ /*
+ * The format is valid, but the type may still be bogus. The
+ * Caller needs to check its oi->typep.
@@ object-file.c: static int loose_object_info(struct repository *r,
struct strbuf hdrbuf = STRBUF_INIT;
unsigned long size_scratch;
+ enum object_type type_scratch;
-+ int parsed_header = 0;
int allow_unknown = flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
if (oi->delta_base_oid)
@@ object-file.c: static int loose_object_info(struct repository *r,
if (oi->disk_sizep)
*oi->disk_sizep = mapsize;
@@ object-file.c: static int loose_object_info(struct repository *r,
+ switch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),
+ allow_unknown ? &hdrbuf : NULL)) {
+ case ULHR_OK:
++ if (parse_loose_header(hdrbuf.len ? hdrbuf.buf : hdr, oi) < 0)
++ status = error(_("unable to parse %s header"), oid_to_hex(oid));
++ else if (!allow_unknown && *oi->typep < 0)
++ die(_("invalid object type"));
++
++ if (!oi->contentp)
++ break;
++ *oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);
++ if (*oi->contentp)
++ goto cleanup;
++
++ status = -1;
+ break;
+ case ULHR_BAD:
status = error(_("unable to unpack %s header"),
- oid_to_hex(oid));
+@@ object-file.c: static int loose_object_info(struct repository *r,
+ break;
}
--
+
- if (status < 0) {
- /* Do nothing */
- } else if (hdrbuf.len) {
- if ((status = parse_loose_header(hdrbuf.buf, oi, flags)) < 0)
- status = error(_("unable to parse %s header with --allow-unknown-type"),
- oid_to_hex(oid));
-- } else if ((status = parse_loose_header(hdr, oi, flags)) < 0) {
+- } else if ((status = parse_loose_header(hdr, oi, flags)) < 0)
- status = error(_("unable to parse %s header"), oid_to_hex(oid));
-+ if (!status) {
-+ if (!parse_loose_header(hdrbuf.len ? hdrbuf.buf : hdr, oi))
-+ /*
-+ * oi->{sizep,typep} are meaningless unless
-+ * parse_loose_header() returns >= 0.
-+ */
-+ parsed_header = 1;
-+ else
-+ status = error(_("unable to parse %s header"), oid_to_hex(oid));
- }
-+ if (!allow_unknown && parsed_header && *oi->typep < 0)
-+ die(_("invalid object type"));
-
+-
- if (status >= 0 && oi->contentp) {
-+ if (parsed_header && oi->contentp) {
- *oi->contentp = unpack_loose_rest(&stream, hdr,
- *oi->sizep, oid);
- if (!*oi->contentp) {
-@@ object-file.c: static int loose_object_info(struct repository *r,
+- *oi->contentp = unpack_loose_rest(&stream, hdr,
+- *oi->sizep, oid);
+- if (!*oi->contentp) {
+- git_inflate_end(&stream);
+- status = -1;
+- }
+- } else
+- git_inflate_end(&stream);
+-
++ git_inflate_end(&stream);
++cleanup:
+ munmap(map, mapsize);
if (oi->sizep == &size_scratch)
oi->sizep = NULL;
strbuf_release(&hdrbuf);
+ if (oi->typep == &type_scratch)
+ oi->typep = NULL;
oi->whence = OI_LOOSE;
- return (status < 0) ? status : 0;
+- return (status < 0) ? status : 0;
++ return status;
}
+
+ int obj_read_use_lock = 0;
@@ object-file.c: int read_loose_object(const char *path,
git_zstream stream;
char hdr[MAX_HEADER_LEN];
@@ object-file.c: int read_loose_object(const char *path,
if (*type == OBJ_BLOB && *size > big_file_threshold) {
if (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)
- ## object-store.h ##
-@@ object-store.h: int for_each_packed_object(each_packed_object_fn, void *,
- int unpack_loose_header(git_zstream *stream, unsigned char *map,
- unsigned long mapsize, void *buffer,
- unsigned long bufsiz, struct strbuf *hdrbuf);
--int parse_loose_header(const char *hdr, struct object_info *oi,
-- unsigned int flags);
-+
-+/**
-+ * parse_loose_header() parses the starting "<type> <len>\0" of an
-+ * object. If it doesn't follow that format -1 is returned. To check
-+ * the validity of the <type> populate the "typep" in the "struct
-+ * object_info". It will be OBJ_BAD if the object type is unknown. The
-+ * parsed <len> can be retrieved via "oi->sizep", and from there
-+ * passed to unpack_loose_rest().
-+ */
-+int parse_loose_header(const char *hdr, struct object_info *oi);
-+
- int check_object_signature(struct repository *r, const struct object_id *oid,
- void *buf, unsigned long size, const char *type);
- int finalize_object_file(const char *tmpfile, const char *filename);
-
## streaming.c ##
@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct repository *r,
{
@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct reposi
st->u.loose.mapped = map_loose_object(r, oid, &st->u.loose.mapsize);
if (!st->u.loose.mapped)
@@ streaming.c: static int open_istream_loose(struct git_istream *st, struct repository *r,
- st->u.loose.hdr,
- sizeof(st->u.loose.hdr),
- NULL) < 0) ||
-- (parse_loose_header(st->u.loose.hdr, &oi, 0) < 0)) {
-+ (parse_loose_header(st->u.loose.hdr, &oi) < 0) ||
-+ *type < 0) {
- git_inflate_end(&st->z);
- munmap(st->u.loose.mapped, st->u.loose.mapsize);
- return -1;
+ case ULHR_TOO_LONG:
+ goto error;
+ }
+- if (parse_loose_header(st->u.loose.hdr, &oi, 0) < 0)
++ if (parse_loose_header(st->u.loose.hdr, &oi) < 0 || *type < 0)
+ goto error;
+
+ st->u.loose.hdr_used = strlen(st->u.loose.hdr) + 1;
15: 3fb660ff944 < -: ----------- object-file.c: guard against future bugs in loose_object_info()
20: 3bf3cf2299d < -: ----------- object-store.h: move read_loose_object() below 'struct object_info'
21: 974f650cddf ! 16: d38067feab3 fsck: report invalid types recorded in objects
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- fsck: report invalid types recorded in objects
+ fsck: don't hard die on invalid object types
- Continue the work in the preceding commit and improve the error on:
+ Change the error fsck emits on invalid object types, such as:
$ git hash-object --stdin -w -t garbage --literally </dev/null
+ <OID>
+
+ From the very ungraceful error of:
+
$ git fsck
- error: hash mismatch for <OID_PATH> (expected <OID>)
- error: <OID>: object corrupt or missing: <OID_PATH>
- [ other fsck output ]
+ fatal: invalid object type
+ $
- To instead emit:
+ To:
$ git fsck
error: <OID>: object is of unknown type 'garbage': <OID_PATH>
[ other fsck output ]
- The complaint about a "hash mismatch" was simply an emergent property
- of how we'd fall though from read_loose_object() into fsck_loose()
- when we didn't get the data we expected. Now we'll correctly note that
- the object type is invalid.
+ We'll still exit with non-zero, but now we'll finish the rest of the
+ traversal. The tests that's being added here asserts that we'll still
+ complain about other fsck issues (e.g. an unrelated dangling blob).
+
+ To do this we need to pass down the "OBJECT_INFO_ALLOW_UNKNOWN_TYPE"
+ flag from read_loose_object() through to parse_loose_header(). Since
+ the read_loose_object() function is only used in builtin/fsck.c we can
+ simply change it to accept a "struct object_info" (which contains the
+ OBJECT_INFO_ALLOW_UNKNOWN_TYPE in its flags). See
+ f6371f92104 (sha1_file: add read_loose_object() function, 2017-01-13)
+ for the introduction of read_loose_object().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ builtin/fsck.c: static int fsck_loose(const struct object_id *oid, const char *p
unsigned long size;
void *contents;
int eaten;
--
-- if (read_loose_object(path, oid, &type, &size, &contents,
-- OBJECT_INFO_ALLOW_UNKNOWN_TYPE) < 0) {
-- errors_found |= ERROR_OBJECT;
+ struct strbuf sb = STRBUF_INIT;
-+ unsigned int oi_flags = OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
-+ struct object_info oi;
-+ int found = 0;
++ struct object_info oi = OBJECT_INFO_INIT;
++ int err = 0;
+
+- if (read_loose_object(path, oid, &type, &size, &contents) < 0) {
+ oi.type_name = &sb;
+ oi.sizep = &size;
+ oi.typep = &type;
+
-+ if (read_loose_object(path, oid, &contents, &oi, oi_flags) < 0) {
-+ found |= ERROR_OBJECT;
- error(_("%s: object corrupt or missing: %s"),
- oid_to_hex(oid), path);
-+ }
-+ if (type < 0) {
-+ found |= ERROR_OBJECT;
-+ error(_("%s: object is of unknown type '%s': %s"),
-+ oid_to_hex(oid), sb.buf, path);
-+ }
-+ if (found) {
-+ errors_found |= ERROR_OBJECT;
++ if (read_loose_object(path, oid, &contents, &oi) < 0)
++ err = error(_("%s: object corrupt or missing: %s"),
++ oid_to_hex(oid), path);
++ if (type < 0)
++ err = error(_("%s: object is of unknown type '%s': %s"),
++ oid_to_hex(oid), sb.buf, path);
++ if (err) {
+ errors_found |= ERROR_OBJECT;
+- error(_("%s: object corrupt or missing: %s"),
+- oid_to_hex(oid), path);
return 0; /* keep checking other objects */
}
@@ object-file.c: static int check_stream_oid(git_zstream *stream,
const struct object_id *expected_oid,
- enum object_type *type,
- unsigned long *size,
- void **contents,
-+ struct object_info *oi,
- unsigned int oi_flags)
+- void **contents)
++ void **contents,
++ struct object_info *oi)
{
int ret = -1;
-@@ object-file.c: int read_loose_object(const char *path,
+ void *map = NULL;
unsigned long mapsize;
git_zstream stream;
char hdr[MAX_HEADER_LEN];
- struct object_info oi = OBJECT_INFO_INIT;
- int allow_unknown = oi_flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
- oi.typep = type;
- oi.sizep = size;
-+ enum object_type *type = oi->typep;
+ unsigned long *size = oi->sizep;
*contents = NULL;
@@ object-file.c: int read_loose_object(const char *path,
error(_("unable to parse header of %s"), path);
git_inflate_end(&stream);
goto out;
+ }
+- if (*type < 0)
+- die(_("invalid object type"));
+
+- if (*type == OBJ_BLOB && *size > big_file_threshold) {
++ if (*oi->typep == OBJ_BLOB && *size > big_file_threshold) {
+ if (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)
+ goto out;
+ } else {
@@ object-file.c: int read_loose_object(const char *path,
goto out;
}
@@ object-file.c: int read_loose_object(const char *path,
free(*contents);
## object-store.h ##
-@@ object-store.h: int oid_object_info_extended(struct repository *r,
+@@ object-store.h: int force_object_loose(const struct object_id *oid, time_t mtime);
/*
* Open the loose object at path, check its hash, and return the contents,
@@ object-store.h: int oid_object_info_extended(struct repository *r,
* type, and size. If the object is a blob, then "contents" may return NULL,
* to allow streaming of large blobs.
*
-@@ object-store.h: int oid_object_info_extended(struct repository *r,
+@@ object-store.h: int force_object_loose(const struct object_id *oid, time_t mtime);
*/
int read_loose_object(const char *path,
const struct object_id *expected_oid,
- enum object_type *type,
- unsigned long *size,
- void **contents,
-+ struct object_info *oi,
- unsigned int oi_flags);
+- void **contents);
++ void **contents,
++ struct object_info *oi);
- /*
+ /* Retry packed storage after checking packed and loose storage */
+ #define HAS_OBJECT_RECHECK_PACKED 1
## t/t1450-fsck.sh ##
-@@ t/t1450-fsck.sh: test_expect_success 'object with hash mismatch' '
- )
- '
+@@ t/t1450-fsck.sh: test_expect_success 'object with hash and type mismatch' '
+ cmt=$(echo bogus | git commit-tree $tree) &&
+ git update-ref refs/heads/bogus $cmt &&
-+test_expect_success 'object with hash and type mismatch' '
-+ git init --bare hash-type-mismatch &&
-+ (
-+ cd hash-type-mismatch &&
-+ oid=$(echo blob | git hash-object -w --stdin -t garbage --literally) &&
-+ old=$(test_oid_to_path "$oid") &&
-+ new=$(dirname $old)/$(test_oid ff_2) &&
-+ oid="$(dirname $new)$(basename $new)" &&
-+ mv objects/$old objects/$new &&
-+ git update-index --add --cacheinfo 100644 $oid foo &&
-+ tree=$(git write-tree) &&
-+ cmt=$(echo bogus | git commit-tree $tree) &&
-+ git update-ref refs/heads/bogus $cmt &&
+- cat >expect <<-\EOF &&
+- fatal: invalid object type
+- EOF
+- test_must_fail git fsck 2>actual &&
+- test_cmp expect actual
++
+ test_must_fail git fsck 2>out &&
+ grep "^error: hash mismatch for " out &&
+ grep "^error: $oid: object is of unknown type '"'"'garbage'"'"'" out
-+ )
-+'
-+
- test_expect_success 'branch pointing to non-commit' '
- git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
- test_when_finished "git update-ref -d refs/heads/invalid" &&
-@@ t/t1450-fsck.sh: test_expect_success 'fsck error and recovery on invalid object type' '
- garbage_blob=$(git -C garbage-type hash-object --stdin -w -t garbage --literally </dev/null) &&
- test_must_fail git -C garbage-type fsck >out 2>err &&
- grep -e "^error" -e "^fatal" err >errors &&
-- test_line_count = 2 errors &&
-- grep "error: hash mismatch for" err &&
-- grep "$garbage_blob: object corrupt or missing:" err &&
-+ test_line_count = 1 errors &&
-+ grep "$garbage_blob: object is of unknown type '"'"'garbage'"'"':" err &&
- grep "dangling blob $empty_blob" out
+ )
+ '
+
+@@ t/t1450-fsck.sh: test_expect_success 'detect corrupt index file in fsck' '
+ test_i18ngrep "bad index file" errors
+ '
+
+-test_expect_success 'fsck hard errors on an invalid object type' '
++test_expect_success 'fsck error and recovery on invalid object type' '
+ git init --bare garbage-type &&
+ (
+ cd garbage-type &&
+@@ t/t1450-fsck.sh: test_expect_success 'fsck hard errors on an invalid object type' '
+ fatal: invalid object type
+ EOF
+ test_must_fail git fsck >out 2>err &&
+- test_cmp err.expect err &&
+- test_must_be_empty out
++ grep -e "^error" -e "^fatal" err >errors &&
++ test_line_count = 1 errors &&
++ grep "$garbage_blob: object is of unknown type '"'"'garbage'"'"':" err &&
++ grep "dangling blob $empty_blob" out
+ )
'
22: 804673a17b0 ! 17: b07e892fc19 fsck: report invalid object type-path combinations
@@ Commit message
value for checking whether we got far enough to be certain that the
issue was indeed this OID mismatch.
- In the case of check_object_signature() I don't really trust all the
- moving parts there to behave consistently, in the face of future
- refactorings. Getting it wrong would mean that we'd potentially emit
- no error at all on a failing check_object_signature(), or worse
- misreport whatever issue we encountered. So let's use the new bug()
- function to ferry and return code up to fsck_loose() in that case.
+ We need to add the "object corrupt or missing" special-case to deal
+ with cases where read_loose_object() will return an error before
+ completing check_object_signature(), e.g. if we have an error in
+ unpack_loose_rest() because we find garbage after the valid gzip
+ content:
+
+ $ git hash-object --stdin -w -t blob </dev/null
+ e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+ $ chmod 755 objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
+ $ echo garbage >>objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
+ $ git fsck
+ error: garbage at end of loose object 'e69d[...]'
+ error: unable to unpack contents of ./objects/e6/9d[...]
+ error: e69d[...]: object corrupt or missing: ./objects/e6/9d[...]
+
+ There is currently some weird messaging in the edge case when the two
+ are combined, i.e. because we're not explicitly passing along an error
+ state about this specific scenario from check_stream_oid() via
+ read_loose_object() we'll end up printing the null OID if an object is
+ of an unknown type *and* it can't be unpacked by zlib, e.g.:
+
+ $ git hash-object --stdin -w -t garbage --literally </dev/null
+ 8315a83d2acc4c174aed59430f9a9c4ed926440f
+ $ chmod 755 objects/83/15a83d2acc4c174aed59430f9a9c4ed926440f
+ $ echo garbage >>objects/83/15a83d2acc4c174aed59430f9a9c4ed926440f
+ $ /usr/bin/git fsck
+ fatal: invalid object type
+ $ ~/g/git/git fsck
+ error: garbage at end of loose object '8315a83d2acc4c174aed59430f9a9c4ed926440f'
+ error: unable to unpack contents of ./objects/83/15a83d2acc4c174aed59430f9a9c4ed926440f
+ error: 8315a83d2acc4c174aed59430f9a9c4ed926440f: object corrupt or missing: ./objects/83/15a83d2acc4c174aed59430f9a9c4ed926440f
+ error: 0000000000000000000000000000000000000000: object is of unknown type 'garbage': ./objects/83/15a83d2acc4c174aed59430f9a9c4ed926440f
+ [...]
+
+ I think it's OK to leave that for future improvements, which would
+ involve enum-ifying more error state as we've done with "enum
+ unpack_loose_header_result" in preceding commits. In these
+ increasingly more obscure cases the worst that can happen is that
+ we'll get slightly nonsensical or inapplicable error messages.
+
+ There's other such potential edge cases, all of which might produce
+ some confusing messaging, but still be handled correctly as far as
+ passing along errors goes. E.g. if check_object_signature() returns
+ and oideq(real_oid, null_oid()) is true, which could happen if it
+ returns -1 due to the read_istream() call having failed.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ builtin/fast-export.c: static void export_blob(const struct object_id *oid)
## builtin/fsck.c ##
@@ builtin/fsck.c: static int fsck_loose(const struct object_id *oid, const char *path, void *data)
+ struct object *obj;
+ enum object_type type;
+ unsigned long size;
+- void *contents;
++ unsigned char *contents = NULL;
+ int eaten;
struct strbuf sb = STRBUF_INIT;
- unsigned int oi_flags = OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
- struct object_info oi;
+ struct object_info oi = OBJECT_INFO_INIT;
+- int err = 0;
+ struct object_id real_oid = *null_oid();
- int found = 0;
++ int ret;
+
oi.type_name = &sb;
oi.sizep = &size;
oi.typep = &type;
-- if (read_loose_object(path, oid, &contents, &oi, oi_flags) < 0) {
-+ if (read_loose_object(path, oid, &real_oid, &contents, &oi, oi_flags) < 0) {
- found |= ERROR_OBJECT;
-- error(_("%s: object corrupt or missing: %s"),
-- oid_to_hex(oid), path);
-+ if (!oideq(&real_oid, oid))
+- if (read_loose_object(path, oid, &contents, &oi) < 0)
+- err = error(_("%s: object corrupt or missing: %s"),
+- oid_to_hex(oid), path);
++ ret = read_loose_object(path, oid, &real_oid, (void **)&contents, &oi);
++ if (ret < 0) {
++ if (contents && !oideq(&real_oid, oid))
+ error(_("%s: hash-path mismatch, found at: %s"),
+ oid_to_hex(&real_oid), path);
+ else
+ error(_("%s: object corrupt or missing: %s"),
+ oid_to_hex(oid), path);
- }
- if (type < 0) {
- found |= ERROR_OBJECT;
- error(_("%s: object is of unknown type '%s': %s"),
-- oid_to_hex(oid), sb.buf, path);
-+ oid_to_hex(&real_oid), sb.buf, path);
- }
- if (found) {
++ }
+ if (type < 0)
+- err = error(_("%s: object is of unknown type '%s': %s"),
+- oid_to_hex(oid), sb.buf, path);
+- if (err) {
++ ret = error(_("%s: object is of unknown type '%s': %s"),
++ oid_to_hex(&real_oid), sb.buf, path);
++ if (ret < 0) {
errors_found |= ERROR_OBJECT;
+ return 0; /* keep checking other objects */
+ }
## builtin/index-pack.c ##
@@ builtin/index-pack.c: static void fix_unresolved_deltas(struct hashfile *f)
@@ builtin/mktag.c: static int verify_object_in_tag(struct object_id *tagged_oid, i
return ret;
+ ## cache.h ##
+@@ cache.h: struct object_info;
+ int parse_loose_header(const char *hdr, struct object_info *oi);
+
+ int check_object_signature(struct repository *r, const struct object_id *oid,
+- void *buf, unsigned long size, const char *type);
++ void *buf, unsigned long size, const char *type,
++ struct object_id *real_oidp);
+
+ int finalize_object_file(const char *tmpfile, const char *filename);
+
+
## object-file.c ##
@@ object-file.c: void *xmmap(void *start, size_t length,
* the streaming interface and rehash it to do the same.
@@ object-file.c: static int check_stream_oid(git_zstream *stream,
const struct object_id *expected_oid,
+ struct object_id *real_oid,
void **contents,
- struct object_info *oi,
- unsigned int oi_flags)
+ struct object_info *oi)
+ {
+@@ object-file.c: int read_loose_object(const char *path,
+ char hdr[MAX_HEADER_LEN];
+ unsigned long *size = oi->sizep;
+
+- *contents = NULL;
+-
+ map = map_loose_object_1(the_repository, path, NULL, &mapsize);
+ if (!map) {
+ error_errno(_("unable to mmap %s"), path);
@@ object-file.c: int read_loose_object(const char *path,
goto out;
}
@@ object-file.c: int read_loose_object(const char *path,
- error(_("hash mismatch for %s (expected %s)"), path,
- oid_to_hex(expected_oid));
+ *contents, *size, oi->type_name->buf, real_oid)) {
-+ if (oideq(real_oid, null_oid()))
-+ BUG("should only get OID mismatch errors with mapped contents");
free(*contents);
goto out;
}
## object-store.h ##
-@@ object-store.h: int oid_object_info_extended(struct repository *r,
+@@ object-store.h: int force_object_loose(const struct object_id *oid, time_t mtime);
*/
int read_loose_object(const char *path,
const struct object_id *expected_oid,
+ struct object_id *real_oid,
void **contents,
- struct object_info *oi,
- unsigned int oi_flags);
-@@ object-store.h: enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
- int parse_loose_header(const char *hdr, struct object_info *oi);
-
- int check_object_signature(struct repository *r, const struct object_id *oid,
-- void *buf, unsigned long size, const char *type);
-+ void *buf, unsigned long size, const char *type,
-+ struct object_id *real_oidp);
- int finalize_object_file(const char *tmpfile, const char *filename);
- int check_and_freshen_file(const char *fn, int freshen);
+ struct object_info *oi);
## object.c ##
@@ t/t1006-cat-file.sh: test_expect_success 'cat-file -t and -s on corrupt loose ob
## t/t1450-fsck.sh ##
@@ t/t1450-fsck.sh: test_expect_success 'object with hash mismatch' '
- (
cd hash-mismatch &&
+
oid=$(echo blob | git hash-object -w --stdin) &&
+ oldoid=$oid &&
old=$(test_oid_to_path "$oid") &&
new=$(dirname $old)/$(test_oid ff_2) &&
oid="$(dirname $new)$(basename $new)" &&
@@ t/t1450-fsck.sh: test_expect_success 'object with hash mismatch' '
- cmt=$(echo bogus | git commit-tree $tree) &&
git update-ref refs/heads/bogus $cmt &&
+
test_must_fail git fsck 2>out &&
-- test_i18ngrep "$oid.*corrupt" out
+- grep "$oid.*corrupt" out
+ grep "$oldoid: hash-path mismatch, found at: .*$new" out
)
'
@@ t/t1450-fsck.sh: test_expect_success 'object with hash and type mismatch' '
- (
cd hash-type-mismatch &&
+
oid=$(echo blob | git hash-object -w --stdin -t garbage --literally) &&
+ oldoid=$oid &&
old=$(test_oid_to_path "$oid") &&
new=$(dirname $old)/$(test_oid ff_2) &&
oid="$(dirname $new)$(basename $new)" &&
@@ t/t1450-fsck.sh: test_expect_success 'object with hash and type mismatch' '
- cmt=$(echo bogus | git commit-tree $tree) &&
- git update-ref refs/heads/bogus $cmt &&
+
+
test_must_fail git fsck 2>out &&
- grep "^error: hash mismatch for " out &&
- grep "^error: $oid: object is of unknown type '"'"'garbage'"'"'" out
--
2.33.0.1098.g29a6526ae47
next prev parent reply other threads:[~2021-09-21 2:58 UTC|newest]
Thread overview: 245+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-28 2:25 [PATCH 0/4] usage.c: add a non-fatal bug() + misc doc fixes Ævar Arnfjörð Bjarmason
2021-03-28 2:26 ` [PATCH 1/4] usage.c: don't copy/paste the same comment three times Ævar Arnfjörð Bjarmason
2021-03-28 2:32 ` Eric Sunshine
2021-03-28 2:26 ` [PATCH 2/4] api docs: document BUG() in api-error-handling.txt Ævar Arnfjörð Bjarmason
2021-03-29 5:37 ` Bagas Sanjaya
2021-03-28 2:26 ` [PATCH 3/4] api docs: document that BUG() emits a trace2 error event Ævar Arnfjörð Bjarmason
2021-03-28 2:26 ` [PATCH 4/4] usage.c: add a non-fatal bug() function to go with BUG() Ævar Arnfjörð Bjarmason
2021-03-28 2:58 ` [PATCH 0/5] fsck: improve error reporting Ævar Arnfjörð Bjarmason
2021-03-28 2:58 ` [PATCH 1/5] cache.h: move object functions to object-store.h Ævar Arnfjörð Bjarmason
2021-03-28 2:58 ` [PATCH 2/5] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-03-28 2:58 ` [PATCH 3/5] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-03-28 2:58 ` [PATCH 4/5] fsck: improve the error " Ævar Arnfjörð Bjarmason
2021-03-28 8:56 ` Johannes Sixt
2021-03-28 2:58 ` [PATCH 5/5] fsck: improve error on loose object hash mismatch Ævar Arnfjörð Bjarmason
2021-04-13 9:43 ` [PATCH v2 0/6] fsck: better "invalid object" error reporting Ævar Arnfjörð Bjarmason
2021-04-13 9:43 ` [PATCH v2 1/6] cache.h: move object functions to object-store.h Ævar Arnfjörð Bjarmason
2021-04-13 9:43 ` [PATCH v2 2/6] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-04-13 9:43 ` [PATCH v2 3/6] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-04-23 14:26 ` Jeff King
2021-04-13 9:43 ` [PATCH v2 4/6] object-store.h: move read_loose_object() below 'struct object_info' Ævar Arnfjörð Bjarmason
2021-04-23 14:27 ` Jeff King
2021-04-13 9:43 ` [PATCH v2 5/6] fsck: report invalid types recorded in objects Ævar Arnfjörð Bjarmason
2021-04-23 14:37 ` Jeff King
2021-04-26 14:28 ` Ævar Arnfjörð Bjarmason
2021-04-26 15:45 ` Jeff King
2021-04-13 9:43 ` [PATCH v2 6/6] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-05-20 11:22 ` [PATCH v3 00/17] fsck: better "invalid object" error reporting Ævar Arnfjörð Bjarmason
2021-05-20 11:22 ` [PATCH v3 01/17] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-05-20 11:22 ` [PATCH v3 02/17] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-05-20 11:22 ` [PATCH v3 03/17] cat-file tests: test for missing object with -t and -s Ævar Arnfjörð Bjarmason
2021-05-20 11:22 ` [PATCH v3 04/17] cat-file tests: test that --allow-unknown-type isn't on by default Ævar Arnfjörð Bjarmason
2021-05-27 21:17 ` Jonathan Nieder
2021-05-28 3:10 ` Ævar Arnfjörð Bjarmason
2021-05-20 11:22 ` [PATCH v3 05/17] rev-list tests: test for behavior with invalid object types Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 06/17] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 07/17] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 08/17] cache.h: move object functions to object-store.h Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 09/17] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 10/17] object-file.c: add missing braces to loose_object_info() Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 11/17] object-file.c: stop dying in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-05-27 17:50 ` Jonathan Tan
2021-05-20 11:23 ` [PATCH v3 12/17] object-file.c: return -2 on "header too long" in unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-05-27 17:54 ` Jonathan Tan
2021-05-20 11:23 ` [PATCH v3 13/17] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 14/17] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-05-27 18:18 ` Jonathan Tan
2021-05-20 11:23 ` [PATCH v3 15/17] object-store.h: move read_loose_object() below 'struct object_info' Ævar Arnfjörð Bjarmason
2021-05-20 11:23 ` [PATCH v3 16/17] fsck: report invalid types recorded in objects Ævar Arnfjörð Bjarmason
2021-05-27 18:24 ` Jonathan Tan
2021-05-20 11:23 ` [PATCH v3 17/17] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-05-27 18:28 ` Jonathan Tan
2021-05-27 17:08 ` [PATCH v3 00/17] fsck: better "invalid object" error reporting Jonathan Tan
2021-05-28 0:18 ` Junio C Hamano
2021-05-28 5:41 ` Felipe Contreras
2021-06-24 19:23 ` [PATCH v4 00/21] " Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 01/21] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-06-24 22:00 ` Andrei Rybak
2021-06-24 22:34 ` Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 02/21] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 03/21] cat-file tests: test for missing object with -t and -s Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 04/21] cat-file tests: test that --allow-unknown-type isn't on by default Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 05/21] rev-list tests: test for behavior with invalid object types Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 06/21] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 07/21] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 08/21] cache.h: move object functions to object-store.h Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 09/21] object-file.c: don't set "typep" when returning non-zero Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 10/21] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 11/21] object-file.c: add missing braces to loose_object_info() Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 12/21] object-file.c: simplify unpack_loose_short_header() Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 13/21] object-file.c: split up ternary in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 14/21] object-file.c: stop dying " Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 15/21] object-file.c: guard against future bugs in loose_object_info() Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 16/21] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 17/21] object-file.c: return -2 on "header too long" in unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 18/21] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 19/21] object-store.h: move read_loose_object() below 'struct object_info' Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 20/21] fsck: report invalid types recorded in objects Ævar Arnfjörð Bjarmason
2021-06-24 19:23 ` [PATCH v4 21/21] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 00/21] fsck: lib-ify object-file.c & better fsck "invalid object" error reporting Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 01/21] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 02/21] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 03/21] cat-file tests: test for missing object with -t and -s Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 04/21] cat-file tests: test that --allow-unknown-type isn't on by default Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 05/21] rev-list tests: test for behavior with invalid object types Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 06/21] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 07/21] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 08/21] cache.h: move object functions to object-store.h Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 09/21] object-file.c: don't set "typep" when returning non-zero Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 10/21] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 11/21] object-file.c: add missing braces to loose_object_info() Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 12/21] object-file.c: simplify unpack_loose_short_header() Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 13/21] object-file.c: split up ternary in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 14/21] object-file.c: stop dying " Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 15/21] object-file.c: guard against future bugs in loose_object_info() Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 16/21] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 17/21] object-file.c: return -2 on "header too long" in unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 18/21] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 19/21] object-store.h: move read_loose_object() below 'struct object_info' Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 20/21] fsck: report invalid types recorded in objects Ævar Arnfjörð Bjarmason
2021-07-10 13:37 ` [PATCH v5 21/21] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-09-07 10:57 ` [PATCH v6 00/22] fsck: lib-ify object-file.c & better fsck "invalid object" error reporting Ævar Arnfjörð Bjarmason
2021-09-07 10:57 ` [PATCH v6 01/22] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-09-16 19:40 ` Taylor Blau
2021-09-17 9:27 ` Ævar Arnfjörð Bjarmason
2021-09-07 10:57 ` [PATCH v6 02/22] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-09-16 19:51 ` Taylor Blau
2021-09-17 9:39 ` Ævar Arnfjörð Bjarmason
2021-09-07 10:57 ` [PATCH v6 03/22] cat-file tests: test for missing object with -t and -s Ævar Arnfjörð Bjarmason
2021-09-16 19:57 ` Taylor Blau
2021-09-16 20:01 ` Taylor Blau
2021-09-16 22:52 ` Ævar Arnfjörð Bjarmason
2021-09-07 10:57 ` [PATCH v6 04/22] cat-file tests: test that --allow-unknown-type isn't on by default Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 05/22] rev-list tests: test for behavior with invalid object types Ævar Arnfjörð Bjarmason
2021-09-16 20:40 ` Taylor Blau
2021-09-17 11:59 ` Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 06/22] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 07/22] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 08/22] object-file.c: don't set "typep" when returning non-zero Ævar Arnfjörð Bjarmason
2021-09-16 21:29 ` Taylor Blau
2021-09-16 21:56 ` Jeff King
2021-09-07 10:58 ` [PATCH v6 09/22] cache.h: move object functions to object-store.h Ævar Arnfjörð Bjarmason
2021-09-16 21:33 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 10/22] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-09-16 21:39 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 11/22] object-file.c: add missing braces to loose_object_info() Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 12/22] object-file.c: simplify unpack_loose_short_header() Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 13/22] object-file.c: split up ternary in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-09-16 21:58 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 14/22] object-file.c: stop dying " Ævar Arnfjörð Bjarmason
2021-09-17 2:32 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 15/22] object-file.c: guard against future bugs in loose_object_info() Ævar Arnfjörð Bjarmason
2021-09-17 2:35 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 16/22] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 17/22] object-file.c: return -2 on "header too long" in unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 18/22] object-file.c: use "enum" return type for unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-17 2:45 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 19/22] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-09-17 3:37 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 20/22] object-store.h: move read_loose_object() below 'struct object_info' Ævar Arnfjörð Bjarmason
2021-09-07 10:58 ` [PATCH v6 21/22] fsck: report invalid types recorded in objects Ævar Arnfjörð Bjarmason
2021-09-17 3:57 ` Taylor Blau
2021-09-07 10:58 ` [PATCH v6 22/22] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-09-17 4:06 ` Taylor Blau
2021-09-17 4:08 ` [PATCH v6 00/22] fsck: lib-ify object-file.c & better fsck "invalid object" error reporting Taylor Blau
2021-09-20 19:04 ` Ævar Arnfjörð Bjarmason [this message]
2021-09-20 19:04 ` [PATCH v7 01/17] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 02/17] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 03/17] fsck tests: test current hash/type mismatch behavior Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 04/17] fsck tests: test for garbage appended to a loose object Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 05/17] cat-file tests: move bogus_* variable declarations earlier Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 06/17] cat-file tests: test for missing/bogus object with -t, -s and -p Ævar Arnfjörð Bjarmason
2021-09-21 3:30 ` Taylor Blau
2021-09-20 19:04 ` [PATCH v7 07/17] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 08/17] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 09/17] object-file.c: don't set "typep" when returning non-zero Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 10/17] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 11/17] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 12/17] object-file.c: simplify unpack_loose_short_header() Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 13/17] object-file.c: use "enum" return type for unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 14/17] object-file.c: return ULHR_TOO_LONG on "header too long" Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 15/17] object-file.c: stop dying in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 16/17] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-09-20 19:04 ` [PATCH v7 17/17] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 00/17] fsck: lib-ify object-file.c & better fsck "invalid object" error reporting Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 01/17] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 02/17] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 03/17] fsck tests: test current hash/type mismatch behavior Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 04/17] fsck tests: test for garbage appended to a loose object Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 05/17] cat-file tests: move bogus_* variable declarations earlier Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 06/17] cat-file tests: test for missing/bogus object with -t, -s and -p Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 07/17] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 08/17] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 09/17] object-file.c: don't set "typep" when returning non-zero Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 10/17] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 11/17] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 12/17] object-file.c: simplify unpack_loose_short_header() Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 13/17] object-file.c: use "enum" return type for unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 14/17] object-file.c: return ULHR_TOO_LONG on "header too long" Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 15/17] object-file.c: stop dying in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 16/17] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-09-28 2:18 ` [PATCH v8 17/17] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-09-29 19:50 ` [PATCH v8 00/17] fsck: lib-ify object-file.c & better fsck "invalid object" error reporting Taylor Blau
2021-09-30 13:37 ` [PATCH v9 " Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 01/17] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-09-30 19:22 ` Andrei Rybak
2021-10-01 9:05 ` Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 02/17] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 03/17] fsck tests: test current hash/type mismatch behavior Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 04/17] fsck tests: test for garbage appended to a loose object Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 05/17] cat-file tests: move bogus_* variable declarations earlier Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 06/17] cat-file tests: test for missing/bogus object with -t, -s and -p Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 07/17] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 08/17] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 09/17] object-file.c: don't set "typep" when returning non-zero Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 10/17] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 11/17] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 12/17] object-file.c: simplify unpack_loose_short_header() Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 13/17] object-file.c: use "enum" return type for unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 14/17] object-file.c: return ULHR_TOO_LONG on "header too long" Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 15/17] object-file.c: stop dying in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 16/17] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-09-30 13:37 ` [PATCH v9 17/17] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-09-30 21:01 ` Junio C Hamano
2021-09-30 19:06 ` [PATCH v9 00/17] fsck: lib-ify object-file.c & better fsck "invalid object" error reporting Taylor Blau
2021-10-01 9:16 ` [PATCH v10 " Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 01/17] fsck tests: add test for fsck-ing an unknown type Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 02/17] fsck tests: refactor one test to use a sub-repo Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 03/17] fsck tests: test current hash/type mismatch behavior Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 04/17] fsck tests: test for garbage appended to a loose object Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 05/17] cat-file tests: move bogus_* variable declarations earlier Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 06/17] cat-file tests: test for missing/bogus object with -t, -s and -p Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 07/17] cat-file tests: add corrupt loose object test Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 08/17] cat-file tests: test for current --allow-unknown-type behavior Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 09/17] object-file.c: don't set "typep" when returning non-zero Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 10/17] object-file.c: return -1, not "status" from unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 11/17] object-file.c: make parse_loose_header_extended() public Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 12/17] object-file.c: simplify unpack_loose_short_header() Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 13/17] object-file.c: use "enum" return type for unpack_loose_header() Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 14/17] object-file.c: return ULHR_TOO_LONG on "header too long" Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 15/17] object-file.c: stop dying in parse_loose_header() Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 16/17] fsck: don't hard die on invalid object types Ævar Arnfjörð Bjarmason
2021-10-01 9:16 ` [PATCH v10 17/17] fsck: report invalid object type-path combinations Ævar Arnfjörð Bjarmason
2021-10-01 22:14 ` Junio C Hamano
2021-10-01 22:33 ` Ævar Arnfjörð Bjarmason
2021-11-11 3:03 ` [PATCH v2] receive-pack: not receive pack file with large object Han Xin
2021-11-11 18:35 ` Junio C Hamano
2021-11-11 3:05 ` [PATCH v10 17/17] fsck: report invalid object type-path combinations Han Xin
2021-11-11 5:18 ` [PATCH 0/2] v2.34.0-rc2 regression: free() of uninitialized in ab/fsck-unexpected-type Ævar Arnfjörð Bjarmason
2021-11-11 5:18 ` [PATCH 1/2] object-file: fix SEGV on free() regression in v2.34.0-rc2 Ævar Arnfjörð Bjarmason
2021-11-11 15:18 ` Jeff King
2021-11-11 18:41 ` Junio C Hamano
2021-11-13 9:00 ` Ævar Arnfjörð Bjarmason
2021-11-11 5:18 ` [PATCH 2/2] object-file: free(*contents) only in read_loose_object() caller Ævar Arnfjörð Bjarmason
2021-11-11 18:54 ` Junio C Hamano
2021-03-28 6:12 ` [PATCH 4/4] usage.c: add a non-fatal bug() function to go with BUG() Junio C Hamano
2021-03-28 7:17 ` Jeff King
2021-03-29 13:25 ` Ævar Arnfjörð Bjarmason
2021-03-31 11:06 ` Jeff King
2021-04-13 9:08 ` [PATCH v2 0/3] trace2 docs: note that BUG() sends an "error" event Ævar Arnfjörð Bjarmason
2021-04-13 9:08 ` [PATCH v2 1/3] usage.c: don't copy/paste the same comment three times Ævar Arnfjörð Bjarmason
2021-04-15 10:09 ` Jeff King
2021-04-13 9:08 ` [PATCH v2 2/3] api docs: document BUG() in api-error-handling.txt Ævar Arnfjörð Bjarmason
2021-04-15 10:00 ` Jeff King
2021-04-13 9:08 ` [PATCH v2 3/3] api docs: document that BUG() emits a trace2 error event Ævar Arnfjörð Bjarmason
2021-04-15 10:10 ` [PATCH v2 0/3] trace2 docs: note that BUG() sends an "error" event Jeff King
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover-v7-00.17-00000000000-20210920T190304Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jonathantanmy@google.com \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
--cc=rybak.a.v@gmail.com \
/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.
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 NNTP newsgroup(s).