diff mbox series

[v2,4/4] ref: add symlink ref check for files backend

Message ID Zs358SAI5AYnX75v@ArchLinux (mailing list archive)
State Superseded
Headers show
Series add ref content check for files backend | expand

Commit Message

shejialuo Aug. 27, 2024, 4:08 p.m. UTC
We have already introduced "files_fsck_symref_target". We should reuse
this function to handle the symrefs which are legacy symbolic links. We
should not check the trailing garbage for symbolic links. Add a new
parameter "symbolic_link" to disable some checks which should only be
used for symbolic ref.

We firstly use the "strbuf_add_real_path" to resolve the symlinks and
get the absolute path "pointee_path" which the symlink ref points to.
Then we can get the absolute path "abs_gitdir" of the "gitdir". By
combining "pointee_path" and "abs_gitdir", we can extract the
"referent". Thus, we can reuse "files_fsck_symref_target" function to
seamlessly check the symlink refs.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 refs/files-backend.c     | 68 +++++++++++++++++++++++++++++-----------
 t/t0602-reffiles-fsck.sh | 44 ++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 18 deletions(-)
diff mbox series

Patch

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 382c73fcf7..8641e3ba65 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1,4 +1,5 @@ 
 #include "../git-compat-util.h"
+#include "../abspath.h"
 #include "../copy.h"
 #include "../environment.h"
 #include "../gettext.h"
