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>

shejialuo committed Feb 28, 2025 at 00:07 UTC e6ba4c07b85a0a8fee84b6ac7ab414d47a5351f2
4 files changed +169 -1
Documentation/fsck-msgids.txt
+3
@@ -16,6 +16,9 @@
16 `badObjectSha1`::
17 (ERROR) An object has a bad sha1.
18
19 +`badPackedRefEntry`::
20 + (ERROR) The "packed-refs" file contains an invalid entry.
21 +
22 `badPackedRefHeader`::
23 (ERROR) The "packed-refs" file contains an invalid
24 header.
fsck.h
+1
@@ -30,6 +30,7 @@ enum fsck_msg_type {
30 FUNC(BAD_EMAIL, ERROR) \
31 FUNC(BAD_NAME, ERROR) \
32 FUNC(BAD_OBJECT_SHA1, ERROR) \
33 + FUNC(BAD_PACKED_REF_ENTRY, ERROR) \
34 FUNC(BAD_PACKED_REF_HEADER, ERROR) \
35 FUNC(BAD_PARENT_SHA1, ERROR) \
36 FUNC(BAD_REF_CONTENT, ERROR) \
refs/packed-backend.c
+121 -1
@@ -1812,9 +1812,114 @@ static int packed_fsck_ref_header(struct fsck_options *o,
1812 return 0;
1813 }
1814
1815 +static int packed_fsck_ref_peeled_line(struct fsck_options *o,
1816 + struct ref_store *ref_store,
1817 + unsigned long line_number,
1818 + const char *start, const char *eol)
1819 +{
1820 + struct strbuf packed_entry = STRBUF_INIT;
1821 + struct fsck_ref_report report = { 0 };
1822 + struct object_id peeled;
1823 + const char *p;
1824 + int ret = 0;
1825 +
1826 + /*
1827 + * Skip the '^' and parse the peeled oid.
1828 + */
1829 + start++;
1830 + if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) {
1831 + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1832 + report.path = packed_entry.buf;
1833 +
1834 + ret = fsck_report_ref(o, &report,
1835 + FSCK_MSG_BAD_PACKED_REF_ENTRY,
1836 + "'%.*s' has invalid peeled oid",
1837 + (int)(eol - start), start);
1838 + goto cleanup;
1839 + }
1840 +
1841 + if (p != eol) {
1842 + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1843 + report.path = packed_entry.buf;
1844 +
1845 + ret = fsck_report_ref(o, &report,
1846 + FSCK_MSG_BAD_PACKED_REF_ENTRY,
1847 + "has trailing garbage after peeled oid '%.*s'",
1848 + (int)(eol - p), p);
1849 + goto cleanup;
1850 + }
1851 +
1852 +cleanup:
1853 + strbuf_release(&packed_entry);
1854 + return ret;
1855 +}
1856 +
1857 +static int packed_fsck_ref_main_line(struct fsck_options *o,
1858 + struct ref_store *ref_store,
1859 + unsigned long line_number,
1860 + struct strbuf *refname,
1861 + const char *start, const char *eol)
1862 +{
1863 + struct strbuf packed_entry = STRBUF_INIT;
1864 + struct fsck_ref_report report = { 0 };
1865 + struct object_id oid;
1866 + const char *p;
1867 + int ret = 0;
1868 +
1869 + if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) {
1870 + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1871 + report.path = packed_entry.buf;
1872 +
1873 + ret = fsck_report_ref(o, &report,
1874 + FSCK_MSG_BAD_PACKED_REF_ENTRY,
1875 + "'%.*s' has invalid oid",
1876 + (int)(eol - start), start);
1877 + goto cleanup;
1878 + }
1879 +
1880 + if (p == eol || !isspace(*p)) {
1881 + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1882 + report.path = packed_entry.buf;
1883 +
1884 + ret = fsck_report_ref(o, &report,
1885 + FSCK_MSG_BAD_PACKED_REF_ENTRY,
1886 + "has no space after oid '%s' but with '%.*s'",
1887 + oid_to_hex(&oid), (int)(eol - p), p);
1888 + goto cleanup;
1889 + }
1890 +
1891 + p++;
1892 + strbuf_reset(refname);
1893 + strbuf_add(refname, p, eol - p);
1894 + if (refname_contains_nul(refname)) {
1895 + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1896 + report.path = packed_entry.buf;
1897 +
1898 + ret = fsck_report_ref(o, &report,
1899 + FSCK_MSG_BAD_PACKED_REF_ENTRY,
1900 + "refname '%s' contains NULL binaries",
1901 + refname->buf);
1902 + }
1903 +
1904 + if (check_refname_format(refname->buf, 0)) {
1905 + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1906 + report.path = packed_entry.buf;
1907 +
1908 + ret = fsck_report_ref(o, &report,
1909 + FSCK_MSG_BAD_REF_NAME,
1910 + "has bad refname '%s'", refname->buf);
1911 + }
1912 +
1913 +cleanup:
1914 + strbuf_release(&packed_entry);
1915 + return ret;
1916 +}
1917 +
1918 static int packed_fsck_ref_content(struct fsck_options *o,
1919 + struct ref_store *ref_store,
1920 const char *start, const char *eof)
1921 {
1922 + struct strbuf refname = STRBUF_INIT;
1923 unsigned long line_number = 1;
1924 const char *eol;
1925 int ret = 0;
@@ -1827,6 +1932,21 @@ static int packed_fsck_ref_content(struct fsck_options *o,
1932 line_number++;
1933 }
1934
1935 + while (start < eof) {
1936 + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol);
1937 + ret |= packed_fsck_ref_main_line(o, ref_store, line_number, &refname, start, eol);
1938 + start = eol + 1;
1939 + line_number++;
1940 + if (start < eof && *start == '^') {
1941 + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol);
1942 + ret |= packed_fsck_ref_peeled_line(o, ref_store, line_number,
1943 + start, eol);
1944 + start = eol + 1;
1945 + line_number++;
1946 + }
1947 + }
1948 +
1949 + strbuf_release(&refname);
1950 return ret;
1951 }
1952
@@ -1884,7 +2004,7 @@ static int packed_fsck(struct ref_store *ref_store,
2004 goto cleanup;
2005 }
2006
1887 - ret = packed_fsck_ref_content(o, packed_ref_content.buf,
2007 + ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf,
2008 packed_ref_content.buf + packed_ref_content.len);
2009
2010 cleanup:
t/t0602-reffiles-fsck.sh
+44
@@ -699,4 +699,48 @@ test_expect_success 'packed-refs unknown traits should not be reported' '
699 )
700 '
701
702 +test_expect_success 'packed-refs content should be checked' '
703 + test_when_finished "rm -rf repo" &&
704 + git init repo &&
705 + (
706 + cd repo &&
707 + test_commit default &&
708 + git branch branch-1 &&
709 + git branch branch-2 &&
710 + git tag -a annotated-tag-1 -m tag-1 &&
711 + git tag -a annotated-tag-2 -m tag-2 &&
712 +
713 + branch_1_oid=$(git rev-parse branch-1) &&
714 + branch_2_oid=$(git rev-parse branch-2) &&
715 + tag_1_oid=$(git rev-parse annotated-tag-1) &&
716 + tag_2_oid=$(git rev-parse annotated-tag-2) &&
717 + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) &&
718 + tag_2_peeled_oid=$(git rev-parse annotated-tag-2^{}) &&
719 + short_oid=$(printf "%s" $tag_1_peeled_oid | cut -c 1-4) &&
720 +
721 + cat >.git/packed-refs <<-EOF &&
722 + # pack-refs with: peeled fully-peeled sorted
723 + $short_oid refs/heads/branch-1
724 + ${branch_1_oid}x
725 + $branch_2_oid refs/heads/bad-branch
726 + $branch_2_oid refs/heads/branch.
727 + $tag_1_oid refs/tags/annotated-tag-3
728 + ^$short_oid
729 + $tag_2_oid refs/tags/annotated-tag-4.
730 + ^$tag_2_peeled_oid garbage
731 + EOF
732 + test_must_fail git refs verify 2>err &&
733 + cat >expect <<-EOF &&
734 + error: packed-refs line 2: badPackedRefEntry: '\''$short_oid refs/heads/branch-1'\'' has invalid oid
735 + error: packed-refs line 3: badPackedRefEntry: has no space after oid '\''$branch_1_oid'\'' but with '\''x'\''
736 + error: packed-refs line 4: badRefName: has bad refname '\'' refs/heads/bad-branch'\''
737 + error: packed-refs line 5: badRefName: has bad refname '\''refs/heads/branch.'\''
738 + error: packed-refs line 7: badPackedRefEntry: '\''$short_oid'\'' has invalid peeled oid
739 + error: packed-refs line 8: badRefName: has bad refname '\''refs/tags/annotated-tag-4.'\''
740 + error: packed-refs line 9: badPackedRefEntry: has trailing garbage after peeled oid '\'' garbage'\''
741 + EOF
742 + test_cmp expect err
743 + )
744 +'
745 +
746 test_done