packed-backend: add "packed-refs" entry consistency check

"packed-backend.c::next_record" will parse the ref entry to check the
consistency. This function has already checked the following things:

1. Parse the main line of the ref entry to inspect whether the oid is
   not correct. Then, check whether the next character is oid. Then
   check the refname.
2. If the next line starts with '^', it would continue to parse the
   peeled oid and check whether the last character is '\n'.

As we decide to implement the ref consistency check for "packed-refs",
let's port these two checks and update the test to exercise the code.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt
index 11906f90fd99cc957b1daa7375d2d11c28fc2e3b..02a7bf0503286b88257d2c985302aa3fcdb37691 100644 (file)
--- a/Documentation/fsck-msgids.txt
+++ b/Documentation/fsck-msgids.txt
@@ -16,6 +16,9 @@
 `badObjectSha1`::
        (ERROR) An object has a bad sha1.
 
+`badPackedRefEntry`::
+       (ERROR) The "packed-refs" file contains an invalid entry.
+
 `badPackedRefHeader`::
        (ERROR) The "packed-refs" file contains an invalid
        header.

diff --git a/fsck.h b/fsck.h
index 67e3c97bc019d8eb2c07823b0769a47b715a30ab..14d70f6653f1c37813c245593b4cd8a23e27ccd6 100644 (file)
--- a/fsck.h
+++ b/fsck.h
@@ -30,6 +30,7 @@ enum fsck_msg_type {
        FUNC(BAD_EMAIL, ERROR) \
        FUNC(BAD_NAME, ERROR) \
        FUNC(BAD_OBJECT_SHA1, ERROR) \
+       FUNC(BAD_PACKED_REF_ENTRY, ERROR) \
        FUNC(BAD_PACKED_REF_HEADER, ERROR) \
        FUNC(BAD_PARENT_SHA1, ERROR) \
        FUNC(BAD_REF_CONTENT, ERROR) \

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 9a90c52f70b56a8a23c4fab971ce988d912ac3fa..ef20300fd324c0ca06d60c16157f715dde872ca2 100644 (file)
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1812,9 +1812,114 @@ static int packed_fsck_ref_header(struct fsck_options *o,
        return 0;
 }
 
+static int packed_fsck_ref_peeled_line(struct fsck_options *o,
+                                      struct ref_store *ref_store,
+                                      unsigned long line_number,
+                                      const char *start, const char *eol)
+{
+       struct strbuf packed_entry = STRBUF_INIT;
+       struct fsck_ref_report report = { 0 };
+       struct object_id peeled;
+       const char *p;
+       int ret = 0;
+
+       /*
+        * Skip the '^' and parse the peeled oid.
+        */
+       start++;
+       if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) {
+               strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
+               report.path = packed_entry.buf;
+
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_BAD_PACKED_REF_ENTRY,
+                                     "'%.*s' has invalid peeled oid",
+                                     (int)(eol - start), start);
+               goto cleanup;
+       }
+
+       if (p != eol) {
+               strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
+               report.path = packed_entry.buf;
+
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_BAD_PACKED_REF_ENTRY,
+                                     "has trailing garbage after peeled oid '%.*s'",
+                                     (int)(eol - p), p);
+               goto cleanup;
+       }
+
+cleanup:
+       strbuf_release(&packed_entry);
+       return ret;
+}
+
+static int packed_fsck_ref_main_line(struct fsck_options *o,
+                                    struct ref_store *ref_store,
+                                    unsigned long line_number,
+                                    struct strbuf *refname,
+                                    const char *start, const char *eol)
+{
+       struct strbuf packed_entry = STRBUF_INIT;
+       struct fsck_ref_report report = { 0 };
+       struct object_id oid;
+       const char *p;
+       int ret = 0;
+
+       if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) {
+               strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
+               report.path = packed_entry.buf;
+
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_BAD_PACKED_REF_ENTRY,
+                                     "'%.*s' has invalid oid",
+                                     (int)(eol - start), start);
+               goto cleanup;
+       }
+
+       if (p == eol || !isspace(*p)) {
+               strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
+               report.path = packed_entry.buf;
+
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_BAD_PACKED_REF_ENTRY,
+                                     "has no space after oid '%s' but with '%.*s'",
+                                     oid_to_hex(&oid), (int)(eol - p), p);
+               goto cleanup;
+       }
+
+       p++;
+       strbuf_reset(refname);
+       strbuf_add(refname, p, eol - p);
+       if (refname_contains_nul(refname)) {
+               strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
+               report.path = packed_entry.buf;
+
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_BAD_PACKED_REF_ENTRY,
+                                     "refname '%s' contains NULL binaries",
+                                     refname->buf);
+       }
+
+       if (check_refname_format(refname->buf, 0)) {
+               strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
+               report.path = packed_entry.buf;
+
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_BAD_REF_NAME,
+                                     "has bad refname '%s'", refname->buf);
+       }
+
+cleanup:
+       strbuf_release(&packed_entry);
+       return ret;
+}
+
 static int packed_fsck_ref_content(struct fsck_options *o,
+                                  struct ref_store *ref_store,
                                   const char *start, const char *eof)
 {
+       struct strbuf refname = STRBUF_INIT;
        unsigned long line_number = 1;
        const char *eol;
        int ret = 0;
@@ -1827,6 +1932,21 @@ static int packed_fsck_ref_content(struct fsck_options *o,
                line_number++;
        }
 
+       while (start < eof) {
+               ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol);
+               ret |= packed_fsck_ref_main_line(o, ref_store, line_number, &refname, start, eol);
+               start = eol + 1;
+               line_number++;
+               if (start < eof && *start == '^') {
+                       ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol);
+                       ret |= packed_fsck_ref_peeled_line(o, ref_store, line_number,
+                                                          start, eol);
+                       start = eol + 1;
+                       line_number++;
+               }
+       }
+
+       strbuf_release(&refname);
        return ret;
 }
 
