From patchwork Fri Sep 13 17:17:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13803861 Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.182]) (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 4D0266F2F6 for ; Fri, 13 Sep 2024 17:16:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.182 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247797; cv=none; b=Imtkp86bkcZqlhpEQlttQsp7R7aepCeDbXFaBBUWfbjWoPD1enh4TcVSwSq+usrSHaa/zG4ss6NfOX+0PIgdUJtNdIsIt24hriOG7C/Of6VgfQKF73z4PIymrXoZInwIe7KliYtt0ksCbtMKPK5He7fd2AgXg2p2cQJdcWkC5ms= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247797; 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=NV3jF7wAyhB5WVcMjMx1Ccvf3s1bv+NZpDCa7ubW+9SbgQwoOdB2NvwQtXp1w3G60RU8IIjIO/LpIsqZ6dusaVi/zUlZQ0FH6B7TePtChnxxMw6pg6gUew17frezgFTLzUdniDsvAFSc7zOr+QW51Rf6sIZoTudY/V6Hs4Sw++k= 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=HtoQp6Ri; arc=none smtp.client-ip=209.85.210.182 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="HtoQp6Ri" Received: by mail-pf1-f182.google.com with SMTP id d2e1a72fcca58-7193010d386so1079564b3a.1 for ; Fri, 13 Sep 2024 10:16:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1726247795; x=1726852595; 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=HtoQp6Ri/W0avdTb0o7ZT1ZeCRexQzQ72Eycwo6iQw9SFN9ozFiLT6xXtO0RFq3K35 Nb6tf2BWq6yrH2z3FHjZjjVA9ho6rFc998X8P+YCZeEJ7WHh1F+W87vsYrTT3UNW7Z2r txg9bBjAU7+Fpap7wprxlnpC8XNOWA0tDmQjNF6wq0zPYLhobX8rbQLiik1Y3zAy09lV rk+2SXy2ztgrJoWmOGmOzS+Facwa9F3dU9AUlcT5YpwDV/C8/9Xn1FwI1OOis8zdeLw8 QF8NgojdJK3Im1Y+Se3pDELW2DE13JF0mj8G/znjRzOBu0+FzJZUnMB4/xulxEmNeb2y 1nHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726247795; x=1726852595; 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=uxVEKvC6fR/jJNTLY0ZYe3XiNFbRIzTO5La9rtSIN9DF3tVGaliHoF/jp545KqZHGf aSbrzFNht9ZW2wiREixo6CLYHNHAJkgNkAFdLwRfEnz3DJocVQHqg3hnRUakm1lP7Rmx pFg1wxUD/uf2WqaDw+HGlrUqklRhKm0kII2EonbxBujzy7/hG1tHPl0TNvYexvkCXiDF faYDHBPgyJSPajZY6E76xg7nwov9mFn0K/cRYV2oW6Id5ugQ8kw1sI2Q9Vab42JUeDbY RAkANlPmTFBNGtNNPgorxr14eR0ao32SDsymvbNfssANoq8HsmHAmZez0ZVfNpwXe/i4 KHkA== X-Gm-Message-State: AOJu0Yyt9pJAC4Gs8eNbh/iM8+OfMxStDFzvnBZmnyiYr3ZiZE82CD6D DKjwPjsf5b5UTCeMNuuJDjoT8ePuQdutyDjxvTGd4U3o2NQMl7t0/Ntr0w== X-Google-Smtp-Source: AGHT+IE8zwkJUOx8+15umuY6pPgNSzQj191O7VjO+aZ7Hbngs8/fPgOdDduMErXZXhb6eve/c8kJmg== X-Received: by 2002:a05:6a00:9188:b0:717:8e49:37ce with SMTP id d2e1a72fcca58-71936afad89mr5437379b3a.21.1726247794955; Fri, 13 Sep 2024 10:16:34 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71908fc7efdsm6351406b3a.7.2024.09.13.10.16.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Sep 2024 10:16:34 -0700 (PDT) Date: Sat, 14 Sep 2024 01:17:42 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v4 1/5] 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 Fri Sep 13 17:17:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13803862 Received: from mail-pf1-f180.google.com (mail-pf1-f180.google.com [209.85.210.180]) (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 9F3CF1EEE0 for ; Fri, 13 Sep 2024 17:16:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.180 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247807; cv=none; b=MbdqoworNS78cAHCGe3YA8E+49y1+N2FF4C2n8b5XUjuaFWZtD7UbwHY2vOvnBGsRCCDe+A2k2N0YJIO/FDJWv6I6FsRDQyJa1wZe1vB6SUjRiBx6vRbDcj1Z02lKdYccXPQyqMjaZsEYAJXpmYrUcU5Xw9P/i0s8mZHV6Q5/I8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247807; c=relaxed/simple; bh=FaV57rEiAL7/iJUjn/Rm1+vBriELlI1QOaQv5uZth44=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=M98zpw+N+U+fq0XvDBbW+EGwprwF7hx1nk0+3bUwYlMKn1BEkoo83hY++kmzyxtEMQgDvBB8Y1XB+wPbV3SzFGpNiq+b2o0yYY7UHbIzxhQH/e7UbUyyS25Mrw1gvV6E48CQT9e4uhgXZb3ZSeqZsuev8J/QDm+gwjlxThh+GgE= 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=ffreELEQ; arc=none smtp.client-ip=209.85.210.180 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="ffreELEQ" Received: by mail-pf1-f180.google.com with SMTP id d2e1a72fcca58-718e11e4186so2223001b3a.2 for ; Fri, 13 Sep 2024 10:16:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1726247804; x=1726852604; 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=ypX13elVG1B9f26um6JWWC8armjF1e0oEu2SmguDvmQ=; b=ffreELEQPppy8E1nL26GmH52EEtuDm2XM6GYvUTXqi5w6CSy60YlRt+KX4VLyOllFX 2zG5+X9T+CxWeqCBA6DIY4Ss6bK+nEJ/HTbJtLrrejlp3GZywlPD8+OYv5FwOEj11zSm sQVMw5t0H0oX38X+Lco2sstQL4y4uWCCknLg3NiRjKClMwAoA+N9tUYX4s0f2ZJZnna0 Lnr9rdK6JKh6aT5TB0Q6LEzcIihtKb4MRRAHUFgMysjvuzBulKkNOSKRD3Z0sIxNuNGh BLyf+bAYJTZ4I5c4iQnsZU89E09ttYvbB1mqM82PH+HGsZbqsr3AUAjgl8GjAgHKUY3U mCLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726247804; x=1726852604; 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=ypX13elVG1B9f26um6JWWC8armjF1e0oEu2SmguDvmQ=; b=lKuiMHe1gUAsffvu8b54u++gFbo0geolHoXrv8XmD2U+o8Wbfm9YcYG3NXEM0pGrfj zqm7p8kcrTyx6+g5XXIY+4Y4ZbrMbX/8XwilTv3ZxOyCEYlKZHMbpvRJJCrK2zZPyYtY GJNjIyu3zP3p5M8CLEBsK+KkUSh7PSaNc/vzkuq0zlC23UIJPu95BQfqtF1a/hseYedQ qhJeuuaFyMaZYhcQZ9cnmHp4LxEvSMFA2+4sxj+28qmSIKywndCJIz8rsn0RQA+b61+q tffVI+zdboP3JB8ridRivUPHQrwjlFOQxXeaQzGmzUZBdzf/av+aL8PAbc8aOkX1ilRb 0Bdw== X-Gm-Message-State: AOJu0Yzox+XOvrf4/dXDc8FXt7E/dQ19+f+I+U+fGQKodAL6oHjFWt1w MBTQcbAh88joLpzbDrGll+MxraQ2ij8Zdwldgf4OAIgufy/GlJzmDWf2rw== X-Google-Smtp-Source: AGHT+IFncdEYCpXAY7oWHuWr+3TooUOcv5fs3tQfNmHY4Mxq2/ianUesneOkk/z1/iGqX3Iy55T7VQ== X-Received: by 2002:a05:6a00:3d12:b0:714:1f6d:11e5 with SMTP id d2e1a72fcca58-719260815d3mr11286747b3a.12.1726247804113; Fri, 13 Sep 2024 10:16:44 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-719090c9779sm6330099b3a.190.2024.09.13.10.16.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Sep 2024 10:16:43 -0700 (PDT) Date: Sat, 14 Sep 2024 01:17:52 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v4 2/5] ref: port git-fsck(1) regular refs 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, we have already set up the infrastructure of the ref consistency checks. We need to port original checks from "git-fsck(1)". Thus, we could clean the "git-fsck(1)" code by removing these implicit checks. The "git-fsck(1)" command reports an error when the ref content is invalid. Following this, add a similar check to "git refs verify". Add a new fsck error message called "badRefContent(ERROR)" to represent that a ref has an invalid content. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 ++ fsck.h | 1 + refs/files-backend.c | 43 +++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 60 +++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 68a2801f15..22c385ea22 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 bad content. + `badRefFiletype`:: (ERROR) A ref has a bad file type. diff --git a/fsck.h b/fsck.h index 500b4c04d2..0d99a87911 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) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index 890d0324e1..b1ed2e5c04 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3430,6 +3430,48 @@ 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}; + 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(_("unable to read ref '%s/%s'"), + refs_check_dir, iter->relative_path); + goto cleanup; + } + + if (parse_loose_ref_contents(ref_store->repo->hash_algo, + ref_content.buf, &oid, &referent, + &type, &failure_errno)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "invalid ref content"); + 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 +3554,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/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 71a4d1a5ae..a1205b3a3b 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -89,4 +89,64 @@ 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 "%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 "%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 + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + test_done From patchwork Fri Sep 13 17:17:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13803863 Received: from mail-il1-f172.google.com (mail-il1-f172.google.com [209.85.166.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 8D8891EEE0 for ; Fri, 13 Sep 2024 17:16:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247815; cv=none; b=Kr9tfFw+XfyUagwflEgUJORKRq/DTofHVWvv6PxnGtmzEdygyBopqgU047k3c20Imk2lEW4Ov2KdL6X/My1ZEhba9sm/uGBa0dt42WDoltSNX4j4AmCzbVjXJGLORGnqwLPReH8IRs951oSBi1mgLZs06meJ8tRYoW6UdHn2bIU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247815; c=relaxed/simple; bh=E9jY84sbocNDJchM+TlVsU9d3CaNQafrcMnFzAvnN6Q=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JKFIzwVoMC0GbY+frFwjKqKSo6IjQ5qw7l3FSFZlMhIaga1leni2fObYbMDLo91hj+BgcqGPq2MqByjkQ+ojS4+diT9Evr5RX5tf7FfgKh7yePNccwoagfWfYfn/eAV2l1o4xnV7M/SKMPg/wl/1iKchAzCa6tZq7XdxZR9jhMk= 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=RQy16weX; arc=none smtp.client-ip=209.85.166.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="RQy16weX" Received: by mail-il1-f172.google.com with SMTP id e9e14a558f8ab-39d3872e542so8569615ab.2 for ; Fri, 13 Sep 2024 10:16:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1726247812; x=1726852612; 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=qeNtttAKEFqx3YaDUdcBobQcT7FZtYK6d0vlfRb1qQ8=; b=RQy16weX05Lq+NVEkZYsYLV0BpSC8hmm+dcbyoGd6zM6VU30MOgiUc1osmxlcabzyt nl1iQWT/TidgOgCGg1Eaz5WbrguceG22XXCptQ6N35iPBNexjkLX5LbaeiGGsQ99UCjU g6Zy3z1lZM0KAGKj9BJ+A/9WoF/C0sFN11Hh/UzvwBjXC+i+BjG9DwmBdbQji0Ph308F vmoWWhJzuHWfYo92zkPRrbKz4/KBt319RhGpyRJQc1zepzddPy0+dC2rr4+gSa9TEzoM 2zPdORZax1bQcI4T8EEQH9NImm/KtljsnCTi4etvJ0Pty4rd57JqAnfEjICejORsFcBd dFAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726247812; x=1726852612; 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=qeNtttAKEFqx3YaDUdcBobQcT7FZtYK6d0vlfRb1qQ8=; b=RuW7ofMYJqh5jjfdg0NMHjD+gpoCHTVVrJnao7u3mTX1JmWeveW0NWfIyFIJMlTalv q5HEkk+uYXwgIr4UIs/GpzpTGaQqPAFCubPSFqasDJS1ljfDKXeWydnaZR32uoYyMArS 2VrMG4Qca9aSL/qs+KlK+u3Med0nm/7ZSIRdJyFh/SPD3psFJ4alrrIJZop1cGaAiP76 t6rqwB32TQoSDx/u9/KfPafuj5qcfxV5V9t79k1G0LN1AoOf0VcDE6xjKAIrduPj+nKX kRJ0aEyVe0Q9dJvxUf++f3/H216yObqrEQliDQXrPQfvuKTmEIx9SmtwKj5q+XNoHfyw vUaw== X-Gm-Message-State: AOJu0Yy/4lzBNnAGNznl/MI5cLKS+EsC/X35pe3m5RbhXsndJ4+flHOb r2gguEKyiMsIR/B/UTC4IrwIuGoHpWCjKlExlFBLv02dfMGPA1Sr01vnCA== X-Google-Smtp-Source: AGHT+IGL9PSdDLKK6cFTzKmgYi5B2Ex67UbZASSbr14zbjWJV22djRxASiWSaPMrqXC/V3St9GUjwQ== X-Received: by 2002:a05:6e02:2168:b0:39d:4ef6:b36d with SMTP id e9e14a558f8ab-3a0848c9f58mr74556535ab.7.1726247811741; Fri, 13 Sep 2024 10:16:51 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7db1fbc76cesm3665487a12.54.2024.09.13.10.16.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Sep 2024 10:16:51 -0700 (PDT) Date: Sat, 14 Sep 2024 01:17:59 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v4 3/5] ref: add more strict checks for regular refs 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 used "parse_loose_ref_contents" function to check whether the ref content is valid in files backend. However, by using "parse_loose_ref_contents", we allow the ref's content to end with garbage or without a newline. 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. We should notify the users about such "curiously formatted" loose refs so that adequate care is taken before we decide to tighten the rules in the future. And it's not suitable either to report a warn fsck message to the user. We don't yet want the "--strict" flag that controls this bit to end up generating errors for such weirdly-formatted reference contents, as we first want to assess whether this retroactive tightening will cause issues for any tools out there. It may cause compatibility issues which may break the repository. So we add the following two fsck infos to represent the situation where the ref content ends without newline or has trailing garbages: 1. refMissingNewline(INFO): A ref does not end with newline. This will be considered an error in the future. 2. trailingRefContent(INFO): A ref has trailing content. This will be considered an error in the future. It might appear that we can't provide the user with any warnings by using FSCK_INFO. 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 issues. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 8 +++++ fsck.h | 2 ++ refs.c | 2 +- refs/files-backend.c | 27 ++++++++++++++-- refs/refs-internal.h | 2 +- t/t0602-reffiles-fsck.sh | 60 +++++++++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 5 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 22c385ea22..8827137ef0 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -173,6 +173,14 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`refMissingNewline`:: + (INFO) A ref does not end with newline. This will be + considered an error in the future. + +`trailingRefContent`:: + (INFO) A ref has trailing content. This will be + considered an error in the future. + `treeNotSorted`:: (ERROR) A tree is not properly sorted. diff --git a/fsck.h b/fsck.h index 0d99a87911..b85072df57 100644 --- a/fsck.h +++ b/fsck.h @@ -85,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 b1ed2e5c04..df4ce270ae 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; } @@ -3439,6 +3443,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, 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; @@ -3458,13 +3463,29 @@ static int files_fsck_refs_content(struct ref_store *ref_store, if (parse_loose_ref_contents(ref_store->repo->hash_algo, ref_content.buf, &oid, &referent, - &type, &failure_errno)) { + &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) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + goto cleanup; + } + + if (*trailing != '\n' || *(trailing + 1)) { + 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); 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 a1205b3a3b..a06ad044f2 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -101,6 +101,54 @@ test_expect_success 'regular ref content should be checked (individual)' ' 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 && @@ -135,6 +183,12 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' 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 && @@ -144,6 +198,12 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' 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 From patchwork Fri Sep 13 17:18:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13803864 Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) (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 E58211EEE0 for ; Fri, 13 Sep 2024 17:17:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.170 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247822; cv=none; b=H31Bjq9bsz1Dn/68t5vu4V8Sq05EvscJG7HkTIYhJJrKZkKpnDX9/df7kbE6chDxeA23q8edI+I8+3OkUiCscLezJAbqwjth7yzsOqizE1OebYr/ptSX9RUnMRk/uGmbnGBfHfMl93x0zSokfOZRJFWsFgLb5xLEgunanU7pVMc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247822; c=relaxed/simple; bh=OhGORKm+WzOH6a6N5pnzZN3RTC+86gZsP+KutYOPL1A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=sJUH68PsY3S+rWgSXLLRCH/YcBcYElYD+5XGay9sX8HH/Jymy3C1ShYl5iG3tui99T9Gh/nonI31qm+SErDhmcPqhSowthI99RESN4MfANrE6H5LJSw7pFVTxZgK/wTNUS1eoWiOSkzHT+63VaUpRih8CgR2pt2efRYO4Pis6Sk= 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=D38CZDup; arc=none smtp.client-ip=209.85.210.170 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="D38CZDup" Received: by mail-pf1-f170.google.com with SMTP id d2e1a72fcca58-718e2855479so1808326b3a.1 for ; Fri, 13 Sep 2024 10:17:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1726247820; x=1726852620; 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=cAM1klxnaoWNDeAZoiNYXJICZCovJJwhKXZYT8wqVl8=; b=D38CZDupfA+/Ua8zIfgJQlCfIExc8fuDrz5WFNyOfm3AbtS9+rmxsK8QiJKtZmJysZ RETEBW0IGe3JJCmXBns5Px1EWKF/SAIdhFwoddK8rZc802StEuw6YYhHXzJkR9hxaPi5 /jL/nj2SMAS8xchFQ4UNaTy2gtxM4IQGJDhpXTaAu8sXdGTA4MGyGko76oqSxJXdZwrc qRObG3jtVtxMinoneIN1OR4L5Z1gpLoxHFpYzR5RkwGyikK76k5kIgkvmVZm2zxS9YA0 aogNbLzTTs3cK1ERpRPfdem1kWqMLfva6spjCACUf5ctYZli2scvCNRl6INycu+SDX1W qO2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726247820; x=1726852620; 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=cAM1klxnaoWNDeAZoiNYXJICZCovJJwhKXZYT8wqVl8=; b=Pq/1Wh5xcSUGiuRsy+MtLYzGwiA2jZMOH7bhJEge62O2QpDTp6PR9ef1L45SYmnOyh FEQYJ/0wxwds+iMJA4dQrmgHW3h0e7SYNcdeY3krgbFPjZ+JZ+smynf+K0yo5qZYT8dh rFrN+9uT3rXP5Rv8OFT7FwU+67UDiGqUFHGMgZEp2VnzY7phfSioIVqbeelG7tL6W/EE 36lJF2C/2U/SLHi72aip9qXpzJgWfGQoe6ULwXGQYovEPadT5ahz+LMmnCcD4X/3S4li IEbA4qP2op9viYHBmV7T2i9YzOeJMDNhN8+gH7ubuncMPmL3jesIL1ZZUOpAGkptbkao rEPg== X-Gm-Message-State: AOJu0Ywq6ZLXAh9hxZgCr6lNdzgp37CCtQc2balaLs5YR+jvM7HB8HSw kLS44sDvWPuU4t0WpMi15d7CHaoV00KRp8kKZbQPzSFoR9ARHmVoupNtVw== X-Google-Smtp-Source: AGHT+IHdmTNOwjllOK4LpRD/2MhbVEFFkg+eeNfknWJ3ByZZ6UCQK9sedooOgxJjG1YEkvKqx6tgWg== X-Received: by 2002:a05:6a20:e30b:b0:1cf:3f2a:d1dd with SMTP id adf61e73a8af0-1cf75ead7b4mr11244710637.12.1726247819571; Fri, 13 Sep 2024 10:16:59 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71908fc8fafsm6321447b3a.34.2024.09.13.10.16.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Sep 2024 10:16:59 -0700 (PDT) Date: Sat, 14 Sep 2024 01:18:07 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v4 4/5] 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 the symref itself. 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" Thus, we could reuse "refMissingNewline" and "trailingRefContent" FSCK_INFOs to do the same retroactive tightening as we introduce for regular references. 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 " And we introduce a new "badReferentName(ERROR)" fsck message to report above errors to the user. 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, if not, we will report "escapeReferent" fsck error message to notify the user this situation. Then, 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 check 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 a directory. We cannot distinguish whether a given reference like "refs/heads/a" is a file or a directory 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 | 9 +++ fsck.h | 3 + refs/files-backend.c | 81 +++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 117 ++++++++++++++++++++++++++++++++++ 4 files changed, 210 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 8827137ef0..03bcb77972 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -28,6 +28,12 @@ `badRefName`:: (ERROR) A ref has an invalid format. +`badReferentFiletype`:: + (ERROR) The referent of a symref has a bad file type. + +`badReferentName`:: + (ERROR) The referent name of a symref is invalid. + `badTagName`:: (INFO) A tag has an invalid format. @@ -49,6 +55,9 @@ `emptyName`:: (WARN) A path contains an empty name. +`escapeReferent`:: + (ERROR) The referent of a symref is outside the "ref" directory. + `extraHeaderEntry`:: (IGNORE) Extra headers found after `tagger`. diff --git a/fsck.h b/fsck.h index b85072df57..c90561c6db 100644 --- a/fsck.h +++ b/fsck.h @@ -34,11 +34,14 @@ enum fsck_msg_type { FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ + FUNC(BAD_REFERENT_FILETYPE, ERROR) \ + FUNC(BAD_REFERENT_NAME, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ FUNC(BAD_TREE, ERROR) \ FUNC(BAD_TREE_SHA1, ERROR) \ FUNC(BAD_TYPE, ERROR) \ FUNC(DUPLICATE_ENTRIES, ERROR) \ + FUNC(ESCAPE_REFERENT, ERROR) \ FUNC(MISSING_AUTHOR, ERROR) \ FUNC(MISSING_COMMITTER, ERROR) \ FUNC(MISSING_EMAIL, ERROR) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index df4ce270ae..0cb4a2da73 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; + struct stat st; + int ret = 0; + + if (!starts_with(referent->buf, "refs/")) { + ret = fsck_report_ref(o, report, + FSCK_MSG_ESCAPE_REFERENT, + "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_REFERENT_NAME, + "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"); + } + + /* + * Dangling symrefs are common and so we don't report them. + */ + if (lstat(referent_path->buf, &st)) { + if (errno != ENOENT) { + ret = error_errno(_("unable to stat '%s'"), + referent_path->buf); + } + goto out; + } + + /* + * We cannot distinguish whether "refs/heads/a" is a directory or not 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_REFERENT_FILETYPE, + "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..9580c340ab 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: badReferentName: 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: escapeReferent: 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: badReferentFiletype: 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: badReferentName: points to refname with invalid format + error: refs/heads/branch-bad-2: escapeReferent: points to ref outside the refs directory + error: refs/heads/branch-bad-3: badReferentFiletype: 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 Fri Sep 13 17:18:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13803865 Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.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 3C27763C for ; Fri, 13 Sep 2024 17:17:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247831; cv=none; b=KlIwprRtdL7xWoTKP2Tez5g2XUuY/+IWqwhVxkjUP55Mqss/EuvyMeFNzlIN02Jh83y/ixLle7dvsEyQxKcL5B9HtEx5Q/Lv7ytwMoydySD7ZL4UEhELQ5FNTzxkfKERySvI61ognwRYqrN36/so0CNbpxOwg+W6lEqt0aeYh3k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726247831; c=relaxed/simple; bh=/9y+/rQE2fkN7EybrSPCt9HreU+OTj2hf+wByK64fj8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ez/qy5fg960Ymwh8gtYmca/O4idiGtnaJ6eT5Wzdef7WiK6NEONYjDGH7WkSAP5om6/TaNvjFy3LeAERtIeFCaJKaU7xwAU18r2+tWWHXqc4Pig8JZsLnB3iO5mwq1dkBTpwPTSv1TvIAdEs6NyS06xy/XElWvp/1+T1lZGUYPw= 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=B6MAmW0N; arc=none smtp.client-ip=209.85.215.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="B6MAmW0N" Received: by mail-pg1-f172.google.com with SMTP id 41be03b00d2f7-7db1f13b14aso2188694a12.1 for ; Fri, 13 Sep 2024 10:17:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1726247828; x=1726852628; 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=gcBP9aqsZjnsFRGAKHy5MquP9gDDmmz9/J03RZMlBg8=; b=B6MAmW0Na8V+qGtaCSvfKD1Wzwd2fBkXVVMOaDpCGd0WdCQD5SJYFWASO8aKBfWEBK uVBg1++8tX8rqJnTttWuNtcrYlknfr8uFt4/6mEaAmo4L7EETGzmTtg8Hp8QXw/lvLVT 4PesW2msYwZD0mNhpo2RA7Uh7X8L6LlhtQi3uAgiFyF0RCVKsewR0wHfEMHRPKpWY2v3 vSvW1YRGiSGVrUNbyJPlq2/A5g+DzEmc/LAKcJelzmL+Z7YVBAE+NBuENOSiITB7xCD5 MRQbf0Z2cP5WdUn8Jsd4PsgEVPJUDv1ciceQnoH9NofzIzWNC2pzYd6Q8rR/i37yHETo JIxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726247828; x=1726852628; 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=gcBP9aqsZjnsFRGAKHy5MquP9gDDmmz9/J03RZMlBg8=; b=EMF+xbt8/hc2Ity9dUgJNEFlzmsFqRASrTI5iZFUJL/heBANtmnT8nSg9wLOpGyRLR tYMwXyckllr0PYWI/9KWldkq9tw0myOhGmtwki3YeBFsIzs/pEp4jQA3bDEZWuT+ho8Z Zi9Wc/otH5Ogns8dDFk0a4HjHSecE1eCAL9Qpr2pPKSFVX34cLjiRwkt/LejnRVa6psG bTybTJgGKzXfciyxrHDUyGDCYV0tnlUXj27vjyXbRmLJvIzY/B6f+hRjiqoTVby6Id4/ LVUurWxhCJMvsNmsB4njFpvEAAm+mPKh3ADSUmF4nlBDs5USCri1Scpi8ApbAvaqbsXK Fbyg== X-Gm-Message-State: AOJu0YyDwKcuRpjaYSINewQ44o571jwgb2K3SwgiU1xuzZQicRIOeL7S +xINFgCSpif+NwkRT4hnf5q6SwYMO8wA/3VM5h0dQIyPDzmgUbrGgz/o0g== X-Google-Smtp-Source: AGHT+IENDj18kS5Z75zceovdOXbnsthn9J1Or1kqTPsXv/4mkn6EnELQwKwK5tl/r3RjeuSM/JCGhw== X-Received: by 2002:a05:6a21:58d:b0:1cf:29a8:8e1c with SMTP id adf61e73a8af0-1cf75f6544fmr11102860637.28.1726247827987; Fri, 13 Sep 2024 10:17:07 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71909092264sm6332555b3a.116.2024.09.13.10.17.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Sep 2024 10:17:07 -0700 (PDT) Date: Sat, 14 Sep 2024 01:18:15 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v4 5/5] 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 consider deprecating writing the symbolic links and for reading, we may or may not deprecate. We first need to asses whether symbolic links may still be used. So, add a new fsck message "symlinkRef(INFO)" 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 | 65 ++++++++++++++++++----- t/t0602-reffiles-fsck.sh | 97 +++++++++++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 14 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 03bcb77972..31626e765b 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -186,6 +186,11 @@ (INFO) A ref does not end with newline. This will be considered an 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 content. This will be considered an error in the future. diff --git a/fsck.h b/fsck.h index c90561c6db..b72ee632a4 100644 --- a/fsck.h +++ b/fsck.h @@ -89,6 +89,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 0cb4a2da73..c511deb509 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; struct stat st; @@ -3454,14 +3459,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_REFERENT_NAME, @@ -3469,7 +3476,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,6 +3516,7 @@ 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}; @@ -3521,8 +3529,35 @@ 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; + + 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_ESCAPE_REFERENT, + "point to target outside gitdir"); + goto cleanup; + } + + strbuf_addstr(&referent, relative_referent_path); + ret = files_fsck_symref_target(o, &report, + &referent, &referent_path, 1); + goto cleanup; + } if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { ret = error_errno(_("unable to read ref '%s/%s'"), @@ -3563,7 +3598,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, + 0); } cleanup: @@ -3571,6 +3607,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 9580c340ab..7c3579705f 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: escapeReferent: 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: escapeReferent: 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: badReferentName: 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: badReferentName: 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: badReferentFiletype: 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: escapeReferent: point to target outside gitdir + error: refs/heads/branch-symbolic-2: escapeReferent: points to ref outside the refs directory + error: refs/heads/branch-symbolic-3: badReferentName: points to refname with invalid format + error: refs/tags/tag-symbolic-1: badReferentName: points to refname with invalid format + error: refs/tags/tag-symbolic-2: badReferentFiletype: 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