From patchwork Sun Aug 18 15:01:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13767455 Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2585F442C for ; Sun, 18 Aug 2024 15:00:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993258; cv=none; b=KFdJUGX5O0XJIvRtc/3Gmi4sqQXw/8idKd+FMqCU+dneH+p2RptHv1vJONcFopQ3cBGNTWAhRnNoPoex8Zybw09/QxeKoOU/S3L5ThmR9sNDTue4U+vl1IYapHhIzrLmSZhH5Ytomm4DR2Rp3Rk3LWkHx+9NhIi9r6pQZ5cosJM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993258; c=relaxed/simple; bh=oc3J+x3Cb4etv0CKFpLB1eyBt0mOWkPLxfTUX7SDKJ0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=itTQw/USrwRSVclNGxUBR/dm3KQ70KwobAG65oz/4FbvY3RSQk82l9/oOyKQCDCq2hMPqBYItRwwg1jYErppIfv60bwUgLEh6Fi6bTMUeaNMvhZze5VR8OyjwwxrppUZS5vjrxRDYM0TNrXFr/6WwBv18B/nyGuDQKT1Wkr7n68= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=hfYKOYt4; arc=none smtp.client-ip=209.85.210.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hfYKOYt4" Received: by mail-pf1-f181.google.com with SMTP id d2e1a72fcca58-710e39961f4so2287786b3a.3 for ; Sun, 18 Aug 2024 08:00:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723993256; x=1724598056; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=VW8oDhZXfBP7ucD9l6y2jGB3MYaxB0TVGcGEZ01xKRk=; b=hfYKOYt4lFOTqtgoKA8e2djGarEXbR3KkGlAzid9aZaUdJ7a/Pw/U1kguWfEkeXRQ5 P3vFBV8gnS37g/yFG7FzHN0VJiQy7kcmXMV6DeOB5IBm7NwTKSm0OI4BvXBCeOAcTYJO nVu9uZJLZ5m8wDvTY599P8pLLAs1YsLI8AzQOI/wxKTpIXk6DLPE9M6JRZZSKI2nFPBM zJWnFMTVNsI+je89PbL2FEpq7ZVeiGGNdRtpQC90aVwbYaq9PkI2XOCn6xM8mQp31HVw jkE7WueMum0uqEm47zKHpewzT9/yanzKncnr2orSOJIOhnYY26l/vW9qS+yGzG0jJ2uL H0JQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723993256; x=1724598056; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=VW8oDhZXfBP7ucD9l6y2jGB3MYaxB0TVGcGEZ01xKRk=; b=lz+TeEEaWLaMYR4Gio7haxHTIMQ5Rnqytu/+eIYr5kUd1TLXkMmi88dP4hXjY+mIhf MOj5HF74aZFzlqvLTPIYu1TaVLjV1PCRNVszp+ReIB9UmM5Kfiqdn7Sy20mUiRlYtMsT mqquZ+pSgzLA64t1KKNEGqtctLQq+g0TviQbaxDE03dENqzeADMZ0qU2hmC/Q3528SzW 21icO1yczsVZu12bJbzF2oNDstx/CfwJfktBjyRf2/CQv8MwF1fFJjW2dkyQn/g4hz4R eDfNA6MngEaTY0moG5f4Ip3DDPx5boEOp52sdoQtj6r2xMGDA+E532qG2RFw5u3QLgBV 23Og== X-Gm-Message-State: AOJu0YxlcuySt6Bs3ehYzYHV7d8ZYVj0QZvJEVGJbAE2AsP3zLn/7JrR 2DaGVP3wj+PdC1pS3unrAX1iYSDTlJbLiD86Jl0krx7Ls+b9rrWxFaMFvA== X-Google-Smtp-Source: AGHT+IEFVDOh4QdoOG4W8In9acnhVahLLZ2pbr+TJmu6Dihp5MvganS2z8spy2UL8yzmVk+vKNcFZA== X-Received: by 2002:a05:6a21:3a81:b0:1c4:a2a7:b18e with SMTP id adf61e73a8af0-1c904fb5106mr8277618637.30.1723993255630; Sun, 18 Aug 2024 08:00:55 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2d3e3171bd3sm5601976a91.31.2024.08.18.08.00.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 18 Aug 2024 08:00:55 -0700 (PDT) Date: Sun, 18 Aug 2024 23:01:36 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v1 1/4] fsck: introduce "FSCK_REF_REPORT_DEFAULT" macro Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "fsck.c::fsck_refs_error_function", we need to tell whether "oid" and "referent" is NULL. So, we need to always initialize these parameters to NULL instead of letting them point to anywhere when creating a new "fsck_ref_report" structure. In order to conveniently create a new "fsck_ref_report", add a new macro "FSCK_REF_REPORT_DEFAULT". Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- fsck.h | 6 ++++++ refs/files-backend.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/fsck.h b/fsck.h index 500b4c04d2..8894394d16 100644 --- a/fsck.h +++ b/fsck.h @@ -152,6 +152,12 @@ struct fsck_ref_report { const char *referent; }; +#define FSCK_REF_REPORT_DEFAULT { \ + .path = NULL, \ + .oid = NULL, \ + .referent = NULL, \ +} + struct fsck_options { fsck_walk_func walk; fsck_error error_func; diff --git a/refs/files-backend.c b/refs/files-backend.c index 8d6ec9458d..725a4f52e3 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3446,7 +3446,7 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, goto cleanup; if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) { - struct fsck_ref_report report = { .path = NULL }; + struct fsck_ref_report report = FSCK_REF_REPORT_DEFAULT; strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); report.path = sb.buf; From patchwork Sun Aug 18 15:01:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13767456 Received: from mail-pf1-f176.google.com (mail-pf1-f176.google.com [209.85.210.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 916192030A for ; Sun, 18 Aug 2024 15:01:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993269; cv=none; b=jlSJ6YzG9UOUQ4x1F6JDH8umo5bc1Mp7fHM6zQU+omoteKxGWXNWXoKnQTvvRFm7QEnLbKsFZ7OyLEK8yVke7911Ng75nWQugz/LQbxdV6NpmHZnPHAjP6udMAyoIFdKe/XmCnj7hfCXwziVn7fBYE0JY6q4LjG0RNybltF4XNs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993269; c=relaxed/simple; bh=w2TzmjcV4fOxOVN6dZ1JLqPOvrRiMez3zw71tNo+/Hs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kf8QK9SWFZcn+ictSdCxh56+1yKYChiTX12VspBJkBtaKfMeLh1kkd1HT82fEStlB7YZxDPDqT/J9Up7mqaTb5OVD0lqh5wAztn3ATsnfymRM5HPJwULyEqN/oObZTMrnOd/alD6u9eok6D9eW/2MY8upxhIK4cpABVMjyjwT4k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=DgR3jY1Z; arc=none smtp.client-ip=209.85.210.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DgR3jY1Z" Received: by mail-pf1-f176.google.com with SMTP id d2e1a72fcca58-713eeb4e4bcso336792b3a.0 for ; Sun, 18 Aug 2024 08:01:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723993266; x=1724598066; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=5PlMMI+oi7JmHvdTuaebNPWFQG5nQnFZ/4eBxAVL5Pc=; b=DgR3jY1Zk2fFvHWpiP+5H3X9n4roY+TjIFu+51OEbLwkakmFChtVZnAB2aJAc+d6Y1 AfU3jXyck1D9939KaKzntwfvHvqFpDdVpZhtv2b7ni8fsHK9LWQUhifPj7hltLoHTrH2 FQNBdk7qpETITCyRK6AoFj7SGom3qjoZleeilWP7QDdhwtYUqwFdICieoM8ysBs+TipC lURZv9GtdUdhdYkVpm069KXgO7Hw1HWUIr2yXi0Q8n4HTrFkgir2mQDP58XxDNOVCc8u KERpB65tzJNJhrE+7Mdx2vu+MJnBn94Qa3EzWtIjshi9ATm2SIzrhDtUZtt3hmTPCuhb NvHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723993266; x=1724598066; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=5PlMMI+oi7JmHvdTuaebNPWFQG5nQnFZ/4eBxAVL5Pc=; b=GK/Vm4oD1/QgqG02IRXfNqBpR20a2QTzLfykichbOcedKIxdfPLJjhIPN90Vx6Ls8W rQFkxzu28w07hdzTs8CXrjF2vowE36LrSDGmQOwp7Z9xZUZ+il9tdtLjyJTD1EWLlcc1 fU/avjR8jimtyVsbHl5I7MTORlrV9badLoEtCE/wGpnLwWuA1CEHuDrTPZ6tJrlJJFtM p8R98AB7bRuAr3E9sjyNQpNFq0qTNjjqbyVO+Tewv8LNSCjevpRq+FCK9z8e+PvEIbUt oJy2Nh1eqqvhDK+PhBVfJpE4sqjA8ObkGQjtT8vKlnIxr5KARhER9KPbZp9tfoJHJZzq tfBQ== X-Gm-Message-State: AOJu0YwEaZe+23P0x4TNl88adNeK5TgLlnJXYnf5A6FSJ9/lhY+tWorq jqrZ5RkH5+uV6+TNjowBvCPVnIOFSske75DDBIpMCciLdEeZRZeTuqoXBQ== X-Google-Smtp-Source: AGHT+IHffoOzDv0RY32fo8f4ok+S4Azr91Iih+Sjm0aeubnWS3I1mBWnVJls+aMlfIj0qjvwjh/G+g== X-Received: by 2002:a05:6a20:9f0a:b0:1c8:da09:5311 with SMTP id adf61e73a8af0-1c904f7eb81mr11984975637.4.1723993264698; Sun, 18 Aug 2024 08:01:04 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7127ae06bb7sm5332449b3a.58.2024.08.18.08.01.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 18 Aug 2024 08:01:04 -0700 (PDT) Date: Sun, 18 Aug 2024 23:01:44 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v1 2/4] ref: add regular ref content check for files backend Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We implicitly reply on "git-fsck(1)" to check the consistency of regular refs. However, when parsing the regular refs for files backend, we allow the ref content to end with no newline or contain some garbages. We should warn the user about above situations. In order to provide above functionality, enhance the "git-refs verify" command by adding consistency check for regular refs for files backend. Add the following three fsck messages to represent the above situations: 1. "badRefContent(ERROR)": A ref has a bad content. 2. "refMissingNewline(WARN)": A valid ref does not end with newline. 3. "trailingRefContent(WARN)": A ref has trailing contents. In order to tell whether the ref has trailing content, add a new parameter "trailing" to "parse_loose_ref_contents". Then introduce a new function "files_fsck_refs_content" to check the regular refs. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 9 ++++ fsck.h | 3 ++ refs.c | 2 +- refs/files-backend.c | 67 ++++++++++++++++++++++++++- refs/refs-internal.h | 2 +- t/t0602-reffiles-fsck.sh | 87 +++++++++++++++++++++++++++++++++++ 6 files changed, 166 insertions(+), 4 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 68a2801f15..1688c2f1fe 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -19,6 +19,9 @@ `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. +`badRefContent`:: + (ERROR) A ref has a bad content. + `badRefFiletype`:: (ERROR) A ref has a bad file type. @@ -170,6 +173,12 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`refMissingNewline`:: + (WARN) A valid ref does not end with newline. + +`trailingRefContent`:: + (WARN) A ref has trailing contents. + `treeNotSorted`:: (ERROR) A tree is not properly sorted. diff --git a/fsck.h b/fsck.h index 8894394d16..975d9b9da9 100644 --- a/fsck.h +++ b/fsck.h @@ -31,6 +31,7 @@ enum fsck_msg_type { FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ + FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ @@ -73,6 +74,8 @@ enum fsck_msg_type { FUNC(HAS_DOTDOT, WARN) \ FUNC(HAS_DOTGIT, WARN) \ FUNC(NULL_SHA1, WARN) \ + FUNC(REF_MISSING_NEWLINE, WARN) \ + FUNC(TRAILING_REF_CONTENT, WARN) \ FUNC(ZERO_PADDED_FILEMODE, WARN) \ FUNC(NUL_IN_COMMIT, WARN) \ FUNC(LARGE_PATHNAME, WARN) \ diff --git a/refs.c b/refs.c index 74de3d3009..5e74881945 100644 --- a/refs.c +++ b/refs.c @@ -1758,7 +1758,7 @@ static int refs_read_special_head(struct ref_store *ref_store, } result = parse_loose_ref_contents(ref_store->repo->hash_algo, content.buf, - oid, referent, type, failure_errno); + oid, referent, type, NULL, failure_errno); done: strbuf_release(&full_path); diff --git a/refs/files-backend.c b/refs/files-backend.c index 725a4f52e3..ae71692f36 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -560,7 +560,7 @@ static int read_ref_internal(struct ref_store *ref_store, const char *refname, buf = sb_contents.buf; ret = parse_loose_ref_contents(ref_store->repo->hash_algo, buf, - oid, referent, type, &myerr); + oid, referent, type, NULL, &myerr); out: if (ret && !myerr) @@ -597,7 +597,7 @@ static int files_read_symbolic_ref(struct ref_store *ref_store, const char *refn int parse_loose_ref_contents(const struct git_hash_algo *algop, const char *buf, struct object_id *oid, struct strbuf *referent, unsigned int *type, - int *failure_errno) + const char **trailing, int *failure_errno) { const char *p; if (skip_prefix(buf, "ref:", &buf)) { @@ -619,6 +619,10 @@ int parse_loose_ref_contents(const struct git_hash_algo *algop, *failure_errno = EINVAL; return -1; } + + if (trailing) + *trailing = p; + return 0; } @@ -3430,6 +3434,64 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refs_check_dir, struct dir_iterator *iter); +static int files_fsck_refs_content(struct ref_store *ref_store, + struct fsck_options *o, + const char *refs_check_dir, + struct dir_iterator *iter) +{ + struct fsck_ref_report report = FSCK_REF_REPORT_DEFAULT; + struct strbuf ref_content = STRBUF_INIT; + struct strbuf referent = STRBUF_INIT; + struct strbuf refname = STRBUF_INIT; + const char *trailing = NULL; + unsigned int type = 0; + int failure_errno = 0; + struct object_id oid; + int ret = 0; + + strbuf_addf(&refname, "%s/%s", refs_check_dir, iter->relative_path); + report.path = refname.buf; + + if (S_ISREG(iter->st.st_mode)) { + if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { + ret = error_errno(_("%s/%s: unable to read the ref"), + refs_check_dir, iter->relative_path); + goto cleanup; + } + + if (parse_loose_ref_contents(ref_store->repo->hash_algo, + ref_content.buf, &oid, &referent, + &type, &trailing, &failure_errno)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "invalid ref content"); + goto cleanup; + } + + if (!(type & REF_ISSYMREF)) { + if (*trailing == '\0') { + ret = fsck_report_ref(o, &report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + goto cleanup; + } + + if (*trailing != '\n' || (*(trailing + 1) != '\0')) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing garbage in ref"); + goto cleanup; + } + } + } + +cleanup: + strbuf_release(&refname); + strbuf_release(&ref_content); + strbuf_release(&referent); + return ret; +} + static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, struct fsck_options *o, const char *refs_check_dir, @@ -3512,6 +3574,7 @@ static int files_fsck_refs(struct ref_store *ref_store, { files_fsck_refs_fn fsck_refs_fn[]= { files_fsck_refs_name, + files_fsck_refs_content, NULL, }; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2313c830d8..73b05f971b 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -715,7 +715,7 @@ struct ref_store { int parse_loose_ref_contents(const struct git_hash_algo *algop, const char *buf, struct object_id *oid, struct strbuf *referent, unsigned int *type, - int *failure_errno); + const char **trailing, int *failure_errno); /* * Fill in the generic part of refs and add it to our collection of diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 71a4d1a5ae..7c1910d784 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -89,4 +89,91 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' test_must_be_empty err ' +test_expect_success 'regular ref 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 commit --allow-empty -m second && + git checkout -b branch-2 && + git tag tag-2 && + git checkout -b a/b/tag-2 && + + printf "%s" "$(git rev-parse branch-1)" > $branch_dir_prefix/branch-1-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-1-no-newline: refMissingNewline: missing newline + EOF + rm $branch_dir_prefix/branch-1-no-newline && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse branch-1)" > $branch_dir_prefix/branch-1-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/branch-1-garbage && + test_cmp expect err && + + printf "%s\n\n\n" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-1-garbage && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-1-garbage && + test_cmp expect err && + + printf "%s garbage\n\na" "$(git rev-parse tag-2)" > $tag_dir_prefix/tag-2-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-2-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-2-garbage && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-garbage && + test_must_fail git -c fsck.trailingRefContent=error refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-1-garbage && + test_cmp expect err && + + printf "%sx" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-1-bad: badRefContent: invalid ref content + EOF + rm $tag_dir_prefix/tag-1-bad && + test_cmp expect err && + + printf "xfsazqfxcadas" > $tag_dir_prefix/tag-2-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-2-bad: badRefContent: invalid ref content + EOF + rm $tag_dir_prefix/tag-2-bad && + test_cmp expect err && + + printf "xfsazqfxcadas" > $branch_dir_prefix/a/b/branch-2-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-2-bad: badRefContent: invalid ref content + EOF + rm $branch_dir_prefix/a/b/branch-2-bad && + test_cmp expect err +' + test_done From patchwork Sun Aug 18 15:01:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13767457 Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E74B22030A for ; Sun, 18 Aug 2024 15:01:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.47 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993275; cv=none; b=ZcOOvGrzqtU2fFQmkQXc1P6vzuVu9JKsJAe1hz0r24UMXZAmKnQA6r2j2cRvOWALw7929g+jmFvKR1AFIET9Rg6suI7h8D8CW1V7SwPwJPs6lUIKjiDifr/TpvD/EI09VHI6yqZ0z7WUai8Ip8chR+U6+8rtqZRwIOyYRRKL5fM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993275; c=relaxed/simple; bh=Zz+nnlGp2ks0g1HFORfhfEwpHfemz4GyC6CftzKBQJQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LYsY+Eb9HJ4yNX9SLN4s9iUBeMOOWw6blyyBcP9asInvwo93RH20xqiMxT0FBC6UGrtwMMTZHq0t0QTLaifGuCHcEJ3ThVERItGBSk+woT2jcu6MQnqgxaJg5iHd/UAQBwcz+JgeVU+v6zeXxKqcup2ppUmlzjjVnXNwPmgsgaU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=aUGA3+nw; arc=none smtp.client-ip=209.85.216.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="aUGA3+nw" Received: by mail-pj1-f47.google.com with SMTP id 98e67ed59e1d1-2d3c05dc63eso2585389a91.0 for ; Sun, 18 Aug 2024 08:01:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723993272; x=1724598072; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=g1B5FyzX7CLB7sr9powUtRqhJHBICxPMVl7lEh95rJs=; b=aUGA3+nwhGt2aHzqubsjUCiRcfYCzswpEjkQjFKY4Jy3HEOBk09B9CEXKZkBsBDXX/ uw9cjHvJ+DL/DLFfd/z+TCVgzsv7Vb5rpdZBprizqqhlT6sGnmfHTQ1DBrZ5+U58eKYJ XWhaW0YT1mlmWC3MtyDgE61zVbejO+nGb5j14/vxHXuDoCm821Hfkib5vddpJvDneeiU OkDpQWMgpSQJ5ssuKlwJpiqrHpViWda1ZDssq2elXJGrp0gv33LKYJ0FJeGm/5jMGCNi A1xV93MtfZi9QVvXALLUA/XXxFyOHKmhcBvvBJasyuG9eAcTTQO1U2FfvmHFyD0vTUL+ b+kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723993272; x=1724598072; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=g1B5FyzX7CLB7sr9powUtRqhJHBICxPMVl7lEh95rJs=; b=LAuhDc60pd7gDWzHMIVjnnSLQhvt1jzFu6BA4DvHcYh/T7oPD30eU9nFaTkPj5IQbD Prlb2raQ+DpYBcv1wHKkmCYxbSVh2dBqAnBHpryfsZKClY54lkjbK0vqEvX9NAVF0HxP iFkSCr/Q3z9m9tPuwyxPx/mUxPOK6YIBA9z0pab6HlfFCFy6c4HgX9Rr4+G+36cPD7eD llpMidS0ITobE5gOr3nDb6FZ0tcJ7QwAR8FOYtKDjw0oZkwqSsBhZI9Ym2OSjSEKoN8o L4dM68LOu3dlJqGKWkkwb2zgSUO1ZSV2Kpi1jRvGzFErB0ur5QNhwoD0MY2nXBeHN8rD ge6g== X-Gm-Message-State: AOJu0Yxw8UjBe6TxvEHaKtnj0O9X64l676yEzTw6F9yRZ9K1AHwR23eo nEJJz//T5QqxmS4fDfsGxUke32DD71qfwhLoSDH3INT7uh5u2v0uBgpagA== X-Google-Smtp-Source: AGHT+IGN4ar71xQeXrBXg4fqeEwItKc/zhETpF4Mm64Q+I34d2ScS5TupGGSWrYAHZKiBZaOaLE2fg== X-Received: by 2002:a17:90a:fa8e:b0:2d4:6ef:cb14 with SMTP id 98e67ed59e1d1-2d406efccc1mr5080469a91.28.1723993271896; Sun, 18 Aug 2024 08:01:11 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2d3d0b3b6fcsm6667045a91.43.2024.08.18.08.01.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 18 Aug 2024 08:01:11 -0700 (PDT) Date: Sun, 18 Aug 2024 23:01:52 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v1 3/4] ref: add symbolic ref content check for files backend Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We have already introduced the checks for regular refs. There is no need to check the consistency of the target which the symbolic ref points to. Instead, we just check the content of the symbolic ref itself. In order to check the content of the symbolic ref, create a function "files_fsck_symref_target". It will first check whether the "pointee" is under the "refs/" directory and then we will check the "pointee" itself. There is no specification about the content of the symbolic ref. Although we do write "ref: %s\n" to create a symbolic ref by using "git-symbolic-ref(1)" command. However, this is not mandatory. We still accept symbolic refs with null trailing garbage. Put it more specific, the following are correct: 1. "ref: refs/heads/master " 2. "ref: refs/heads/master \n \n" 3. "ref: refs/heads/master\n\n" But we do not allow any non-null trailing garbage. The following are bad symbolic contents. 1. "ref: refs/heads/master garbage\n" 2. "ref: refs/heads/master \n\n\n garbage " In order to provide above checks, we will traverse the "pointee" to report the user whether this is null-garbage or no newline. And if symbolic refs contain non-null garbage, we will report "FSCK_MSG_BAD_REF_CONTENT" to the user. Then, we will check the name of the "pointee" is correct by using "check_refname_format". And then if we can access the "pointee_path" in the file system, we should ensure that the file type is correct. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 ++ fsck.h | 1 + refs/files-backend.c | 87 +++++++++++++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 52 +++++++++++++++++++++ 4 files changed, 143 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 1688c2f1fe..73587661dc 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -28,6 +28,9 @@ `badRefName`:: (ERROR) A ref has an invalid format. +`badSymrefPointee`:: + (ERROR) The pointee of a symref is bad. + `badTagName`:: (INFO) A tag has an invalid format. diff --git a/fsck.h b/fsck.h index 975d9b9da9..985b674dd9 100644 --- a/fsck.h +++ b/fsck.h @@ -34,6 +34,7 @@ enum fsck_msg_type { FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ + FUNC(BAD_SYMREF_POINTEE, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ FUNC(BAD_TREE, ERROR) \ FUNC(BAD_TREE_SHA1, ERROR) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index ae71692f36..bfb8d338d2 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3434,12 +3434,92 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refs_check_dir, struct dir_iterator *iter); +/* + * 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:". + */ +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) +{ + unsigned int newline_num = 0; + unsigned int space_num = 0; + const char *p = NULL; + struct stat st; + int ret = 0; + + if (!skip_prefix(pointee_name->buf, "refs/", &p)) { + + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "points to ref outside the refs directory"); + goto out; + } + + while (*p != '\0') { + if ((space_num || newline_num) && !isspace(*p)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_REF_CONTENT, + "contains non-null garbage"); + goto out; + } + + if (*p == '\n') { + newline_num++; + } else if (*p == ' ') { + space_num++; + } + p++; + } + + if (space_num || newline_num > 1) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing null-garbage"); + } else if (!newline_num) { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + } + + strbuf_rtrim(pointee_name); + + if (check_refname_format(pointee_name->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "points to refname with invalid format"); + } + + /* + * Missing target should not be treated as any error worthy event and + * not even warn. It is a common case that a symbolic ref points to a + * ref that does not exist yet. If the target ref does not exist, just + * skip the check for the file type. + */ + if (lstat(pointee_path->buf, &st) < 0) + goto out; + + if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "points to an invalid file type"); + goto out; + } + +out: + return ret; +} + static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_options *o, const char *refs_check_dir, struct dir_iterator *iter) { struct fsck_ref_report report = FSCK_REF_REPORT_DEFAULT; + struct strbuf pointee_path = STRBUF_INIT; struct strbuf ref_content = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct strbuf refname = STRBUF_INIT; @@ -3482,6 +3562,12 @@ static int files_fsck_refs_content(struct ref_store *ref_store, "trailing garbage in ref"); goto cleanup; } + } else { + strbuf_addf(&pointee_path, "%s/%s", + ref_store->gitdir, referent.buf); + ret = files_fsck_symref_target(o, &report, refname.buf, + &referent, + &pointee_path); } } @@ -3489,6 +3575,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, strbuf_release(&refname); strbuf_release(&ref_content); strbuf_release(&referent); + strbuf_release(&pointee_path); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 7c1910d784..e8fc2ef015 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -176,4 +176,56 @@ test_expect_success 'regular ref content should be checked' ' test_cmp expect err ' +test_expect_success 'symbolic ref 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 && + + printf "ref: refs/heads/branch" > $branch_dir_prefix/branch-1-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-1-no-newline: refMissingNewline: missing newline + EOF + rm $branch_dir_prefix/branch-1-no-newline && + test_cmp expect err && + + printf "ref: refs/heads/branch " > $branch_dir_prefix/a/b/branch-trailing && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing: trailingRefContent: trailing null-garbage + EOF + rm $branch_dir_prefix/a/b/branch-trailing && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" > $branch_dir_prefix/a/b/branch-trailing && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing: trailingRefContent: trailing null-garbage + EOF + rm $branch_dir_prefix/a/b/branch-trailing && + test_cmp expect err && + + printf "ref: refs/heads/branch \n\n " > $branch_dir_prefix/a/b/branch-trailing && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing: trailingRefContent: trailing null-garbage + EOF + rm $branch_dir_prefix/a/b/branch-trailing && + test_cmp expect err && + + printf "ref: refs/heads/.branch\n" > $branch_dir_prefix/branch-2-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-2-bad: badSymrefPointee: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-2-bad && + test_cmp expect err +' + test_done From patchwork Sun Aug 18 15:02:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13767458 Received: from mail-pf1-f174.google.com (mail-pf1-f174.google.com [209.85.210.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DAEB61DA21 for ; Sun, 18 Aug 2024 15:01:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993282; cv=none; b=fhGifnV8sunTkmE8CCefh64aUop9hp59VbQJbaNZYzkh4h+jHGK5z6ISMTCKAHhOQpqB2ttM3Q+lZKeQYppZ4X0Vt+Q/bOKqli+sDzev69WbXq5mmzJMIbPAjiIeMtNrUm71UXPAOdIJWpQzYOlVPMgq4kfgTHmtXhYaMMILpe4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723993282; c=relaxed/simple; bh=q4FTt78BvEY0oOnAG/otUtSUNl0K5eei994covvB1zg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uTqbqA7/1zlMC0Jxc+NHYZoKyTiJHN/DOWb5NU0ZRAeOwWMcLuQhw6K2ZXQYM6LAAhQtvF+gxN0nnpZumMq1tbcOPLMLX3Pr3197I+o6kA7gsA4lx49wUKwHFfiYL6LKR/r/Krg913U6NFpsdv5qau4FuDXFqu6mc6rX80V40W8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Y52nqJ1v; arc=none smtp.client-ip=209.85.210.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Y52nqJ1v" Received: by mail-pf1-f174.google.com with SMTP id d2e1a72fcca58-710ffaf921fso2223701b3a.1 for ; Sun, 18 Aug 2024 08:01:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723993280; x=1724598080; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=EcNTuNXb7qa8AAgktcTFmjFCMbe3wM5N69E31da+ryg=; b=Y52nqJ1vRPCCu7yEv+ddr6PxADFAy+VQYQcU4sIiw3+vAyqRUQ3bNE4TDyVmCNPt7G yrNv7dK2r2niabVHgJ1Jec+eqhwm51MP6r5wHh5SQ8NpMxH0WL2RSM3kTsqqYzH9cuiW av8683+xNb+rg8IKmmf0aCmENE23wPZV2YkhZ0tvKPO7rlg79pMGCs8HCce2pyQXXq7k 3fdiTZyXrzVkCKuejauDSFXkxgg0B3RGC1fN5Q2Kkat9Y3SJmmoOFRbY89PFP+u3VHJv CF/06WGsFPYJaUsoPltQO+9D0L0mmgIe57Z1sLoldYGJXO8a5vCsHOTmIXZ+3YLkuDtD v6dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723993280; x=1724598080; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=EcNTuNXb7qa8AAgktcTFmjFCMbe3wM5N69E31da+ryg=; b=gluzebql4R1eu+xrrFr5Lrjpozas8kYUaw0Q8Kf5K6Ubb7oMSfq55zPxQvvfltTTvH jJkP5iIh0ANbtFnZBxvV6LCB42DBsj2qmLp7p5eN4fDAr4C9CVRQdduL9gAew9x0IfLI 7rx7PE4aFPrie/yCuHmhpXZ4oYRjB2v2KLvDkxG4fxJFm9/eHGNcEQy/2PYKD37gX13y UxmDKTzC/vehhG4xHaT4o/8kA3PM2w/IoJ2ch0CyNmTTnpYt6kHI5C11AEpU69wITUXG sAlQzrR0o8+kiKfBGtkX1Gqy8lwLLOqO8dP6cvdDbfFNq2sQsyWmsLwE46iVoAYnU/hX 7bBw== X-Gm-Message-State: AOJu0YxsQ2E/XTkCI8m9btY/5vgqqS9DJA6JYBzLgdnG6Dahy2OCoaH/ 2a4IhMPsBt5JQlr2oA5of+ipvnUFG553MrqRmYxzygVtl05YFCEeGXJJ1g== X-Google-Smtp-Source: AGHT+IF7i0UG2ZpKHVoIEXdHofcvErAki3BHpNzMFMhtDjq/VhmiUPD6YpDIDs5h+wSrQfCXS3qY8A== X-Received: by 2002:a05:6a00:4f01:b0:706:5daf:efa5 with SMTP id d2e1a72fcca58-71276f6321dmr18097287b3a.9.1723993279395; Sun, 18 Aug 2024 08:01:19 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-713f3c35cf0sm418629b3a.173.2024.08.18.08.01.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 18 Aug 2024 08:01:18 -0700 (PDT) Date: Sun, 18 Aug 2024 23:02:00 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v1 4/4] ref: add symlink ref consistency check for files backend Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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 Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 82 ++++++++++++++++++++++++++++------------ t/t0602-reffiles-fsck.sh | 44 +++++++++++++++++++++ 2 files changed, 101 insertions(+), 25 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index bfb8d338d2..398afedaf0 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) { unsigned int newline_num = 0; unsigned int space_num = 0; @@ -3459,34 +3462,36 @@ static int files_fsck_symref_target(struct fsck_options *o, goto out; } - while (*p != '\0') { - if ((space_num || newline_num) && !isspace(*p)) { - ret = fsck_report_ref(o, report, - FSCK_MSG_BAD_REF_CONTENT, - "contains non-null garbage"); - goto out; + if (!symbolic_link) { + while (*p != '\0') { + if ((space_num || newline_num) && !isspace(*p)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_REF_CONTENT, + "contains non-null garbage"); + goto out; + } + + if (*p == '\n') { + newline_num++; + } else if (*p == ' ') { + space_num++; + } + p++; } - if (*p == '\n') { - newline_num++; - } else if (*p == ' ') { - space_num++; + if (space_num || newline_num > 1) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing null-garbage"); + } else if (!newline_num) { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); } - p++; - } - if (space_num || newline_num > 1) { - ret = fsck_report_ref(o, report, - FSCK_MSG_TRAILING_REF_CONTENT, - "trailing null-garbage"); - } else if (!newline_num) { - ret = fsck_report_ref(o, report, - FSCK_MSG_REF_MISSING_NEWLINE, - "missing newline"); + strbuf_rtrim(pointee_name); } - strbuf_rtrim(pointee_name); - if (check_refname_format(pointee_name->buf, 0)) { ret = fsck_report_ref(o, report, FSCK_MSG_BAD_SYMREF_POINTEE, @@ -3521,8 +3526,10 @@ static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_ref_report report = FSCK_REF_REPORT_DEFAULT; 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; + unsigned int symbolic_link = 0; const char *trailing = NULL; unsigned int type = 0; int failure_errno = 0; @@ -3567,8 +3574,32 @@ 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); + } + } else if (S_ISLNK(iter->st.st_mode)) { + const char *pointee_name = NULL; + + 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: @@ -3576,6 +3607,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, 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 e8fc2ef015..c6e93e4757 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -228,4 +228,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