@@ -1884,7 +2004,7 @@ static int packed_fsck(struct ref_store *ref_store,
                goto cleanup;
        }
 
-       ret = packed_fsck_ref_content(o, packed_ref_content.buf,
+       ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf,
                                      packed_ref_content.buf + packed_ref_content.len);
 
 cleanup:

diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index 74d876984db35f7ea6ebde5fb1c3a4b0c99a41be..a88c792ce1a312b46c6db433c9ce463808b6c5d8 100755 (executable)
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -699,4 +699,48 @@ test_expect_success 'packed-refs unknown traits should not be reported' '
        )
 '
 
+test_expect_success 'packed-refs content should be checked' '
+       test_when_finished "rm -rf repo" &&
+       git init repo &&
+       (
+               cd repo &&
+               test_commit default &&
+               git branch branch-1 &&
+               git branch branch-2 &&
+               git tag -a annotated-tag-1 -m tag-1 &&
+               git tag -a annotated-tag-2 -m tag-2 &&
+
+               branch_1_oid=$(git rev-parse branch-1) &&
+               branch_2_oid=$(git rev-parse branch-2) &&
+               tag_1_oid=$(git rev-parse annotated-tag-1) &&
+               tag_2_oid=$(git rev-parse annotated-tag-2) &&
+               tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) &&
+               tag_2_peeled_oid=$(git rev-parse annotated-tag-2^{}) &&
+               short_oid=$(printf "%s" $tag_1_peeled_oid | cut -c 1-4) &&
+
+               cat >.git/packed-refs <<-EOF &&
+               # pack-refs with: peeled fully-peeled sorted
+               $short_oid refs/heads/branch-1
+               ${branch_1_oid}x
+               $branch_2_oid   refs/heads/bad-branch
+               $branch_2_oid refs/heads/branch.
+               $tag_1_oid refs/tags/annotated-tag-3
+               ^$short_oid
+               $tag_2_oid refs/tags/annotated-tag-4.
+               ^$tag_2_peeled_oid garbage
+               EOF
+               test_must_fail git refs verify 2>err &&
+               cat >expect <<-EOF &&
+               error: packed-refs line 2: badPackedRefEntry: '\''$short_oid refs/heads/branch-1'\'' has invalid oid
+               error: packed-refs line 3: badPackedRefEntry: has no space after oid '\''$branch_1_oid'\'' but with '\''x'\''
+               error: packed-refs line 4: badRefName: has bad refname '\''  refs/heads/bad-branch'\''
+               error: packed-refs line 5: badRefName: has bad refname '\''refs/heads/branch.'\''
+               error: packed-refs line 7: badPackedRefEntry: '\''$short_oid'\'' has invalid peeled oid
+               error: packed-refs line 8: badRefName: has bad refname '\''refs/tags/annotated-tag-4.'\''
+               error: packed-refs line 9: badPackedRefEntry: has trailing garbage after peeled oid '\'' garbage'\''
+               EOF
+               test_cmp expect err
+       )
+'
+
 test_done