@@ -3437,13 +3438,15 @@  typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
 /*
  * Check the symref "pointee_name" and "pointee_path". The caller should
  * make sure that "pointee_path" is absolute. For symbolic ref, "pointee_name"
- * would be the content after "refs:".
+ * would be the content after "refs:". For symblic link, "pointee_name" would
+ * be the relative path agaignst "gitdir".
  */
 static int files_fsck_symref_target(struct fsck_options *o,
 				    struct fsck_ref_report *report,
 				    const char *refname,
 				    struct strbuf *pointee_name,
-				    struct strbuf *pointee_path)
+				    struct strbuf *pointee_path,
+				    unsigned int symbolic_link)
 {
 	const char *newline_pos = NULL;
 	const char *p = NULL;
@@ -3458,24 +3461,28 @@  static int files_fsck_symref_target(struct fsck_options *o,
 		goto out;
 	}
 
-	newline_pos = strrchr(p, '\n');
-	if (!newline_pos || *(newline_pos + 1)) {
-		ret = fsck_report_ref(o, report,
-				      FSCK_MSG_REF_MISSING_NEWLINE,
-				      "missing newline");
+	if (!symbolic_link) {
+		newline_pos = strrchr(p, '\n');
+		if (!newline_pos || *(newline_pos + 1)) {
+			ret = fsck_report_ref(o, report,
+					      FSCK_MSG_REF_MISSING_NEWLINE,
+					      "missing newline");
+		}
 	}
 
 	if (check_refname_format(pointee_name->buf, 0)) {
-		/*
-		 * When containing null-garbage, "check_refname_format" will
-		 * fail, we should trim the "pointee" to check again.
-		 */
-		strbuf_rtrim(pointee_name);
-		if (!check_refname_format(pointee_name->buf, 0)) {
-			ret = fsck_report_ref(o, report,
-					      FSCK_MSG_TRAILING_REF_CONTENT,
-					      "trailing null-garbage");
-			goto out;
+		if (!symbolic_link) {
+			/*
+			* When containing null-garbage, "check_refname_format" will
+			* fail, we should trim the "pointee" to check again.
+			*/
+			strbuf_rtrim(pointee_name);
+			if (!check_refname_format(pointee_name->buf, 0)) {
+				ret = fsck_report_ref(o, report,
+						      FSCK_MSG_TRAILING_REF_CONTENT,
+						      "trailing null-garbage");
+				goto out;
+			}
 		}
 
 		ret = fsck_report_ref(o, report,
@@ -3510,9 +3517,12 @@  static int files_fsck_refs_content(struct ref_store *ref_store,
 {
 	struct strbuf pointee_path = STRBUF_INIT;
 	struct strbuf ref_content = STRBUF_INIT;
+	struct strbuf abs_gitdir = STRBUF_INIT;
 	struct strbuf referent = STRBUF_INIT;
 	struct strbuf refname = STRBUF_INIT;
 	struct fsck_ref_report report = {0};
+	const char *pointee_name = NULL;
+	unsigned int symbolic_link = 0;
 	const char *trailing = NULL;
 	unsigned int type = 0;
 	int failure_errno = 0;
@@ -3557,16 +3567,38 @@  static int files_fsck_refs_content(struct ref_store *ref_store,
 				    ref_store->gitdir, referent.buf);
 			ret = files_fsck_symref_target(o, &report, refname.buf,
 						       &referent,
-						       &pointee_path);
+						       &pointee_path,
+						       symbolic_link);
 		}
 		goto cleanup;
 	}
 
+	symbolic_link = 1;
+
+	strbuf_add_real_path(&pointee_path, iter->path.buf);
+	strbuf_add_absolute_path(&abs_gitdir, ref_store->gitdir);
+	strbuf_normalize_path(&abs_gitdir);
+	if (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1]))
+		strbuf_addch(&abs_gitdir, '/');
+
+	if (!skip_prefix(pointee_path.buf, abs_gitdir.buf, &pointee_name)) {
+		ret = fsck_report_ref(o, &report,
+				      FSCK_MSG_BAD_SYMREF_POINTEE,
+				      "point to target outside gitdir");
+		goto cleanup;
+	}
+
+	strbuf_addstr(&referent, pointee_name);
+	ret = files_fsck_symref_target(o, &report, refname.buf,
+				       &referent, &pointee_path,
+				       symbolic_link);
+
 cleanup:
 	strbuf_release(&refname);
 	strbuf_release(&ref_content);
 	strbuf_release(&referent);
 	strbuf_release(&pointee_path);
+	strbuf_release(&abs_gitdir);
 	return ret;
 }
 
diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index 69280795ca..36992fbc7f 100755
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -230,4 +230,48 @@  test_expect_success 'symbolic ref content should be checked' '
 	test_cmp expect err
 '
 
+test_expect_success SYMLINKS 'symbolic ref (symbolic link) content should be checked' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	branch_dir_prefix=.git/refs/heads &&
+	tag_dir_prefix=.git/refs/tags &&
+	cd repo &&
+	git commit --allow-empty -m initial &&
+	git checkout -b branch-1 &&
+	git tag tag-1 &&
+	git checkout -b a/b/branch-2 &&
+
+	ln -sf ../../../../branch $branch_dir_prefix/branch-symbolic &&
+	test_must_fail git refs verify 2>err &&
+	cat >expect <<-EOF &&
+	error: refs/heads/branch-symbolic: badSymrefPointee: point to target outside gitdir
+	EOF
+	rm $branch_dir_prefix/branch-symbolic &&
+	test_cmp expect err &&
+
+	ln -sf ../../logs/branch-bad $branch_dir_prefix/branch-symbolic &&
+	test_must_fail git refs verify 2>err &&
+	cat >expect <<-EOF &&
+	error: refs/heads/branch-symbolic: badSymrefPointee: points to ref outside the refs directory
+	EOF
+	rm $branch_dir_prefix/branch-symbolic &&
+	test_cmp expect err &&
+
+	ln -sf ./"branch   space" $branch_dir_prefix/branch-symbolic &&
+	test_must_fail git refs verify 2>err &&
+	cat >expect <<-EOF &&
+	error: refs/heads/branch-symbolic: badSymrefPointee: points to refname with invalid format
+	EOF
+	rm $branch_dir_prefix/branch-symbolic &&
+	test_cmp expect err &&
+
+	ln -sf ./".branch" $branch_dir_prefix/branch-symbolic &&
+	test_must_fail git refs verify 2>err &&
+	cat >expect <<-EOF &&
+	error: refs/heads/branch-symbolic: badSymrefPointee: points to refname with invalid format
+	EOF
+	rm $branch_dir_prefix/branch-symbolic &&
+	test_cmp expect err
+'
+
 test_done