From patchwork Tue Sep 3 12:20:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13788610 Received: from mail-pg1-f171.google.com (mail-pg1-f171.google.com [209.85.215.171]) (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 15D751C9877 for ; Tue, 3 Sep 2024 12:19:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.171 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725365983; cv=none; b=JDY10qYdbhG9BZ7QEOf+MjrPt58VJAZHaHaxLjE/P9s3TBi3jGCCanfH/L3cKKWDe9vnsXMwVuM+tuezMa/XjslOophe2dBWSMcJ2kRIX/n34qQ18h6TbXC5n0hw9EJRJpKwrXa7BrvPyldQnnEMM1xKG22+A4e+wALE0602nUA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725365983; c=relaxed/simple; bh=Ei76NNn9ZuIt6/c5YZrfkWpABLETbb7A2ZN7vYE2UAw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kDzPimVMecp3bJV1MmRgPBlTcxxXR9FNvLn1B+ffeu8dTX5E0XefNN9gSgCkUbiPrl/YQ83UzELO2yKZ+gOQwJuM/kGi9ZoLc0Lfyp4eHC7qS0xuyLtM/kYCaasYDbDzkT1m06ASTeKvnxU2K6rymXxBGT58tP7GPzkXUibH7w8= 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=JdpezdN9; arc=none smtp.client-ip=209.85.215.171 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="JdpezdN9" Received: by mail-pg1-f171.google.com with SMTP id 41be03b00d2f7-7d1fa104851so2955226a12.3 for ; Tue, 03 Sep 2024 05:19:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725365980; x=1725970780; 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=/QJReyzWosByjug4OQcBfjDsyWNBRNw0n2w0RI6Hd40=; b=JdpezdN9J1neEGa/yihxxNW1MPHn0NYQ1PrjVGzyx4eXeMHu+rhv5sj2SwE2O5pXOE uFGKhwXvOA0nYbIKfVb+OGGojBZ4DkEBMfgN/IM8Hc/TTXk5Rfdxv3jn4fgGLZBDXf9F Oq75zVYaPx+rVxZkDccBFPACJqx4z0l4aeoYffk1MdnNP04qhL4mkoMhinU3J/VmHxKB hSySJ2qbyUvFv0KwQziA4dq1XX5g2SlT804n4CWVhCzzaW8XhLRW8vzqp6WOnSGXfq60 ewhAS6r6Ri8BHW/P8/bIxW2Tiy4iJHzE+VwG/uonrpPG9CmUkxzwCMp0mOWvs26UbNg3 Nzyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725365980; x=1725970780; 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=/QJReyzWosByjug4OQcBfjDsyWNBRNw0n2w0RI6Hd40=; b=NhPjq1rtiuWjQwcx4HS2mUxwJ6rhsCZdOHiDoh1mQ4CQCuLtcrgyCB2FU1BdxkQrQ7 sJBhO6Qs8HlMW326El0rVt52Ns+9Ufk+7ACKPEVBxlAZCaf7xd3vIovYxdcLcovdARE9 Ki+cquZCxrUt/QLr5D+sdXtIiKbkgBoMy7GGVqDef/WmXvb5X/xqQ1gVepArAt9xXHuc 5xJAQIr1aQWz5pHUXDQLWxex7Y261KoRCd/aF7OW9dWPYWykCw0zb+CJCOy1Nhbw+gUG qSZUAG192oADlWE1NU2XVtdDskeHg69CkaUG0bJOAgiJWmRwRQWlC4fAJkJ595pXs+Gs 9ddw== X-Gm-Message-State: AOJu0YyWZnvUGXq2/hizmdk8lDDa5ZgyI2IyseADdTCEdyVPQV8Mzeun HGL5bN1QJYb1+iIWpSQ/tyJZjuzXjTptobVnZfmMd27QERM9GRYVXbvaAQ== X-Google-Smtp-Source: AGHT+IFQgemMoC7EHxY0il7krLQCZJ8MCv2m/LGNo4+5ZLfm+zY7uFFjhlzKPWHbTahApu0fQL1vFQ== X-Received: by 2002:a05:6a21:e90:b0:1ca:c673:9792 with SMTP id adf61e73a8af0-1cecf49af5amr12398761637.10.1725365979631; Tue, 03 Sep 2024 05:19:39 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-715e55a6ff1sm8389970b3a.86.2024.09.03.05.19.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Sep 2024 05:19:39 -0700 (PDT) Date: Tue, 3 Sep 2024 20:20:37 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v3 1/4] ref: initialize "fsck_ref_report" with zero 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. The original code explicitly initializes the "path" member in the "struct fsck_ref_report" to NULL (which implicitly 0-initializes other members in the struct). It is more customary to use " {0} " to express that we are 0-initializing everything. In order to be align with the the codebase, initialize "fsck_ref_report" with zero. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 8d6ec9458d..890d0324e1 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 = { 0 }; strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); report.path = sb.buf; From patchwork Tue Sep 3 12:20:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13788611 Received: from mail-oa1-f49.google.com (mail-oa1-f49.google.com [209.85.160.49]) (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 099091C9856 for ; Tue, 3 Sep 2024 12:19:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.49 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725365991; cv=none; b=shteQPzPkr8gtmSohaqfSLPjzFX4Xqw/TgZ5s/Oz50UVHS18MTCf2wtdxe03uBX37Bul+LPMUaG5Yn7znZHVZs64DRdRvKY7i4M4eNsVr9jaHlqns9/N+QGNsAL4J8VLiYciDSkHT3R0yhGiDdiLzMJsXBmezL3Qwe4WRog1MpM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725365991; c=relaxed/simple; bh=mdw9p/I/ToQsj4fmR8irvgLJjJcvftghaTSMUEXLTQQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Cc2Y+I7m86vWBP1KD/674EaLaj5jG5zazFd32039dxQObEM32x83uT9UIVSWd7Xq+PaIIHSyKTsjNEm1d9+qNg2/pzMoJNCvcsiliiFVEjXAIcQTYlRQpiw+cTCnhMzqReJiimlbOXvLQ3lnfysC2WeUY6lpVhLi4rYReCCWrpM= 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=WNkWTpk6; arc=none smtp.client-ip=209.85.160.49 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="WNkWTpk6" Received: by mail-oa1-f49.google.com with SMTP id 586e51a60fabf-268eec6c7c1so2617574fac.3 for ; Tue, 03 Sep 2024 05:19:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725365988; x=1725970788; 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=QN1FKJ0XYDPZdwIAhDnu5iaqp/bSjb842VzbUVu5FcI=; b=WNkWTpk6QwVpToLWxAk0BTYb709FBrmeelQaSiB3aozknxxdEGhZbJmOGeIH93YQIm 6uHPPXJTIK+02IgfV/cfGFDGiYkrbBNAopmOzlAYam+6dROPeQ5SiX2p3rsPkqiCHEYz BHcTx3nyTUSNv5tTaJr4hYcVifS67aa0teTK3LXt1+4OzfqnIBmhd6AIJ18Q0c0iqhg9 hOUg2NOGvQ2Bk9VmBYTEZWZbPvn+ttrXkKxwHpCesK01kRvpCI9nT6IN4gRjeIpzKuqb 2iKwW+tAaMDirVw3HQJ7NoPwNg97D9ZHss4y0jj/tx6ypCiV7cIFjDZ5kxz41U4vVI92 88MQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725365988; x=1725970788; 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=QN1FKJ0XYDPZdwIAhDnu5iaqp/bSjb842VzbUVu5FcI=; b=B28kDK6fkxMsRaPAQZeEBxxvHTpuUBpEooTitE3gdqA2GoE/sF5habnOS0ECt2aEEO u5s0sTdgbdyh0XtGCN7gcFHcbPp4voi7vSbzosMwAWcKuhWe88M0Yg/bCbchn7rTD4KJ n95yZlpGszo/nXJWsQEkwOcXFDHdxw/A9eyIzNbXCzCs0hMxbI3UxJKkw6i0/vq1rnZ0 TMXrhcM/m27+7BkwTTXfeREyHKa/8eSz2fvIC4OAPU7jc88HYxCsBtvTNHjfpYieetjR Bn5BQAFl6FXavAUuEK/ODwg7RYKbaKV+ittcmFq8wkUZhmhVKEHwXfqESKdflSBhd5GJ YEEw== X-Gm-Message-State: AOJu0YxLIdHAWN/f6vlgIuke5sIkTkvQvDsWDQ9JhmWv6SIrGY5mJsb+ hSvb5+xJid90ZLQWOjdAZoWYCjRWYejyxL4NhGPSrc8H0cr2P1AyGjSSLg== X-Google-Smtp-Source: AGHT+IHvcy0/fH3CLWjcBIQadFExMgOgcuGilcJDfjz7e6+rgNlrSJDdvnNOQyW21Na6vJkwb05M0A== X-Received: by 2002:a05:6871:8ab:b0:270:1f1e:e3ea with SMTP id 586e51a60fabf-2779013e567mr20924663fac.28.1725365988260; Tue, 03 Sep 2024 05:19:48 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-715e55a5956sm8393941b3a.75.2024.09.03.05.19.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Sep 2024 05:19:47 -0700 (PDT) Date: Tue, 3 Sep 2024 20:20:46 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v3 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 rely on "git-fsck(1)" to check the consistency of regular refs. However, when parsing the regular refs for files backend by using "files-backend.c::parse_loose_ref_contents", we allow the ref content to end with no newline or to contain some garbages. Even though we never create such loose refs ourselves, we have accepted such loose refs. So, it is entirely possible that some third-party tools may rely on such loose refs being valid. We should not report an error fsck message at current. But let's notice such a "curiously formatted" loose refs being valid and tell the user our findings, so we can access the possible extent of damage when we tighten the parsing rules in the future. And it's not suitable to either report a warn fsck message to the user. This is because if the caller set the "strict" field in "fsck_options", fsck warns will be automatically upgraded to errors. We should not allow user to specify the "--strict" flag to upgrade the fsck warnings to errors at current. It might cause compatibility issue which may break the legacy repository. So we add the following two fsck infos to represent the situation where the ref content ends without newline or has garbages: 1. "refMissingNewline(INFO)": A ref does not end with newline. This kind of ref may be considered ERROR in the future. 2. "trailingRefContent(INFO)": A ref has trailing contents. This kind of ref may be considered ERROR in the future. It may seem that we could not give the user any warnings by creating fsck infos. However, in "fsck.c::fsck_vreport", we will convert "FSCK_INFO" to "FSCK_WARN" and we can still warn the user about these situations when using "git-refs verify" without introducing compatibility issue. In current "git-fsck(1)", it will report an error when the ref content is bad, so we should following this to report an error to the user when "parse_loose_ref_contents" fails. And we add a new fsck error message called "badRefContent(ERROR)" to represent that a ref has a bad content. 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 to enhance the "git-refs verify". Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 11 ++++ fsck.h | 3 + refs.c | 2 +- refs/files-backend.c | 68 ++++++++++++++++++- refs/refs-internal.h | 2 +- t/t0602-reffiles-fsck.sh | 120 ++++++++++++++++++++++++++++++++++ 6 files changed, 202 insertions(+), 4 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 68a2801f15..06d045ac48 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,14 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`refMissingNewline`:: + (INFO) A ref does not end with newline. This kind of ref may + be considered ERROR in the future. + +`trailingRefContent`:: + (INFO) A ref has trailing contents. This kind of ref may be + considered ERROR in the future. + `treeNotSorted`:: (ERROR) A tree is not properly sorted. diff --git a/fsck.h b/fsck.h index 500b4c04d2..b85072df57 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) \ @@ -84,6 +85,8 @@ enum fsck_msg_type { FUNC(MAILMAP_SYMLINK, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ + FUNC(REF_MISSING_NEWLINE, INFO) \ + FUNC(TRAILING_REF_CONTENT, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) 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 890d0324e1..0187b85c5f 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,65 @@ 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 strbuf ref_content = STRBUF_INIT; + struct strbuf referent = STRBUF_INIT; + struct strbuf refname = STRBUF_INIT; + struct fsck_ref_report report = {0}; + 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_ISLNK(iter->st.st_mode)) + goto cleanup; + + 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 +3575,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..a06ad044f2 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -89,4 +89,124 @@ 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 (individual)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + git refs verify 2>err && + test_must_be_empty err && + + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline: refMissingNewline: missing newline + EOF + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err && + + printf "%s\n\n\n" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-garbage-1: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-garbage-1 && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-2 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-garbage-2: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-garbage-2 && + test_cmp expect err && + + printf "%s garbage\n\na" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-garbage-3: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-garbage-3 && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-4 && + test_must_fail git -c fsck.trailingRefContent=error refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-garbage-4: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-garbage-4 && + test_cmp expect err && + + printf "%sx" "$(git rev-parse main)" >$tag_dir_prefix/tag-bad-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-bad-1: badRefContent: invalid ref content + EOF + rm $tag_dir_prefix/tag-bad-1 && + test_cmp expect err && + + printf "xfsazqfxcadas" >$tag_dir_prefix/tag-bad-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-bad-2: badRefContent: invalid ref content + EOF + rm $tag_dir_prefix/tag-bad-2 && + test_cmp expect err && + + printf "xfsazqfxcadas" >$branch_dir_prefix/a/b/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: invalid ref content + EOF + rm $branch_dir_prefix/a/b/branch-bad && + test_cmp expect err +' + +test_expect_success 'regular ref content should be checked (aggregate)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && + printf "%s\n\n\n" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-1 && + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-2 && + printf "%s garbage\n\na" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-3 && + printf "%s garbage" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-4 && + printf "%sx" "$(git rev-parse main)" >$tag_dir_prefix/tag-bad-1 && + printf "xfsazqfxcadas" >$tag_dir_prefix/tag-bad-2 && + printf "xfsazqfxcadas" >$branch_dir_prefix/a/b/branch-bad && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: invalid ref content + error: refs/tags/tag-bad-1: badRefContent: invalid ref content + error: refs/tags/tag-bad-2: badRefContent: invalid ref content + warning: refs/heads/branch-garbage: trailingRefContent: trailing garbage in ref + warning: refs/heads/branch-no-newline: refMissingNewline: missing newline + warning: refs/tags/tag-garbage-1: trailingRefContent: trailing garbage in ref + warning: refs/tags/tag-garbage-2: trailingRefContent: trailing garbage in ref + warning: refs/tags/tag-garbage-3: trailingRefContent: trailing garbage in ref + warning: refs/tags/tag-garbage-4: trailingRefContent: trailing garbage in ref + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + test_done From patchwork Tue Sep 3 12:20:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13788612 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) (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 C2C381D0959 for ; Tue, 3 Sep 2024 12:19:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725366000; cv=none; b=tEeYIhDP/jO42x1DC91p+LVjiqevhGNh8DmsHc952/sUVI/WepRl1ds2p8EcoGQPmPARfXxPpQXbPobYaNdyKoZmGx1yXRf9aWnpVWkYzWZpU+adWeHVdKP3W2yQSyjJGWcvWeK4So9iHxPS3Ks0LTcfdpbiEJ1YDN7E6IbruE8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725366000; c=relaxed/simple; bh=nTUFVEU1sg54y98hc6zSfI27BEdRfK80ULzDm+q50fQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tdlnxBG7gtRqLO6FX3nmiRNy6IshFAuhBsU/MYWtE5CWaqVHDUQLZsNGiJAKswM8PaIGACWtV+x3hSash0rK+NuAHJu3Xz10O9VzQ+yohaw47lCkJhKSjt55EOwIfxRUiyXfQVVKWsHjyQsvQfWnTUYY0yWUh1EjKBd/ylCE2fk= 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=BBj2NCX/; arc=none smtp.client-ip=209.85.214.173 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="BBj2NCX/" Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-2057917c493so14699155ad.0 for ; Tue, 03 Sep 2024 05:19:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725365997; x=1725970797; 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=FqogiwME54/0KhEoXddl+tZPabeEmKtz8FYWOuboFvk=; b=BBj2NCX/T9XqCbPxZXgvalJ4IfAr0/dmubDBRj5JdXsUlOVTGmz+CuoV1Wy+BAVCNK ttn9KPHEvLUsCAyX1DFItTcavdQAX6KEVY9U3MqIyfUSWvRzjPFCkd0V7/p4a9kWWK4G xj0alGDqVbBQe9MvkhHIDBgATIsY43WNziLqLRX39T0lQ3Zjt/6JbWkXJ+mz3YYJdnDd aQ0a07hP7uTMZeYAFOa0E6adBOR/Xby4EvktqAPiEeXMaL8cAAapoy4mpuXq8r/MGvjc l/LiWY0xd6voy6c9qXPkdW6V05oLJja9TAgWjaztUBD2uodmtyyf42BB9bkJiFyFUwlM +CZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725365997; x=1725970797; 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=FqogiwME54/0KhEoXddl+tZPabeEmKtz8FYWOuboFvk=; b=BRRXqcXbC9nxgus64A7V6EF++bxafXcl/OK9eRMk7rAv/Nx6cRBWKbl9BvA3Veuf6P rPTCpYjrQe1gQGhpLHICZueRJJdjXO8M7UXKE1o+HMFkRwA15jGKowjFwmLururXhp+x kiRdaSHJjufocFzfjL0JQTaTZsGUaFlQ7Xh+ZbsDWz9Z3gQyUERHh+EOXL1UQoEgjJaG nvtGOuGiSE8D/Hh/gwdhVGTeJaPWSSZjVSaQcxf8DtmJlw+ABAyf/R4TaexdV7HxMw5F aDcuCs2Uul1meEclPNjaLwVybN7VQ0GVxcQnp6lWyjZKiav2A9FN/kKWfuzqRzugS5YW u/Og== X-Gm-Message-State: AOJu0YykyuzRSFueZoc2JqFKarKXdInMAwXetKSQLk+hDYctsbiz2dkd vjVZwCCHzNAoK6ix5N3K8VW5Bzw8IU0Ogta5O7jvxrfvgJHFBDB3kJ6BlA== X-Google-Smtp-Source: AGHT+IFrm6QXaaslwfrdUvjS1urYLKgIHiDef73a5picF6IJHaPSLxu+GF6oMMW9BMberPw6bJOnOg== X-Received: by 2002:a17:902:dace:b0:205:8b84:d5e8 with SMTP id d9443c01a7336-2058b84dad4mr92713825ad.18.1725365997330; Tue, 03 Sep 2024 05:19:57 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2051553703asm80043785ad.181.2024.09.03.05.19.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Sep 2024 05:19:56 -0700 (PDT) Date: Tue, 3 Sep 2024 20:20:54 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v3 3/4] ref: add symref 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 symref points to. Instead, we just need to check the content of teh symref itself. In order to check the content of the symref, create a function "files_fsck_symref_target". It will first check whether the "referent" is under the "refs/" directory and then we will check the symref contents. A regular file is accepted as a textual symref if it begins with "ref:", followed by zero or more whitespaces, followed by the full refname, followed only by whitespace characters. We always write a single SP after "ref:" and a single LF after the refname, but third-party reimplementations of Git may have taken advantage of the looser syntax. Put it more specific, we accept the following contents of the symref: 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 other trailing garbage. The followings are bad symref contents which will be reported as fsck error by "git-fsck(1)". 1. "ref: refs/heads/master garbage\n" 2. "ref: refs/heads/master \n\n\n garbage " In order to provide above checks, we will first check whether the symref content misses the newline by peeking the last byte of the "referent" to see whether it is '\n'. And we will remember the untrimmed length of the "referent" and call "strbuf_rtrim()" on "referent". Then, we will call "check_refname_format" to chceck whether the trimmed referent format is valid. If not, we will report to the user that the symref points to referent which has invalid format. If it is valid, we will compare the untrimmed length and trimmed length, if they are not the same, we need to warn the user there is some trailing garbage in the symref content. At last, we need to check whether the referent is the directory. We cannot distinguish whether the "refs/heads/a" is a directory or not by using "check_refname_format". We have already checked bad file type when iterating the "refs/" directory but we ignore the directory. Thus, we need to explicitly add check here. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 4 ++ fsck.h | 1 + refs/files-backend.c | 81 +++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 117 ++++++++++++++++++++++++++++++++++ 4 files changed, 203 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 06d045ac48..beb6c4e49e 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -28,6 +28,10 @@ `badRefName`:: (ERROR) A ref has an invalid format. +`badSymrefTarget`:: + (ERROR) The symref target points outside the ref directory or + the name of the symref target is invalid. + `badTagName`:: (INFO) A tag has an invalid format. diff --git a/fsck.h b/fsck.h index b85072df57..5ea874916d 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_TARGET, 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 0187b85c5f..fef32e607f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3434,11 +3434,80 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refs_check_dir, struct dir_iterator *iter); +/* + * Check the symref "referent" and "referent_path". For textual symref, + * "referent" would be the content after "refs:". + */ +static int files_fsck_symref_target(struct fsck_options *o, + struct fsck_ref_report *report, + struct strbuf *referent, + struct strbuf *referent_path) +{ + size_t len = referent->len - 1; + const char *p = NULL; + struct stat st; + int ret = 0; + + if (!skip_prefix(referent->buf, "refs/", &p)) { + + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_TARGET, + "points to ref outside the refs directory"); + goto out; + } + + if (referent->buf[referent->len - 1] != '\n') { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + len++; + } + + strbuf_rtrim(referent); + if (check_refname_format(referent->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_TARGET, + "points to refname with invalid format"); + goto out; + } + + if (len != referent->len) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing garbage in ref"); + } + + /* + * 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(referent_path->buf, &st)) + goto out; + + /* + * We cannot distinguish whether "refs/heads/a" is directory or nots by + * using "check_refname_format(referent->buf, 0)". Instead, we need to + * check the file type of the target. + */ + if (S_ISDIR(st.st_mode)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_TARGET, + "points to the directory"); + 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 strbuf referent_path = STRBUF_INIT; struct strbuf ref_content = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct strbuf refname = STRBUF_INIT; @@ -3484,12 +3553,24 @@ static int files_fsck_refs_content(struct ref_store *ref_store, "trailing garbage in ref"); goto cleanup; } + } else { + strbuf_addf(&referent_path, "%s/%s", + ref_store->gitdir, referent.buf); + /* + * the referent may contain the spaces and the newline, need to + * trim for path. + */ + strbuf_rtrim(&referent_path); + ret = files_fsck_symref_target(o, &report, + &referent, + &referent_path); } cleanup: strbuf_release(&refname); strbuf_release(&ref_content); strbuf_release(&referent); + strbuf_release(&referent_path); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index a06ad044f2..e0bf8c8c8b 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -209,4 +209,121 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' test_cmp expect sorted_err ' +test_expect_success 'textual symref content should be checked (individual)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err && + + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline-1: refMissingNewline: missing newline + EOF + rm $branch_dir_prefix/branch-no-newline-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-trailing-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-trailing-2 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-trailing-3 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-complicated: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-complicated: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-complicated && + test_cmp expect err && + + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badSymrefTarget: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err && + + printf "ref: reflogs/heads/main\n" >$branch_dir_prefix/branch-bad-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-2: badSymrefTarget: points to ref outside the refs directory + EOF + rm $branch_dir_prefix/branch-bad-2 && + test_cmp expect err && + + printf "ref: refs/heads/a\n" >$branch_dir_prefix/branch-bad-3 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-3: badSymrefTarget: points to the directory + EOF + rm $branch_dir_prefix/branch-bad-3 && + test_cmp expect err +' + +test_expect_success 'textual symref content should be checked (aggregate)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + printf "ref: reflogs/heads/main\n" >$branch_dir_prefix/branch-bad-2 && + printf "ref: refs/heads/a\n" >$branch_dir_prefix/branch-bad-3 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badSymrefTarget: points to refname with invalid format + error: refs/heads/branch-bad-2: badSymrefTarget: points to ref outside the refs directory + error: refs/heads/branch-bad-3: badSymrefTarget: points to the directory + warning: refs/heads/a/b/branch-complicated: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-complicated: trailingRefContent: trailing garbage in ref + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: trailing garbage in ref + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: trailing garbage in ref + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: trailing garbage in ref + warning: refs/heads/branch-no-newline-1: refMissingNewline: missing newline + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + test_done From patchwork Tue Sep 3 12:21:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13788613 Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) (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 4617243AD2 for ; Tue, 3 Sep 2024 12:20:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725366009; cv=none; b=DhDsga1CJQKI/gSzAI4b/ZIy44xtQZE3vSAfavWa5gBNXXsgsw2YF+vD/MbN91xz+txAMXUZ3OAYqZgIiBJpRN8K5DfXb+GeMqi2EdSsdX9338mJur+Fs6iT8SUdBZDmO0KnJgmMAbNg5usKDKb5XpjCBbEBHBfoD9RU7xLlMAc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725366009; c=relaxed/simple; bh=TquNTPss3vt5Zz6jFxMITLmh6Od4ehYcfB/66Ndep+E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=O7T3nm3YlOEZ0CEyBZEiTM2/nOIQ3Q3gU3zeMYaUWqxjXknN7q70XDVc69st5NWy0e6Flv9x9ySm47MDnYa4PLfFfGBqC6ZTSRNRL7qz/BkLnfzaB+ERUnLyGdA6chEp58//khKE4f0+ZW72PnKSI9g00qcBGp4ldGoYfM7dV3k= 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=hjJu6UZq; arc=none smtp.client-ip=209.85.210.172 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="hjJu6UZq" Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-715abede256so3882771b3a.3 for ; Tue, 03 Sep 2024 05:20:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725366006; x=1725970806; 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=1oQUzJU1uezWQSiSKf2V4jt5Q2GJMakZjotgycN8kXY=; b=hjJu6UZqgpb7vLYatxsUljZgzrw99BK/HHGxHrpddsSIMrAvi24yyHhL1N/ztzhN23 InrqqMwfwxIj4d5nI5kTOE5b9wnkUDEPt3Jn8enkk653JD3ijD1h4wx+PuiBy44YMaiI aCfYWdGF3Za22/IzJv6afB9cTy1cEp/CEON9XwDZNwUEN3JMnW8SloQyD3OFsqQdPRu0 wadbbfasaLu6WzZtUcNASyNAIQS/rVXISCN93VggeJhVFQVzI/YmHn1f3ZW7/0DwaqDQ xTZcEaV0QH29ofKXNdXbeEWXikdVTQ6te4kTsHBbcyEZxzLHtuQ+iPGyCXyKQ059Ujwr lEfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725366006; x=1725970806; 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=1oQUzJU1uezWQSiSKf2V4jt5Q2GJMakZjotgycN8kXY=; b=f2pGmFuUsMfhk+jFb7z+C6A7jtUCE9jrcPNiGbCnxQGg1CebnWdzut9rrWFB5AhuCy n0g2A/HXIrZBVaG9YCRpaiuiNVlllXAC/Nhox3A3bYJy8pcOy/4uZd3M54UlPn+/F/AK XqSWtfEEnFbB+zV+WP1igPhsmB1/z44h38ABIi6bfXUBLuDXVE14QYBS8OGVqPIHzyLL L1fY5lYIovtePwqnZ6wviVmJ59sQTWb1SNSnCKwlOjkcBvy/UYIqsL/pMFxlbRn9SvMs PDwFBIx55sStoi90pY0xOHVSuonWkXrbsTGxzr/6fUu+I1VlNDH4jRHqK/RbQENPKvos vYJQ== X-Gm-Message-State: AOJu0YwfWipxRMbkM6SVALosUtN6/+Gzgv0i2gL1Sq+Jq8E9gu9wKwGe HvFNR06GLjlMkT2iM8PkxJWczXBOTuQObNnWtFDEZKM2Rns14BCFyZuqZA== X-Google-Smtp-Source: AGHT+IGdWQuEbCyAnBW/01e08IEzP5F5txTqVjQv8h4QS/PANaZGoM+sh0lYbsllfkHLyZxU+JxG/g== X-Received: by 2002:a05:6a20:c6ca:b0:1c6:cb01:db61 with SMTP id adf61e73a8af0-1ced62a2caamr10026980637.28.1725366005879; Tue, 03 Sep 2024 05:20:05 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2d8818feacesm8139413a91.30.2024.09.03.05.20.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Sep 2024 05:20:05 -0700 (PDT) Date: Tue, 3 Sep 2024 20:21:03 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v3 4/4] ref: add symlink 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 "files_fsck_symref_target". We should reuse this function to handle the symrefs which use legacy symbolic links. We should not check the trailing garbage for symbolic refs. Add a new parameter "symbolic_link" to disable some checks which should only be executed for textual symrefs. We firstly use the "strbuf_add_real_path" to resolve the symlink and get the absolute path "referent_path" which the symlink ref points to. Then we can get the absolute path "abs_gitdir" of the "gitdir". By combining "referent_path" and "abs_gitdir", we can extract the "referent". Thus, we can reuse "files_fsck_symref_target" function to seamlessly check the symlink refs. Because we are going to drop support for "core.prefersymlinkrefs", add a new fsck message "symlinkRef" to let the user be aware of this information. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 5 ++ fsck.h | 1 + refs/files-backend.c | 68 +++++++++++++++++++----- t/t0602-reffiles-fsck.sh | 97 +++++++++++++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 14 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index beb6c4e49e..9e8e1ac7f0 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -181,6 +181,11 @@ (INFO) A ref does not end with newline. This kind of ref may be considered ERROR in the future. +`symlinkRef`:: + (INFO) A symref uses the symbolic link. This kind of symref may + be considered ERROR in the future when totally dropping the + symlink support. + `trailingRefContent`:: (INFO) A ref has trailing contents. This kind of ref may be considered ERROR in the future. diff --git a/fsck.h b/fsck.h index 5ea874916d..1c6f750812 100644 --- a/fsck.h +++ b/fsck.h @@ -87,6 +87,7 @@ enum fsck_msg_type { FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ FUNC(REF_MISSING_NEWLINE, INFO) \ + FUNC(SYMLINK_REF, INFO) \ FUNC(TRAILING_REF_CONTENT, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) diff --git a/refs/files-backend.c b/refs/files-backend.c index fef32e607f..2a1b952f0d 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" @@ -1950,10 +1951,13 @@ static int commit_ref_update(struct files_ref_store *refs, return 0; } +#ifdef NO_SYMLINK_HEAD +#define create_ref_symlink(a, b) (-1) +#else static int create_ref_symlink(struct ref_lock *lock, const char *target) { int ret = -1; -#ifndef NO_SYMLINK_HEAD + char *ref_path = get_locked_file_path(&lock->lk); unlink(ref_path); ret = symlink(target, ref_path); @@ -1961,13 +1965,12 @@ static int create_ref_symlink(struct ref_lock *lock, const char *target) if (ret) fprintf(stderr, "no symlink - falling back to symbolic ref\n"); -#endif return ret; } +#endif -static int create_symref_lock(struct files_ref_store *refs, - struct ref_lock *lock, const char *refname, - const char *target, struct strbuf *err) +static int create_symref_lock(struct ref_lock *lock, const char *target, + struct strbuf *err) { if (!fdopen_lock_file(&lock->lk, "w")) { strbuf_addf(err, "unable to fdopen %s: %s", @@ -2583,8 +2586,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, } if (update->new_target && !(update->flags & REF_LOG_ONLY)) { - if (create_symref_lock(refs, lock, update->refname, - update->new_target, err)) { + if (create_symref_lock(lock, update->new_target, err)) { ret = TRANSACTION_GENERIC_ERROR; goto out; } @@ -3436,12 +3438,15 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, /* * Check the symref "referent" and "referent_path". For textual symref, - * "referent" would be the content after "refs:". + * "referent" would be the content after "refs:". For symlink ref, + * "referent" would be the relative path agaignst "gitdir" which should + * be the same as the textual symref literally. */ static int files_fsck_symref_target(struct fsck_options *o, struct fsck_ref_report *report, struct strbuf *referent, - struct strbuf *referent_path) + struct strbuf *referent_path, + unsigned int symbolic_link) { size_t len = referent->len - 1; const char *p = NULL; @@ -3456,14 +3461,16 @@ static int files_fsck_symref_target(struct fsck_options *o, goto out; } - if (referent->buf[referent->len - 1] != '\n') { + if (!symbolic_link && referent->buf[referent->len - 1] != '\n') { ret = fsck_report_ref(o, report, FSCK_MSG_REF_MISSING_NEWLINE, "missing newline"); len++; } - strbuf_rtrim(referent); + if (!symbolic_link) + strbuf_rtrim(referent); + if (check_refname_format(referent->buf, 0)) { ret = fsck_report_ref(o, report, FSCK_MSG_BAD_SYMREF_TARGET, @@ -3471,7 +3478,7 @@ static int files_fsck_symref_target(struct fsck_options *o, goto out; } - if (len != referent->len) { + if (!symbolic_link && len != referent->len) { ret = fsck_report_ref(o, report, FSCK_MSG_TRAILING_REF_CONTENT, "trailing garbage in ref"); @@ -3509,9 +3516,11 @@ static int files_fsck_refs_content(struct ref_store *ref_store, { struct strbuf referent_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}; + unsigned int symbolic_link = 0; const char *trailing = NULL; unsigned int type = 0; int failure_errno = 0; @@ -3521,8 +3530,37 @@ static int files_fsck_refs_content(struct ref_store *ref_store, strbuf_addf(&refname, "%s/%s", refs_check_dir, iter->relative_path); report.path = refname.buf; - if (S_ISLNK(iter->st.st_mode)) + if (S_ISLNK(iter->st.st_mode)) { + const char* relative_referent_path; + + symbolic_link = 1; + ret = fsck_report_ref(o, &report, + FSCK_MSG_SYMLINK_REF, + "use deprecated symbolic link for symref"); + + 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, '/'); + + strbuf_add_real_path(&referent_path, iter->path.buf); + + if (!skip_prefix(referent_path.buf, + abs_gitdir.buf, + &relative_referent_path)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_SYMREF_TARGET, + "point to target outside gitdir"); + goto cleanup; + } + + strbuf_addstr(&referent, relative_referent_path); + ret = files_fsck_symref_target(o, &report, + &referent, &referent_path, + symbolic_link); + goto cleanup; + } if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { ret = error_errno(_("%s/%s: unable to read the ref"), @@ -3563,7 +3601,8 @@ static int files_fsck_refs_content(struct ref_store *ref_store, strbuf_rtrim(&referent_path); ret = files_fsck_symref_target(o, &report, &referent, - &referent_path); + &referent_path, + symbolic_link); } cleanup: @@ -3571,6 +3610,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, strbuf_release(&ref_content); strbuf_release(&referent); strbuf_release(&referent_path); + strbuf_release(&abs_gitdir); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index e0bf8c8c8b..e735816d5b 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -326,4 +326,101 @@ test_expect_success 'textual symref content should be checked (aggregate)' ' test_cmp expect sorted_err ' +test_expect_success SYMLINKS 'symlink symref content should be checked (individual)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + ln -sf ./main $branch_dir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $branch_dir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../../../branch $branch_dir_prefix/branch-symbolic-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-1: symlinkRef: use deprecated symbolic link for symref + error: refs/heads/branch-symbolic-1: badSymrefTarget: point to target outside gitdir + EOF + rm $branch_dir_prefix/branch-symbolic-1 && + test_cmp expect err && + + ln -sf ../../logs/branch-bad $branch_dir_prefix/branch-symbolic-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-2: symlinkRef: use deprecated symbolic link for symref + error: refs/heads/branch-symbolic-2: badSymrefTarget: points to ref outside the refs directory + EOF + rm $branch_dir_prefix/branch-symbolic-2 && + test_cmp expect err && + + ln -sf ./"branch space" $branch_dir_prefix/branch-symbolic-3 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-3: symlinkRef: use deprecated symbolic link for symref + error: refs/heads/branch-symbolic-3: badSymrefTarget: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-symbolic-3 && + test_cmp expect err && + + ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref + error: refs/tags/tag-symbolic-1: badSymrefTarget: points to refname with invalid format + EOF + rm $tag_dir_prefix/tag-symbolic-1 && + test_cmp expect err && + + ln -sf ./ $tag_dir_prefix/tag-symbolic-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-symbolic-2: symlinkRef: use deprecated symbolic link for symref + error: refs/tags/tag-symbolic-2: badSymrefTarget: points to the directory + EOF + rm $tag_dir_prefix/tag-symbolic-2 && + test_cmp expect err +' + +test_expect_success SYMLINKS 'symlink symref content should be checked (aggregate)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + ln -sf ./main $branch_dir_prefix/branch-symbolic-good && + ln -sf ../../../../branch $branch_dir_prefix/branch-symbolic-1 && + ln -sf ../../logs/branch-bad $branch_dir_prefix/branch-symbolic-2 && + ln -sf ./"branch space" $branch_dir_prefix/branch-symbolic-3 && + ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && + ln -sf ./ $tag_dir_prefix/tag-symbolic-2 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic-1: badSymrefTarget: point to target outside gitdir + error: refs/heads/branch-symbolic-2: badSymrefTarget: points to ref outside the refs directory + error: refs/heads/branch-symbolic-3: badSymrefTarget: points to refname with invalid format + error: refs/tags/tag-symbolic-1: badSymrefTarget: points to refname with invalid format + error: refs/tags/tag-symbolic-2: badSymrefTarget: points to the directory + warning: refs/heads/branch-symbolic-1: symlinkRef: use deprecated symbolic link for symref + warning: refs/heads/branch-symbolic-2: symlinkRef: use deprecated symbolic link for symref + warning: refs/heads/branch-symbolic-3: symlinkRef: use deprecated symbolic link for symref + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref + warning: refs/tags/tag-symbolic-2: symlinkRef: use deprecated symbolic link for symref + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + test_done