@matheusbsbs / matheus-bagliano / commits / c92e7e156e

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

In "packed-backend.c::create_snapshot", if there is a header (the line which starts with '#'), we will check whether the line starts with "# pack-refs with: ". However, we need to consider other situations and discuss whether we need to add checks. 1. If the header does not exist, we should not report an error to the user. This is because in older Git version, we never write header in the "packed-refs" file. Also, we do allow no header in "packed-refs" in runtime. 2. If the header content does not start with "# packed-ref with: ", we should report an error just like what "create_snapshot" does. So, create a new fsck message "badPackedRefHeader(ERROR)" for this. 3. If the header content is not the same as the constant string "PACKED_REFS_HEADER". This is expected because we make it extensible intentionally and runtime "create_snapshot" won't complain about unknown traits. In order to align with the runtime behavior. There is no need to report. As we have analyzed, we only need to check the case 2 in the above. In order to do this, use "open_nofollow" function to get the file descriptor and then read the "packed-refs" file via "strbuf_read". Like what "create_snapshot" and other functions do, we could split the line by finding the next newline in the buffer. When we cannot find a newline, we could report an error. So, create a function "packed_fsck_ref_next_line" to find the next newline and if there is no such newline, use "packedRefEntryNotTerminated(ERROR)" to report an error to the user. Then, parse the first line to apply the checks. 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:06 UTC c92e7e156e6b406e7555fb5df058d18758a0b3f0
4 files changed +135
Documentation/fsck-msgids.txt
+8
@@ -16,6 +16,10 @@
16 `badObjectSha1`::
17 (ERROR) An object has a bad sha1.
18
19 +`badPackedRefHeader`::
20 + (ERROR) The "packed-refs" file contains an invalid
21 + header.
22 +
23 `badParentSha1`::
24 (ERROR) A commit object has a bad parent sha1.
25
@@ -176,6 +180,10 @@
180 `nullSha1`::
181 (WARN) Tree contains entries pointing to a null sha1.
182
183 +`packedRefEntryNotTerminated`::
184 + (ERROR) The "packed-refs" file contains an entry that is
185 + not terminated by a newline.
186 +
187 `refMissingNewline`::
188 (INFO) A loose ref that does not end with newline(LF). As
189 valid implementations of Git never created such a loose ref
fsck.h
+2
@@ -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_HEADER, ERROR) \
34 FUNC(BAD_PARENT_SHA1, ERROR) \
35 FUNC(BAD_REF_CONTENT, ERROR) \
36 FUNC(BAD_REF_FILETYPE, ERROR) \
@@ -53,6 +54,7 @@ enum fsck_msg_type {
54 FUNC(MISSING_TYPE, ERROR) \
55 FUNC(MISSING_TYPE_ENTRY, ERROR) \
56 FUNC(MULTIPLE_AUTHORS, ERROR) \
57 + FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \
58 FUNC(TREE_NOT_SORTED, ERROR) \
59 FUNC(UNKNOWN_TYPE, ERROR) \
60 FUNC(ZERO_PADDED_DATE, ERROR) \
refs/packed-backend.c
+73
@@ -1749,12 +1749,76 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s
1749 return empty_ref_iterator_begin();
1750 }
1751
1752 +static int packed_fsck_ref_next_line(struct fsck_options *o,
1753 + unsigned long line_number, const char *start,
1754 + const char *eof, const char **eol)
1755 +{
1756 + int ret = 0;
1757 +
1758 + *eol = memchr(start, '\n', eof - start);
1759 + if (!*eol) {
1760 + struct strbuf packed_entry = STRBUF_INIT;
1761 + struct fsck_ref_report report = { 0 };
1762 +
1763 + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);
1764 + report.path = packed_entry.buf;
1765 + ret = fsck_report_ref(o, &report,
1766 + FSCK_MSG_PACKED_REF_ENTRY_NOT_TERMINATED,
1767 + "'%.*s' is not terminated with a newline",
1768 + (int)(eof - start), start);
1769 +
1770 + /*
1771 + * There is no newline but we still want to parse it to the end of
1772 + * the buffer.
1773 + */
1774 + *eol = eof;
1775 + strbuf_release(&packed_entry);
1776 + }
1777 +
1778 + return ret;
1779 +}
1780 +
1781 +static int packed_fsck_ref_header(struct fsck_options *o,
1782 + const char *start, const char *eol)
1783 +{
1784 + if (!starts_with(start, "# pack-refs with: ")) {
1785 + struct fsck_ref_report report = { 0 };
1786 + report.path = "packed-refs.header";
1787 +
1788 + return fsck_report_ref(o, &report,
1789 + FSCK_MSG_BAD_PACKED_REF_HEADER,
1790 + "'%.*s' does not start with '# pack-refs with: '",
1791 + (int)(eol - start), start);
1792 + }
1793 +
1794 + return 0;
1795 +}
1796 +
1797 +static int packed_fsck_ref_content(struct fsck_options *o,
1798 + const char *start, const char *eof)
1799 +{
1800 + unsigned long line_number = 1;
1801 + const char *eol;
1802 + int ret = 0;
1803 +
1804 + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol);
1805 + if (*start == '#') {
1806 + ret |= packed_fsck_ref_header(o, start, eol);
1807 +
1808 + start = eol + 1;
1809 + line_number++;
1810 + }
1811 +
1812 + return ret;
1813 +}
1814 +
1815 static int packed_fsck(struct ref_store *ref_store,
1816 struct fsck_options *o,
1817 struct worktree *wt)
1818 {
1819 struct packed_ref_store *refs = packed_downcast(ref_store,
1820 REF_STORE_READ, "fsck");
1821 + struct strbuf packed_ref_content = STRBUF_INIT;
1822 struct stat st;
1823 int ret = 0;
1824 int fd = -1;
@@ -1797,9 +1861,18 @@ static int packed_fsck(struct ref_store *ref_store,
1861 goto cleanup;
1862 }
1863
1864 + if (strbuf_read(&packed_ref_content, fd, 0) < 0) {
1865 + ret = error_errno(_("unable to read '%s'"), refs->path);
1866 + goto cleanup;
1867 + }
1868 +
1869 + ret = packed_fsck_ref_content(o, packed_ref_content.buf,
1870 + packed_ref_content.buf + packed_ref_content.len);
1871 +
1872 cleanup:
1873 if (fd >= 0)
1874 close(fd);
1875 + strbuf_release(&packed_ref_content);
1876 return ret;
1877 }
1878
t/t0602-reffiles-fsck.sh
+52
@@ -647,4 +647,56 @@ test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' '
647 )
648 '
649
650 +test_expect_success 'packed-refs header should be checked' '
651 + test_when_finished "rm -rf repo" &&
652 + git init repo &&
653 + (
654 + cd repo &&
655 + test_commit default &&
656 +
657 + git refs verify 2>err &&
658 + test_must_be_empty err &&
659 +
660 + for bad_header in "# pack-refs wit: peeled fully-peeled sorted " \
661 + "# pack-refs with traits: peeled fully-peeled sorted " \
662 + "# pack-refs with a: peeled fully-peeled" \
663 + "# pack-refs with:peeled fully-peeled sorted"
664 + do
665 + printf "%s\n" "$bad_header" >.git/packed-refs &&
666 + test_must_fail git refs verify 2>err &&
667 + cat >expect <<-EOF &&
668 + error: packed-refs.header: badPackedRefHeader: '\''$bad_header'\'' does not start with '\''# pack-refs with: '\''
669 + EOF
670 + rm .git/packed-refs &&
671 + test_cmp expect err || return 1
672 + done
673 + )
674 +'
675 +
676 +test_expect_success 'packed-refs missing header should not be reported' '
677 + test_when_finished "rm -rf repo" &&
678 + git init repo &&
679 + (
680 + cd repo &&
681 + test_commit default &&
682 +
683 + printf "$(git rev-parse HEAD) refs/heads/main\n" >.git/packed-refs &&
684 + git refs verify 2>err &&
685 + test_must_be_empty err
686 + )
687 +'
688 +
689 +test_expect_success 'packed-refs unknown traits should not be reported' '
690 + test_when_finished "rm -rf repo" &&
691 + git init repo &&
692 + (
693 + cd repo &&
694 + test_commit default &&
695 +
696 + printf "# pack-refs with: peeled fully-peeled sorted foo\n" >.git/packed-refs &&
697 + git refs verify 2>err &&
698 + test_must_be_empty err
699 + )
700 +'
701 +
702 test_done