From patchwork Wed Nov 20 11:51:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881081 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3D29A1A4E77 for ; Wed, 20 Nov 2024 11:51:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103464; cv=none; b=GaMe2iw8zhvzpMgNeNp8ci2XoVYMQ1zX+e6BjaGhbUZGH5O4fsfoTMzM4ls+rSXvUjfNa5zsdUeksw9I3IlsCuipQcnXbwjvYIM9ljEGb8PQ5ZEuiWGoOWwOrPipBLoZvBjPbgxNH3k8m1vdUaE97HCBcFc3n/BF86SXPkFWiq8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103464; c=relaxed/simple; bh=Iq240slf6F8USCuNlFVvJmIT6fjkny4AKF4dW34UQj8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=r9HqiqEJ0XKZiMC62iRLHIh9qtoPlfx7ubcakhT2wivgPtl/Dm77vufhLqKGdydp3ohrVJjan0YcdzTCizn/zuiqwMQZ8GtUdrPvbckiPGjOr8QWb5z+aaCgmBOXjMbodrQUSpHWzMiDFztgN3emDBDUhhhWo1S6IFLbKCRrxR0= 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=DamvhaKt; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DamvhaKt" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-20cdda5cfb6so49301685ad.3 for ; Wed, 20 Nov 2024 03:51:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103459; x=1732708259; 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=Vc/fmkvpcUY3It7x/Wyto6u14vgd1/H96CWyWXBd6jA=; b=DamvhaKta1lzHkwd0+o0HizNc+0xh0evt++KgRaE8rO9QnKGSvY7B1ogiE0PB6tdwi 4u9tV3sdLpMTMdD6qhZ/3bxdO8OLVMySud8vVBFeM030M/E+BQPNdKx8HsqxRg2u6xER 7BJFOp5UVcw0Ulckltz2XJpqc4a5GAaZy4gyIoYh3PZD2s+WLZ/A+7xwXnPciTLFgEPM gX1Vqb5JQi9vBxDyetY1irc7oOoXXHVB9ShLDRSTEDEAAHWW/s6bt9LE1JPsQcIUU3n1 wiXpEWTrabE6NC5li2cd/FEZFpFX3JKJIWtuGNJpbLQbhtZuvhYcX8r1KFRQG/oKgAjD 4Tig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103459; x=1732708259; 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=Vc/fmkvpcUY3It7x/Wyto6u14vgd1/H96CWyWXBd6jA=; b=nOmmcllDvmQ0fZtPM7qv+Xp5qbxe1MTLjlRrH+6L3Aae+KQmpnjewACAO5v9KAEUuR epGzDeNUPp35zHWiyvWihsyem9ZaCwn4hIyAMHRZSg/usmFZ2QIrSKOLi/RJZ2NexkRZ 8hzZxuIaZ3++Rk/RvqY0EoYClQ2acRUfafgzO+xRY2BixBJMuPD9q//zhq0ImBOfOuv2 cfNO4Ou1DHOmTRB9PA4vjCN9vOTuiEdrcc5Qu2cYogYQZFFCho+Jz4756pwVYzTVlKp3 Ywg5pYlBIV3YcxvCKwxEjMPCs+pYXH9K+UMy2UoiRpuP9Ngc+1ong7Bf6bQBoUdraM/R pRMg== X-Gm-Message-State: AOJu0YzshhvTzf9DVD+SSJrerDQfKF1d8Dck3EW5FTauQrrf7PbHn4df b6GhrDBMNt2icW3gLV9iObPVavcNknIxV7rXaFABgNwNe4vFfWl1oKUWpg== X-Google-Smtp-Source: AGHT+IGl96+Ct34Xuh8LCOUCj2OHCp3AwDonmcetHr0YFIgqTxNYuN00QeyLFdWVsjvUfUMk8vc9rA== X-Received: by 2002:a17:902:e80c:b0:20c:a44b:3221 with SMTP id d9443c01a7336-2126b06938amr24864335ad.15.1732103459510; Wed, 20 Nov 2024 03:50:59 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-211d0f34903sm90350615ad.158.2024.11.20.03.50.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:50:58 -0800 (PST) Date: Wed, 20 Nov 2024 19:51:08 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 1/9] 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 align with 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 0824c0b8a9..03d2503276 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3520,7 +3520,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 Wed Nov 20 11:51:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881082 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.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 054CC19E971 for ; Wed, 20 Nov 2024 11:51:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.171 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103470; cv=none; b=Ei2pbdi3WMyYo3FA3PVY3ZGU2TDIIZ33yjjF4EmncZhoHRcZBiSO12ftAY2tkoXyzDxS3J9d/xfphvqMYTT/KOgJPC7zFKVmnW7ZnOu5ZXCSAaHRLrE0Cz8Qjf3OUldXU7qAkEGv9iLKej9zsppGPinHRfGNEw7V+tjHF6CsCSk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103470; c=relaxed/simple; bh=4iAJA46zpl/v6qmBKcIfVl8MSuXSf8PZ1daMDFkHi9A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=T/CRvoGwTj3t0d6fOHfjeiwIEOOsffizlYRdGly83SCRpIz9TR6Y1u5kVasuAizqVhDtoAcgD2bNHmkc3Sik7zHjpvVNgQFru6PPsmzw8lMws7uGewnr3/7tG/kWJlgMZR2LMZWcnuHKylFMEdZiuUdAu7PJzJMs3AtXlvd+TFQ= 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=eanxGAC3; arc=none smtp.client-ip=209.85.214.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="eanxGAC3" Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-20c805a0753so19912975ad.0 for ; Wed, 20 Nov 2024 03:51:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103468; x=1732708268; 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=ML4KPi4wQjf1qC2Qte9G4jPx11iLAGGFB8tIl6WAl74=; b=eanxGAC39J83WanP1dqxSEDq8mhbHucLxmvpfaBpXA3UYO9IwSfVJdUT8inWh3C5eN Lq86XregXmSlqWZn1XmegNOsjPghDk/9pKadGlEXoVhBQ36UYtjErA9SV2c680eI+GNQ fNm0Iea7LZxD4RTVWSGBqp4PyN2vf2xGrFMGGcQfcZCcFiDCc0W4HXoDwqo9iE/dzfXD rDpK0iFBh40KtAoRddESji0nrCP9dLy9bKuuYVZw1gEQ6oVjv6XLN7+3ejNQvQEm+r8a 5PHIArGrdYgipKeE2q/bJiRttEBmJ4CHGMJuyoEUs8LrroyYWdkVJgmj5pOtehVsEF/z UZTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103468; x=1732708268; 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=ML4KPi4wQjf1qC2Qte9G4jPx11iLAGGFB8tIl6WAl74=; b=bJau0NfDhaypICpX4yeKBFgoYJp0FVbhIYX12TPewP5UDabWVMDgdxTC6IOR41eXzu Vg45Wa64yiXC/RZZGR9U1PmrkSD8XE2/xiJ144Vt+PxEyyjRtjAwnLqbEwHlZnE9pdOC ttqJZDtgz6W6gfCGHdhyiyj2Rs/Kep1kGMX8ORTo8KbuMJZ4+pgPC7GWoy9665osmfdh LbyAlaHRZPF1WMjr1b/VqqQMjyUusT+XXmtFOC9AZEkkbRrCEExGSZ9DCfBgVWcvoDLi ym2LubXoagE5h+4yse5q4Z3WNiW2giOC1OQPqfIppbXxyTPOHyBX/165dSv/I7ssoXWG +iaw== X-Gm-Message-State: AOJu0Yxx/bi8H1uGIzJqdc2whtiPBre9gPNDIrENFnudwiTg9lMvoEKR eZoGCMV9sRNZySZ9HbakL0PDSDzQIqdBWi9eQCe22fa8Lhjlcht0Zr5Stg== X-Gm-Gg: ASbGncterVqB7AnTo+3iiveGbQ482XYVeeJn8W4WEQ9F1rxQcR0XNWNutJ/iX/prvm+ ROFeJhyFZKXaX00rVx5mgiM+hi5sotZmcpGLTAFM1zqeh/uTWQ96HrLpXFM6aXRfD2mV44IkyQa cmdug/60ATLGU7rMd3aw8QquohphuYFLep4ia9vQl3DGAeLDgKejTXvCbqSmK3vivMzZwN1KIEH rOgVuNcIbXlapYajTOKEG+Fpkft1aUd0y08k0aCQnaA+w== X-Google-Smtp-Source: AGHT+IFSFa3q3ex1gM+fk4TihTd9rJp//FNPJXGvmWYcfsmagJeUTmD0sGUwKCmTyGShnqq4CBDKwg== X-Received: by 2002:a17:903:32c8:b0:20b:a10c:9bdf with SMTP id d9443c01a7336-2126a3f4199mr29128795ad.32.1732103467905; Wed, 20 Nov 2024 03:51:07 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2ead04d2b9dsm1114905a91.33.2024.11.20.03.51.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:51:07 -0800 (PST) Date: Wed, 20 Nov 2024 19:51:16 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 2/9] ref: check the full refname instead of basename 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 "files-backend.c::files_fsck_refs_name", we validate the refname format by using "check_refname_format" to check the basename of the iterator with "REFNAME_ALLOW_ONELEVEL" flag. However, this is a bad implementation. Although we doesn't allow a single "@" in ".git" directory, we do allow "refs/heads/@". So, we will report an error wrongly when there is a "refs/heads/@" ref by using one level refname "@". Because we just check one level refname, we either cannot check the other parts of the full refname. And we will ignore the following errors: "refs/heads/ new-feature/test" "refs/heads/~new-feature/test" In order to fix the above problem, enhance "files_fsck_refs_name" to use the full name for "check_refname_format". Then, replace the tests which are related to "@" and add tests to exercise the above situations using for loop to avoid repetition. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 7 ++- t/t0602-reffiles-fsck.sh | 92 ++++++++++++++++++++++++---------------- 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 03d2503276..b055edc061 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3519,10 +3519,13 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock")) goto cleanup; - if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) { + /* + * This works right now because we never check the root refs. + */ + strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); + if (check_refname_format(sb.buf, 0)) { struct fsck_ref_report report = { 0 }; - strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); report.path = sb.buf; ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_NAME, diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 71a4d1a5ae..2a172c913d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -18,63 +18,81 @@ test_expect_success 'ref name should be checked' ' cd repo && git commit --allow-empty -m initial && - git checkout -b branch-1 && - git tag tag-1 && - git commit --allow-empty -m second && - git checkout -b branch-2 && - git tag tag-2 && - git tag multi_hierarchy/tag-2 && + git checkout -b default-branch && + git tag default-tag && + git tag multi_hierarchy/default-tag && - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/.branch-1: badRefName: invalid refname format - EOF - rm $branch_dir_prefix/.branch-1 && - test_cmp expect err && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/@: badRefName: invalid refname format - EOF + cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && + git refs verify 2>err && + test_must_be_empty err && rm $branch_dir_prefix/@ && - test_cmp expect err && - cp $tag_dir_prefix/multi_hierarchy/tag-2 $tag_dir_prefix/multi_hierarchy/@ && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/multi_hierarchy/@: badRefName: invalid refname format - EOF - rm $tag_dir_prefix/multi_hierarchy/@ && - test_cmp expect err && - - cp $tag_dir_prefix/tag-1 $tag_dir_prefix/tag-1.lock && + cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && git refs verify 2>err && rm $tag_dir_prefix/tag-1.lock && test_must_be_empty err && - cp $tag_dir_prefix/tag-1 $tag_dir_prefix/.lock && + cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && test_must_fail git refs verify 2>err && cat >expect <<-EOF && error: refs/tags/.lock: badRefName: invalid refname format EOF rm $tag_dir_prefix/.lock && - test_cmp expect err + test_cmp expect err && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname: badRefName: invalid refname format + EOF + rm "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/multi_hierarchy/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + mkdir "$branch_dir_prefix/$refname" && + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname/default-branch: badRefName: invalid refname format + EOF + rm -r "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done ' test_expect_success 'ref name check should be adapted into fsck messages' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - tag_dir_prefix=.git/refs/tags && cd repo && git commit --allow-empty -m initial && git checkout -b branch-1 && - git tag tag-1 && - git commit --allow-empty -m second && - git checkout -b branch-2 && - git tag tag-2 && cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && git -c fsck.badRefName=warn refs verify 2>err && @@ -84,7 +102,7 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' rm $branch_dir_prefix/.branch-1 && test_cmp expect err && - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ && + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && git -c fsck.badRefName=ignore refs verify 2>err && test_must_be_empty err ' From patchwork Wed Nov 20 11:51:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881083 Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) (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 2B8601AA7B1 for ; Wed, 20 Nov 2024 11:51:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103478; cv=none; b=OtMnN1osO6mg6PvTZ4s4i7KRsqdHc/X4mcaKIyf74ZNFm9IIszhGCrNwGX5PxHbVLYWwB1+biPOTtuSZ6BTIfZJ+U2B1Lj1ekDyhr7Tva+5GNrQIFm4JnmJfeP8O2XJopso4tlF6qKxymI9khtMuvrdLQXBVlF0uEMpzInjHIfU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103478; c=relaxed/simple; bh=5V0XiAbP8HWSYG2I6t6wyCWdiAvyF9V1n8no7T7g7Rw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=dUdIcEgVa95NMAsojBHYA4CWiOB4ScUm129U36dvqLsBO3UbXwHwaL4yGVNvfCK5SxyGN15r76EKemkKUEmbucY7vwNA3zn0wKtc0hpdqLI7IAdbXtC0t4HLIKlS4INQ65ArE9SWJ/lAv1JZXuqeaLNBsho4I+SyU1LAkJRTDAg= 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=QMLIowbd; arc=none smtp.client-ip=209.85.210.179 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="QMLIowbd" Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-724455f40a0so1534348b3a.0 for ; Wed, 20 Nov 2024 03:51:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103476; x=1732708276; 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=zJ75sjuyqz5k++4m10zIcF0Gbi0g1hHVhXO/JjRwBWE=; b=QMLIowbdJQ6AOybTrfNLr92sH3JHKook3CpXtjZzQZDl3Wwk7LXlNTUEFzN2fVnbfa w9tJBajA/O15a5Fu3l2nyCbZOqtNR7QYrrVRBGInAo4sbP0FZBO80NESoYsrS12yxhUC Uyhg2r7fzS2CtOgyk0iRtpRTVhkk7jMUL8jeyWaIw9cxuBxy8sJG5RIaxGaQB8i+RKvZ VtEPJKZHbjKloDM0cDQ2M+SMhZp/ybqrJjPFUsjCO05GOpp/uhiMy0MRtctJRJX7i958 ZC8650CKjhS3lu9QaER/l3ac+sNJgUdBELg/AaKHuEyVMN2wqjwCD7EagzsCphhVC/Fv qtRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103476; x=1732708276; 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=zJ75sjuyqz5k++4m10zIcF0Gbi0g1hHVhXO/JjRwBWE=; b=ARRN5diEKaBESfl0x0NOh6aL75dCVfRgKgDVUMTrpj519lYMmEiYwGCR/izpxZjNwv EnmfSKZv8I3QsPXDbh0DLfelavXGCxCt9+eHvdgZGoQvEM3uw9O6YMGXtrei5h7aDQO7 beHU24sA/F3+kuHTeFsAnKitV9RmLg0vs0bn+Sm4jp2yRCh+lA8ceRN5rCFColGwqkYG U//bkQDTZm1Hj9IfyvJjqVTXbYXQYtp8QjgSQjehdyjVLZgDjE0FulpMFl44GOtc78Ce LdZvn7fbdWKv9T3aOvV+ORQ790F+9feTwQXNgHEmtB3Ovq6P/awT2ocxJPzGW7cFpG5V LxEA== X-Gm-Message-State: AOJu0YyqX3Q607cTwt2YBN97sKbFnUlSpAJ4ncHNiroZ451EpTEF4XOn TWWJNL5qbJAzGVYWdarsBHfeCDabs6kDXAFfbm61PM2/kTZOQzOYuHJcXg== X-Google-Smtp-Source: AGHT+IGVB2BDk9XLCCxpmo/EEXUKN5Y//luOBrhJ7DDbpu670NGL4rMc4vDLfXPpqGgez0d6ktG6OQ== X-Received: by 2002:a05:6a00:c94:b0:71e:4196:ddb2 with SMTP id d2e1a72fcca58-724becf6a48mr3158500b3a.9.1732103475801; Wed, 20 Nov 2024 03:51:15 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-724befe9b4dsm1413792b3a.191.2024.11.20.03.51.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:51:15 -0800 (PST) Date: Wed, 20 Nov 2024 19:51:24 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 3/9] ref: initialize ref name outside of check functions 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 passes "refs_check_dir" to the "files_fsck_refs_name" function which allows it to create the checked ref name later. However, when we introduce a new check function, we have to allocate redundant memory and re-calculate the ref name. It's bad for us to allocate redundant memory and duplicate logic. Instead, we should allocate and calculate it only once and pass the ref name to the check functions. In order not to do repeat calculation, rename "refs_check_dir" to "refname". And in "files_fsck_refs_dir", create a new strbuf "refname", thus whenever we handle a new ref, calculate the name and call the check functions one by one. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index b055edc061..8edb700568 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3501,12 +3501,12 @@ static int files_ref_store_remove_on_disk(struct ref_store *ref_store, */ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, struct fsck_options *o, - const char *refs_check_dir, + const char *refname, struct dir_iterator *iter); static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, struct fsck_options *o, - const char *refs_check_dir, + const char *refname, struct dir_iterator *iter) { struct strbuf sb = STRBUF_INIT; @@ -3522,11 +3522,10 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, /* * This works right now because we never check the root refs. */ - strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); - if (check_refname_format(sb.buf, 0)) { + if (check_refname_format(refname, 0)) { struct fsck_ref_report report = { 0 }; - report.path = sb.buf; + report.path = refname; ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_NAME, "invalid refname format"); @@ -3542,6 +3541,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, const char *refs_check_dir, files_fsck_refs_fn *fsck_refs_fn) { + struct strbuf refname = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; struct dir_iterator *iter; int iter_status; @@ -3560,11 +3560,15 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, continue; } else if (S_ISREG(iter->st.st_mode) || S_ISLNK(iter->st.st_mode)) { + strbuf_reset(&refname); + strbuf_addf(&refname, "%s/%s", refs_check_dir, + iter->relative_path); + if (o->verbose) - fprintf_ln(stderr, "Checking %s/%s", - refs_check_dir, iter->relative_path); + fprintf_ln(stderr, "Checking %s", refname.buf); + for (size_t i = 0; fsck_refs_fn[i]; i++) { - if (fsck_refs_fn[i](ref_store, o, refs_check_dir, iter)) + if (fsck_refs_fn[i](ref_store, o, refname.buf, iter)) ret = -1; } } else { @@ -3581,6 +3585,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, out: strbuf_release(&sb); + strbuf_release(&refname); return ret; } From patchwork Wed Nov 20 11:51:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881084 Received: from mail-oa1-f46.google.com (mail-oa1-f46.google.com [209.85.160.46]) (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 493AE1AA7BA for ; Wed, 20 Nov 2024 11:51:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103487; cv=none; b=km+Toazta/ipWORfKQRpOof5prIYmliWfhR1jsgCPihm1gtEKnePRZkCqBogdW+U9W8kwMHBPnq1w0cT3St97n9rRtd6tzklQ15koE84n/msFqlhwxBF9GXwdHpnZ56lbjmeiVa/I9jMM2j/C3myZhhQMeJiYwf5W4X2RzN2mUw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103487; c=relaxed/simple; bh=UMmNFe+UimwvkhTx+Oxb3XjhyVe27DK+GxSHcDqvSRw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=HUKsomUwSTRPw/6vZD5p2BgeUnD9v51XdKQr8DRbA9Jky+f3er+KlD1KKj1qnVpo2XoC4CvHybokytI4rUggmMz2JMRsPtm0Yt4dZCCvSEr6hxW8Ehc30QiQ+/qA+QnOnQ3MZNzINSfYDAhVM0zzDpNRMuL/r9Nxcg95/dQX0FM= 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=bUqiZ6+g; arc=none smtp.client-ip=209.85.160.46 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="bUqiZ6+g" Received: by mail-oa1-f46.google.com with SMTP id 586e51a60fabf-29666708da7so2262188fac.0 for ; Wed, 20 Nov 2024 03:51:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103484; x=1732708284; 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=oCrwS1CZXCGmjcEuTdAojT1OFTke1dmbq+oiF708lmM=; b=bUqiZ6+gNXkJh3HzXNHZHg1BU5Kh8ENtIRH6rI7Yxn3Z7t9ARCLIVWNb03E7FaM4oQ LucoXuVJffxKB1jjtVu1prwYQbe7FqOdpmLiSvljLVD33Lsk3EcvuEW0FLI72329RMev pxwStALDom9HbTxtC0461MCoQ5sisgdrypmFwYnhCJMaHhsnDokRNuT2Vc402LE5BpTs dklIKux3I6pROAHDiQub0IWrRWIb3TaDIQbihGGyo+Iqcg8LlXBSfi9dGoKj/23Oe/NV +1dueiyppfLmMMJg71tEFryUNWzQTufwYWuTTLToikoFu30eYG/8qaHluP4bex2nXM9k BeBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103484; x=1732708284; 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=oCrwS1CZXCGmjcEuTdAojT1OFTke1dmbq+oiF708lmM=; b=joeA1D08d1APcQpDpULWL5FW6yuZj5T3BA5Lc/giNou4zGevis0A7TjWTHIN1pWBPb 9b6N4d3ooNwukoQ9yL+/0r8epCkqx3iPId+wbK5VvXsrhRjBhM+ZZmyc97jraS3+qyjG 7MxCoqWkJjsVJrby3vfS7okZOYUO/s+/GYxD4n8+Wl0iAYgkxNHPqDJmcFZtqVG0oFM5 7tKnxpQe++puzUW2boOGY8GKqNdivC8kMFlFlaOtUJGelMONHSy8ErXzQK9g2hui26xP 8wKDOi/CMGLYUKkozQrzph7s6KOTVAMjmA8qGqKwuY+xmOmdI1xgH0pxAwGJB+t/SjhR mULQ== X-Gm-Message-State: AOJu0YyEf6m1JQJA8DsZkXTHDDiBYF6eif6yGjOy1XoYFdcbWAx7zqzh WWbyWvvZFsuvpwpcu0DlXnYE5Eqp3PgjTVCSFkk3UOQn5u809EG3vCWhuQ== X-Google-Smtp-Source: AGHT+IFSzSg7VdbeDagfEPij9OfG+BL+B631kecTIMiMDS9cEeoQYBCzApeiHe5Fk6zbnnEs6PksAg== X-Received: by 2002:a05:6870:b153:b0:277:db7f:cfb2 with SMTP id 586e51a60fabf-296d9b95fb6mr2441225fac.14.1732103483762; Wed, 20 Nov 2024 03:51:23 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7f8c1c31b93sm9297889a12.22.2024.11.20.03.51.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:51:23 -0800 (PST) Date: Wed, 20 Nov 2024 19:51:32 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 4/9] ref: support multiple worktrees check for 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 set up the infrastructure to check the consistency for refs, but we do not support multiple worktrees. However, "git-fsck(1)" will check the refs of worktrees. As we decide to get feature parity with "git-fsck(1)", we need to set up support for multiple worktrees. Because each worktree has its own specific refs, instead of just showing the users "refs/worktree/foo", we need to display the full name such as "worktrees//refs/worktree/foo". So we should know the id of the worktree to get the full name. Add a new parameter "struct worktree *" for "refs-internal.h::fsck_fn". Then change the related functions to follow this new interface. The "packed-refs" only exists in the main worktree, so we should only check "packed-refs" in the main worktree. Use "is_main_worktree" method to skip checking "packed-refs" in "packed_fsck" function. Then, enhance the "files-backend.c::files_fsck_refs_dir" function to add "worktree//" prefix when we are not in the main worktree. Last, add a new test to check the refname when there are multiple worktrees to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/refs.c | 10 ++++++-- refs.c | 5 ++-- refs.h | 3 ++- refs/debug.c | 5 ++-- refs/files-backend.c | 17 ++++++++++---- refs/packed-backend.c | 8 ++++++- refs/refs-internal.h | 3 ++- refs/reftable-backend.c | 3 ++- t/t0602-reffiles-fsck.sh | 51 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 90 insertions(+), 15 deletions(-) diff --git a/builtin/refs.c b/builtin/refs.c index 24978a7b7b..394b4101c6 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -5,6 +5,7 @@ #include "parse-options.h" #include "refs.h" #include "strbuf.h" +#include "worktree.h" #define REFS_MIGRATE_USAGE \ N_("git refs migrate --ref-format= [--dry-run]") @@ -66,6 +67,7 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix) static int cmd_refs_verify(int argc, const char **argv, const char *prefix) { struct fsck_options fsck_refs_options = FSCK_REFS_OPTIONS_DEFAULT; + struct worktree **worktrees; const char * const verify_usage[] = { REFS_VERIFY_USAGE, NULL, @@ -75,7 +77,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "strict", &fsck_refs_options.strict, N_("enable strict checking")), OPT_END(), }; - int ret; + int ret = 0; argc = parse_options(argc, argv, prefix, options, verify_usage, 0); if (argc) @@ -84,9 +86,13 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix) git_config(git_fsck_config, &fsck_refs_options); prepare_repo_settings(the_repository); - ret = refs_fsck(get_main_ref_store(the_repository), &fsck_refs_options); + worktrees = get_worktrees(); + for (size_t i = 0; worktrees[i]; i++) + ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), + &fsck_refs_options, worktrees[i]); fsck_options_clear(&fsck_refs_options); + free_worktrees(worktrees); return ret; } diff --git a/refs.c b/refs.c index 5f729ed412..395a17273c 100644 --- a/refs.c +++ b/refs.c @@ -318,9 +318,10 @@ int check_refname_format(const char *refname, int flags) return check_or_sanitize_refname(refname, flags, NULL); } -int refs_fsck(struct ref_store *refs, struct fsck_options *o) +int refs_fsck(struct ref_store *refs, struct fsck_options *o, + struct worktree *wt) { - return refs->be->fsck(refs, o); + return refs->be->fsck(refs, o, wt); } void sanitize_refname_component(const char *refname, struct strbuf *out) diff --git a/refs.h b/refs.h index 108dfc93b3..341d43239c 100644 --- a/refs.h +++ b/refs.h @@ -549,7 +549,8 @@ int check_refname_format(const char *refname, int flags); * reflogs are consistent, and non-zero otherwise. The errors will be * written to stderr. */ -int refs_fsck(struct ref_store *refs, struct fsck_options *o); +int refs_fsck(struct ref_store *refs, struct fsck_options *o, + struct worktree *wt); /* * Apply the rules from check_refname_format, but mutate the result until it diff --git a/refs/debug.c b/refs/debug.c index 45e2e784a0..72e80ddd6d 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -420,10 +420,11 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname, } static int debug_fsck(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; - int res = drefs->refs->be->fsck(drefs->refs, o); + int res = drefs->refs->be->fsck(drefs->refs, o, wt); trace_printf_key(&trace_refs, "fsck: %d\n", res); return res; } diff --git a/refs/files-backend.c b/refs/files-backend.c index 8edb700568..8bfdce64bc 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -23,6 +23,7 @@ #include "../dir.h" #include "../chdir-notify.h" #include "../setup.h" +#include "../worktree.h" #include "../wrapper.h" #include "../write-or-die.h" #include "../revision.h" @@ -3539,6 +3540,7 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, static int files_fsck_refs_dir(struct ref_store *ref_store, struct fsck_options *o, const char *refs_check_dir, + struct worktree *wt, files_fsck_refs_fn *fsck_refs_fn) { struct strbuf refname = STRBUF_INIT; @@ -3561,6 +3563,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, } else if (S_ISREG(iter->st.st_mode) || S_ISLNK(iter->st.st_mode)) { strbuf_reset(&refname); + + if (!is_main_worktree(wt)) + strbuf_addf(&refname, "worktrees/%s/", wt->id); strbuf_addf(&refname, "%s/%s", refs_check_dir, iter->relative_path); @@ -3590,7 +3595,8 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, } static int files_fsck_refs(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { files_fsck_refs_fn fsck_refs_fn[]= { files_fsck_refs_name, @@ -3599,17 +3605,18 @@ static int files_fsck_refs(struct ref_store *ref_store, if (o->verbose) fprintf_ln(stderr, _("Checking references consistency")); - return files_fsck_refs_dir(ref_store, o, "refs", fsck_refs_fn); + return files_fsck_refs_dir(ref_store, o, "refs", wt, fsck_refs_fn); } static int files_fsck(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ, "fsck"); - return files_fsck_refs(ref_store, o) | - refs->packed_ref_store->be->fsck(refs->packed_ref_store, o); + return files_fsck_refs(ref_store, o, wt) | + refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt); } struct ref_storage_be refs_be_files = { diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 07c57fd541..46dcaec654 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -13,6 +13,7 @@ #include "../lockfile.h" #include "../chdir-notify.h" #include "../statinfo.h" +#include "../worktree.h" #include "../wrapper.h" #include "../write-or-die.h" #include "../trace2.h" @@ -1754,8 +1755,13 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s } static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED) + struct fsck_options *o UNUSED, + struct worktree *wt) { + + if (!is_main_worktree(wt)) + return 0; + return 0; } diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2313c830d8..037d7991cd 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -653,7 +653,8 @@ typedef int read_symbolic_ref_fn(struct ref_store *ref_store, const char *refnam struct strbuf *referent); typedef int fsck_fn(struct ref_store *ref_store, - struct fsck_options *o); + struct fsck_options *o, + struct worktree *wt); struct ref_storage_be { const char *name; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index f5f957e6de..b6a63c1015 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -2443,7 +2443,8 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store, } static int reftable_be_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED) + struct fsck_options *o UNUSED, + struct worktree *wt UNUSED) { return 0; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 2a172c913d..1e17393a3d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -107,4 +107,55 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' test_must_be_empty err ' +test_expect_success 'ref name check should work for multiple worktrees' ' + test_when_finished "rm -rf repo" && + git init repo && + + cd repo && + test_commit initial && + git checkout -b branch-1 && + test_commit second && + git checkout -b branch-2 && + test_commit third && + git checkout -b branch-3 && + git worktree add ./worktree-1 branch-1 && + git worktree add ./worktree-2 branch-2 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err && + + for worktree in "worktree-1" "worktree-2" + do + ( + cd $worktree && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err || return 1 + ) + done +' + test_done From patchwork Wed Nov 20 11:51:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881085 Received: from mail-pg1-f176.google.com (mail-pg1-f176.google.com [209.85.215.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7FBAD1AAE0C for ; Wed, 20 Nov 2024 11:51:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103497; cv=none; b=NLgk9QZllwlMSwNB3fXs5PZrMLGUyJYaHlMzaN7QP8kIEiXY2+Ktzb6D5NW/3GJ5rC7Eb/JC7YLVDbZn5vSi1/Lb5VHqnalSFwgQvasbRGlFDCt3KIqCNOHgy+UwCyuDF1V+C3Ndey0hixjB7kKa/JvBtCcAqgpFodDOe6r6jrA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103497; c=relaxed/simple; bh=qJ81ExCHbCNdSUmSHASX4hcz6ZOAPSepsQw1focHBHs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=n2Ow2/DUYswDlCGoCg31nlY9I92mUKGMhSCCHG4uXaGfH9YvMeun3Oj5sNg+zCET+vakhou1QDh4ak3/O7rocQWsX3tE0JTpZWrx2HXBppHSOfYuo0ZZfeJHWISh4tGBa0QfzAlsLp4a1Z1gU+/vjxAY+XJFCSVFcVjfBSFohQY= 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=BSV5JKaQ; arc=none smtp.client-ip=209.85.215.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BSV5JKaQ" Received: by mail-pg1-f176.google.com with SMTP id 41be03b00d2f7-7f3da2c2cb5so1476570a12.2 for ; Wed, 20 Nov 2024 03:51:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103493; x=1732708293; 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=L+hX8rRC7mutbNXprXUpAnbvB+6epTFpOsVa5uBlYZA=; b=BSV5JKaQzTswcAT/cae7Z0ilf7f/iJVJ+fz2l+VLP9WpcEISz/ibKwJ1dPyut3iQ0w DHoF0gjSzX1Kmvhd2sAgd58uSdiAu7VbYuIYbC+d6/CX1cFBxyM7W78HHzzmLkw2qtub ij/Y69Y80EuuOipgWy6WE70HQaC17J3KJCGkjMPCuR7z4UpezHh9ILMf1DTI5mMIkRyb eZkBdVPDrWU+7CNf6OKgSpFEZK3Aztp45xPH4ejZRpwy7D5Of6fD/wqK8twwY8t9WkJQ GEkNK9uMpe90VqCULuJw+BuMRkGQdN7vqJ3hEk1E9Q79DzgCgTxhAd9gM6wsjfq8npHx woUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103493; x=1732708293; 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=L+hX8rRC7mutbNXprXUpAnbvB+6epTFpOsVa5uBlYZA=; b=LS0pDtD6+OrHUNmTDGisqYsj0NdAkFmetCe8b6fgDiIGQ8EA9m7e/1aiEzDn4EtYeF 1i0dzm3cXvFeg3nlKkV+cmWx17OekLj6GcHW3MPmc1FOzfE9Rapo78CNug5SpIINtv6r g5FdPkCUncXleCVmLqzKKLLctP8ClvavHhWsI0Lz2dZIsaP11i8Zkpv3ABThj7kpaMnO gQU3vykIZqtsAj/78GhkdZGtXyMhWYDq2FLeJw+1JKaYbPFxp5S9v4h6aPpR4dRTuLbb 0CbsmP6XxgJqvsuM5cOfArI17FiXOQ/fDCiJ9rzs30vghFjuXvdHJwUowNVNJbuTeuT9 FS7w== X-Gm-Message-State: AOJu0YwJNF5Xo2lnq5f+9FPxKFCC2F6pT3wHpSWtlfBMPeJ5tfwzelKL Rsr4AjH3nookeRUARxzdK/NNNpXrhPIzt+vT5V1kZV70hdD0gtmUtDYvCA== X-Gm-Gg: ASbGncvXdbAp6yVEav+qrdp0RwLxMHJyzN8zWMpOmZQaQwGX13Ipo3HwjJF4wq50aK+ IR3OfXSvk8D7kXZkAB+is/+Jw6BqPaSSg5wBey5ZqgYEfOePIHsxYIeTIE7tNQyrmG3VJiZF2U5 Fpx29vDqCZL9ISeUmD1Ylifgxyrg3cKHnJpIqt2CKjraJNRgmdqBF5LfcD4wUOC8rXilhR87/Tr yfkBOscqOsgjLae2yGBE77dSl9ucFe0R3iZep1ZSQDUZA== X-Google-Smtp-Source: AGHT+IEvbwYYEYU4Q6eu2A39BupO7+5o0dpK0o9adwskRVm5r24wp+ujAz/eGIXJhfPeFR/gzMM6hQ== X-Received: by 2002:a05:6a20:8418:b0:1db:f823:109f with SMTP id adf61e73a8af0-1ddb0625812mr3559495637.31.1732103493267; Wed, 20 Nov 2024 03:51:33 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-724beeb82fdsm1422752b3a.14.2024.11.20.03.51.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:51:32 -0800 (PST) Date: Wed, 20 Nov 2024 19:51:42 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 5/9] 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: "git-fsck(1)" implicitly checks the ref content by passing the callback "fsck_handle_ref" to the "refs.c::refs_for_each_rawref". Then, it will check whether the ref content (eventually "oid") is valid. If not, it will report the following error to the user. error: refs/heads/main: invalid sha1 pointer 0000... And it will also report above errors when there are dangling symrefs in the repository wrongly. This does not align with the behavior of the "git symbolic-ref" command which allows users to create dangling symrefs. As we have already introduced the "git refs verify" command, we'd better check the ref content explicitly in the "git refs verify" command thus later we could remove these checks in "git-fsck(1)" and launch a subprocess to call "git refs verify" in "git-fsck(1)" to make the "git-fsck(1)" more clean. Following what "git-fsck(1)" does, add a similar check to "git refs verify". Then add a new fsck error message "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 | 47 +++++++++++++++ t/t0602-reffiles-fsck.sh | 105 ++++++++++++++++++++++++++++++++++ 4 files changed, 156 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 8bfdce64bc..9f300a7d3c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3505,6 +3505,52 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refname, struct dir_iterator *iter); +static int files_fsck_refs_content(struct ref_store *ref_store, + struct fsck_options *o, + const char *target_name, + struct dir_iterator *iter) +{ + struct strbuf ref_content = STRBUF_INIT; + struct strbuf referent = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + unsigned int type = 0; + int failure_errno = 0; + struct object_id oid; + int ret = 0; + + report.path = target_name; + + if (S_ISLNK(iter->st.st_mode)) + goto cleanup; + + if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { + /* + * Ref file could be removed by another concurrent process. We should + * ignore this error and continue to the next ref. + */ + if (errno == ENOENT) + goto cleanup; + + ret = error_errno(_("cannot read ref file '%s'"), iter->path.buf); + goto cleanup; + } + + if (parse_loose_ref_contents(ref_store->repo->hash_algo, + ref_content.buf, &oid, &referent, + &type, &failure_errno)) { + strbuf_rtrim(&ref_content); + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "%s", ref_content.buf); + goto cleanup; + } + +cleanup: + 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 *refname, @@ -3600,6 +3646,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 1e17393a3d..162370077b 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -158,4 +158,109 @@ test_expect_success 'ref name check should work for multiple worktrees' ' done ' +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 && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + git refs verify 2>err && + test_must_be_empty err && + + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && + + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$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: $bad_content + EOF + rm $branch_dir_prefix/a/b/branch-bad && + test_cmp expect err || return 1 + done +' + +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" && + + bad_content_1=$(git rev-parse main)x && + bad_content_2=xfsazqfxcadas && + bad_content_3=Xfsazqfxcadas && + printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && + printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && + printf "%s" $bad_content_3 >$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: $bad_content_3 + error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 + error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + +test_expect_success 'ref content checks should work with worktrees' ' + test_when_finished "rm -rf repo" && + git init repo && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + EOF + rm $worktree1_refdir_prefix/bad-branch-1 && + test_cmp expect err || return 1 + done && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + EOF + rm $worktree2_refdir_prefix/bad-branch-2 && + test_cmp expect err || return 1 + done +' + test_done From patchwork Wed Nov 20 11:51:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881086 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.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 BAD351AB521 for ; Wed, 20 Nov 2024 11:51:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103504; cv=none; b=drlyQxl0Oj9czrFq5kPFgvGWNeZrBIKb0iMMcvHG8+SHTx3acuVkEpNOQGVvVD67fw+6xhZLqAx+EMuWs5wJVK191IeQhWDm0kVrW0HuogOk7LTJqRvKNDyVDq/4/RFYymfAs6UhIDsYWyraEchBdi8ZdSMl84ZBssUh9q75fGY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103504; c=relaxed/simple; bh=VXTNCjMxU7IGFnOUdKV1X6Jyr1LU3pXvCgj4Tar9PsQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=sWBM/ERpiPFR7QhyRVmTGzPsOwmZE1mKH4S3k16ESuf1cf81KBy1FSTStwPXKk3m8gF1NSg8/7jHcm8ev2foBo7vxGzHDe0NhHEWwS6kBsSZKYDMbQjhkb6JaY4DG065fNNtWd08KNl9raYsoWopQ5trF1MR2VZFnLhCU/px0TE= 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=L4TTtYwm; arc=none smtp.client-ip=209.85.214.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="L4TTtYwm" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-211fcadd960so31314645ad.2 for ; Wed, 20 Nov 2024 03:51:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103501; x=1732708301; 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=pLLMOHnIlwhlDAu2xCp95eCTzbt38epAoYjaE0zY0yo=; b=L4TTtYwmcmJPkzBVbQ30+fSzA8FBx8rbQqXBmtF01+rimMddwK4UGAGy3/cbMpTRLL NJEoZ/P955o5h+bJsgV+PKmuUJBsNIr54J4QjLVho5ctdf+qGWY2JAwBQZ3+JDvf+x/i EOq2VIc5pBXmXfW6055WCHecqvjKmrX2O/7cO0TcWopXo62cVlfzDvy59cBZyERXgukY IaBrlnW0wrgK8Z4bGdbF+zDCSdn9HnGJ4W66QeEGNKxnxzKL+jFdl5dAMpPMwy2izwjV QE56JRwt85c4OcSt3SsCSni9YTrGLf68NW+6z3+z185vCDzEif4QCQj0poajQBUbRQ6z u+gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103501; x=1732708301; 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=pLLMOHnIlwhlDAu2xCp95eCTzbt38epAoYjaE0zY0yo=; b=UKoG6tSYWqLmslYMLoTbRwOgeyzsxv4Z7Gac+blRISx4ymqc337ma4syVgCctdK3Ev tI0wnrrNmU/wVm4iE5Vmwzn4tuj6fpcdJxrtO3fG1qHrX7VQEKsVOZd4jj9FlXeN4I8t FTO6qWtEtISaNvwNNS5DeZuIkkpNN0/WBw2g/RjsgX5YxtC4qmYBJYALHKrPZaIGQZCz T04GqRqjQR/3s2qpysaSBw/O1kPDaoNFbW6Yezlsb1J4me9mvVxuyFMG5ssSsBOWmKcD ttYEPyenqcloxjntwJuEapGs+Ij6F6sjN2Ils6W6cptkxPi7n21kzzt5qzhzsOB6fEHJ +Tnw== X-Gm-Message-State: AOJu0YwVJxVqN+pz0dKY1aQ2Cl6WdnceCnPn/srselZhhuPyrFqiF5pO gkgleXKi4SnkGjbu9PwObDSBP7MzGAwbBpOIYc7IFd3G69R/lhssMtLknw== X-Google-Smtp-Source: AGHT+IGLAueBmaecIl7OyrqN+kPPPVakEkjU1eYgZHBTEDJbAT+r5CNpY+Z1qBsSX19Wsp2Wx6Y4cQ== X-Received: by 2002:a17:902:da8b:b0:211:e9c0:31ca with SMTP id d9443c01a7336-2126c965d15mr32072545ad.36.1732103501469; Wed, 20 Nov 2024 03:51:41 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-211e9d0c389sm76904665ad.161.2024.11.20.03.51.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:51:40 -0800 (PST) Date: Wed, 20 Nov 2024 19:51:49 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 6/9] 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 loose ref that does not end with newline(LF). 2. trailingRefContent(INFO): A loose ref has trailing content. 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 | 14 +++++++++ fsck.h | 2 ++ refs.c | 2 +- refs/files-backend.c | 26 ++++++++++++++-- refs/refs-internal.h | 2 +- t/t0602-reffiles-fsck.sh | 57 +++++++++++++++++++++++++++++++++-- 6 files changed, 96 insertions(+), 7 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 22c385ea22..6db0eaa84a 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -173,6 +173,20 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`refMissingNewline`:: + (INFO) A loose ref that does not end with newline(LF). As + valid implementations of Git never created such a loose ref + file, it may become an error in the future. Report to the + git@vger.kernel.org mailing list if you see this error, as + we need to know what tools created such a file. + +`trailingRefContent`:: + (INFO) A loose ref has trailing content. As valid implementations + of Git never created such a loose ref file, it may become an + error in the future. Report to the git@vger.kernel.org mailing + list if you see this error, as we need to know what tools + created such a file. + `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 395a17273c..f88b32a633 100644 --- a/refs.c +++ b/refs.c @@ -1789,7 +1789,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 9f300a7d3c..3d4d612420 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -569,7 +569,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) @@ -606,7 +606,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)) { @@ -628,6 +628,10 @@ int parse_loose_ref_contents(const struct git_hash_algo *algop, *failure_errno = EINVAL; return -1; } + + if (trailing) + *trailing = p; + return 0; } @@ -3513,6 +3517,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, struct strbuf ref_content = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct fsck_ref_report report = { 0 }; + const char *trailing = NULL; unsigned int type = 0; int failure_errno = 0; struct object_id oid; @@ -3537,7 +3542,7 @@ 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)) { strbuf_rtrim(&ref_content); ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_CONTENT, @@ -3545,6 +3550,21 @@ static int files_fsck_refs_content(struct ref_store *ref_store, goto cleanup; } + if (!(type & REF_ISSYMREF)) { + if (!*trailing) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_REF_MISSING_NEWLINE, + "misses LF at the end"); + goto cleanup; + } + if (*trailing != '\n' || *(trailing + 1)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_TRAILING_REF_CONTENT, + "has trailing garbage: '%s'", trailing); + goto cleanup; + } + } + cleanup: strbuf_release(&ref_content); strbuf_release(&referent); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 037d7991cd..125f1fe735 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -716,7 +716,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 162370077b..33e7a390ad 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -189,7 +189,48 @@ test_expect_success 'regular ref content should be checked (individual)' ' EOF rm $branch_dir_prefix/a/b/branch-bad && test_cmp expect err || return 1 - done + done && + + 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: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && + + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err || return 1 + done && + + printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + + + '\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + + + garbage'\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err ' test_expect_success 'regular ref content should be checked (aggregate)' ' @@ -207,12 +248,16 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && test_must_fail git refs verify 2>err && cat >expect <<-EOF && error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end EOF sort err >sorted_err && test_cmp expect sorted_err @@ -260,7 +305,15 @@ test_expect_success 'ref content checks should work with worktrees' ' EOF rm $worktree2_refdir_prefix/bad-branch-2 && test_cmp expect err || return 1 - done + done && + + printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end + EOF + rm $worktree1_refdir_prefix/branch-no-newline && + test_cmp expect err ' test_done From patchwork Wed Nov 20 11:52:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881087 Received: from mail-ot1-f45.google.com (mail-ot1-f45.google.com [209.85.210.45]) (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 A64281AB522 for ; Wed, 20 Nov 2024 11:51:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103515; cv=none; b=WggVsdNHN4UeR2D22EpKS790MifdRdKzbVBfr0NXtz4BQhcymTzdYYMEERhpikQz0Ba1J/KS98pkCdtQ+JCWSzd9/N3sMJqSaairZWx/oZsgZTKwgjS+Pw8sMp/yoaK/tVQ5672shIWKfsElgNaLGQ5ZTL37CRxUeqbBp/6cuL8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103515; c=relaxed/simple; bh=Nktntsil2G5AtxRr+EyUWm6oCV/PTcE7+SJLjxxWujU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=aRC7KMV6SXM2qvj7kAxlfMbHB+MGnkMjWQWT+NjPMmxeBcRTO2j5DMTGiOaWSaLlB2bYOwtfh406vaZB4iJNV1EXNMzlq9vC+lyt2yebwiD07APcz9WLTVm5ibx7F4Uk8Nm4aVFrHSTuns57KfYGmX/CAHeRrpb4ph+/EunkST4= 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=NZC4WHkv; arc=none smtp.client-ip=209.85.210.45 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="NZC4WHkv" Received: by mail-ot1-f45.google.com with SMTP id 46e09a7af769-7180dc76075so1051527a34.3 for ; Wed, 20 Nov 2024 03:51:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103512; x=1732708312; 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=MccQ/jzT2lZ6XSy4YgxEoE5R8k6KAr8mb9pafaLobNE=; b=NZC4WHkvqHR26XmVqpkF35VlqxCta368ReYZcWt3nM6v/IQdJTRATSHhQ+HuoT2wy6 W3UzEGSliyYVTNiwoWTwM+9y8rRPci0G8wXqNsD5g+HEYp4tMDBFX1INPlChkfDSJRwp sqcA2F0IN3QWDrurr0tZcSLtV8aANRjEFABYIth/8k6DeWlhwaQGtbciBBMDP88du4Rx y3FCnrKoSLXw4R5zD2UXCsLPijTUbAwmq3p0rhxwOjGcGupvq88qiZDRdphV1XXky/eP g6s6jOXKULC4JTyUSQzbXQQDTbp3svyBsITHSC2/+IuMZ2aVAYnUZJ9datYg/Tsa5veF x4sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103512; x=1732708312; 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=MccQ/jzT2lZ6XSy4YgxEoE5R8k6KAr8mb9pafaLobNE=; b=PfCXXrirK3TyC7i38M/BfyB+sgZ6RrslrbtE9VEZQfVTQ44b3R1njJA6D9KNCfs1Fy tOH/N3NrvdLFjTTZfnvxXZLqLO5V5GPcG5//Z9fOdA7bs5PBYDZzzAwLDXFStPJWdT5V RRYsWN+oZaCgxr64TgYJynKsBmMElCgaqrzPkIR376RJ7UrO2c+Mg7ABBB//fXwf5HkE FEHHhrZ0nlKTXG+2HPWuBvd2EI8cmeEGIpeSGTOXIv+POGF+i7urFu8Wof5bpTnVG/bJ cqlsCC17YY3WOp20/Sw8cxgeIK2k8AbRj1YpxLr9VRereeyd+ZF85GLCO84BTjalkORI KLQg== X-Gm-Message-State: AOJu0Yy4y2cOL08TAF3GuUq48ydonrlNZspGxKbA5/3jHS+wa/K/JwHI nXyUJ8WcFtajnoNUB5g5zDi2xj0kT3mG5EPJOyc3QdSLrhrd2YhrAm3EOQ== X-Gm-Gg: ASbGncsfK5iUZZdoHTCPEQpWGlJKdwCZxXOR1qD1u+SXAS+kDq6JGFhNn1qHiGUTP0O J8oYy5NqjPv1JQPtnwgXvav2XrW/N4fVqOVWnPHbwj6yVqtZDEzF1hRvUzRJT/rFm8eTGAzTdx+ 9Am+irZkJAziXqE1slvqLOSucrBm4EEBCuuUmuM9SJrZ+tA7MphGB8PGPIDdclJF2q9NQIWRGjR 3W7YRL0NRDO4HuZYcamlE/+c2yPcUA/tBhCs0FL1Z+oeg== X-Google-Smtp-Source: AGHT+IEhcBaUe4eH3/LFTijflHIiP6LHdpduSHcojhZZbGAyMhjDYtJlS3z6BQmeSSJcJcsq2ZelGw== X-Received: by 2002:a05:6830:6281:b0:718:9d91:6a0f with SMTP id 46e09a7af769-71ab31a6a07mr2505326a34.15.1732103512169; Wed, 20 Nov 2024 03:51:52 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7f8c1c2c505sm9251170a12.18.2024.11.20.03.51.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:51:51 -0800 (PST) Date: Wed, 20 Nov 2024 19:52:00 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 7/9] ref: add basic 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 code that checks regular ref contents, but we do not yet check the contents of symbolic refs. By using "parse_loose_ref_content" for symbolic refs, we will get the information of the "referent". We do not need to check the "referent" by opening the file. This is because if "referent" exists in the file system, we will eventually check its correctness by inspecting every file in the "refs" directory. If the "referent" does not exist in the filesystem, this is OK as it is seen as the dangling symref. So we just need to check the "referent" string content. A regular ref could be 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. However, we always write a single SP after "ref:" and a single LF after the refname. It may seem that we should report a fsck error message when the "referent" does not apply above rules and we should not be so aggressive because third-party reimplementations of Git may have taken advantage of the looser syntax. Put it more specific, we accept the following contents: 1. "ref: refs/heads/master " 2. "ref: refs/heads/master \n \n" 3. "ref: refs/heads/master\n\n" When introducing the regular ref content checks, we created two fsck infos "refMissingNewline" and "trailingRefContent" which exactly represents above situations. So we will reuse these two fsck messages to write checks to info the user about these situations. 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 by using "is_root_ref" and "check_refname_format" to check the "referent". Since both "is_root_ref" and "check_refname_format" don't work with whitespaces, we use the trimmed version of "referent" with these functions. In order to add checks, we will do the following things: 1. Record the untrimmed length "orig_len" and untrimmed last byte "orig_last_byte". 2. Use "strbuf_rtrim" to trim the whitespaces or newlines to make sure "is_root_ref" and "check_refname_format" won't be failed by them. 3. Use "orig_len" and "orig_last_byte" to check whether the "referent" misses '\n' at the end or it has trailing whitespaces or newlines. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 + fsck.h | 1 + refs/files-backend.c | 40 ++++++++++++ t/t0602-reffiles-fsck.sh | 111 ++++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 6db0eaa84a..dcea05edfc 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -28,6 +28,9 @@ `badRefName`:: (ERROR) A ref has an invalid format. +`badReferentName`:: + (ERROR) The referent name of a symref is invalid. + `badTagName`:: (INFO) A tag has an invalid format. diff --git a/fsck.h b/fsck.h index b85072df57..5227dfdef2 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_REFERENT_NAME, 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 3d4d612420..f4342e3f3e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3509,6 +3509,43 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refname, struct dir_iterator *iter); +static int files_fsck_symref_target(struct fsck_options *o, + struct fsck_ref_report *report, + struct strbuf *referent) +{ + char orig_last_byte; + size_t orig_len; + int ret = 0; + + orig_len = referent->len; + orig_last_byte = referent->buf[orig_len - 1]; + strbuf_rtrim(referent); + + if (!is_root_ref(referent->buf) && + check_refname_format(referent->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_REFERENT_NAME, + "points to invalid refname '%s'", referent->buf); + goto out; + } + + if (referent->len == orig_len || + (referent->len < orig_len && orig_last_byte != '\n')) { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "misses LF at the end"); + } + + if (referent->len != orig_len && referent->len != orig_len - 1) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "has trailing whitespaces or newlines"); + } + +out: + return ret; +} + static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_options *o, const char *target_name, @@ -3563,6 +3600,9 @@ static int files_fsck_refs_content(struct ref_store *ref_store, "has trailing garbage: '%s'", trailing); goto cleanup; } + } else { + ret = files_fsck_symref_target(o, &report, &referent); + goto cleanup; } cleanup: diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 33e7a390ad..ee1e5f2864 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -263,6 +263,109 @@ 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 && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" + do + printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && + + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline && + 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: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + 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: has trailing whitespaces or newlines + 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: has trailing whitespaces or newlines + 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: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-complicated && + 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: HEAD\n" >$branch_dir_prefix/branch-head && + 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 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && @@ -313,6 +416,14 @@ test_expect_success 'ref content checks should work with worktrees' ' warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end EOF rm $worktree1_refdir_prefix/branch-no-newline && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' + EOF + rm $worktree1_refdir_prefix/branch-garbage && test_cmp expect err ' From patchwork Wed Nov 20 11:52:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881088 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.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 0A41119E7D0 for ; Wed, 20 Nov 2024 11:52:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.171 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103523; cv=none; b=OTfOYSOsimwjMzB5eGjsbT08mt1t/YmRMaIbpjhsH6CxQjT2uVQeb5iiiySDYVNuC/gdWtWY5W3ZaOCXxYWwOxFYkVs21UzsxH/0fuPrwRpp/dI/uIbfkQmMznfomkLGV5Rv8m1L4Uemz7OhJBb4pQBrjJrMD2cSe34fqxhpwoI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103523; c=relaxed/simple; bh=4DCT3wjkdpMOZNpjLJ+Ud/P1fD2xWBYjJZf0Kt6sLJs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=YNoIYo/jPYBxbGcprWyZUQTvvt/sgZUGMDWPmB2iYpzlOH0nGhRZT3tOp6ntdftJ3ToU9VIY8Cq2OGzXQ0IDb+YWIt1HSrzT6GnGxiisu4U+G90yG7tCSsL+Ouo+sX3cozOf0RoJD/D33//JV9JHqsdR2ExDpWFOGL68DmSW328= 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=QjdMjGEl; arc=none smtp.client-ip=209.85.214.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="QjdMjGEl" Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-21270d64faeso3112525ad.1 for ; Wed, 20 Nov 2024 03:52:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103520; x=1732708320; 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=TiqeCFZDqhipWUnEYmgeXzyRKYNgnuZZpfW5Y2b78gg=; b=QjdMjGEl7mN8vE5/0WxIRhA0AzZqDfTT2/zV4hbJmu65M5eE5gKZE38eXKhXf+Hcnn 8hfG3V+GDrnLfVMihYBAQ6vK3r5ld1MBUpe2L10Ly56v3NGWCErlqej7MS/WNj0Qs3L2 Nukii8fRututT1pX9BMasMWTeD6XHKNSrFNIyNTqq/WTSnshWXdXnaYlXuoUWmOHkyVH DilT0AndCZxRduxGgLpRdS8kMKRvVeVjAo9rXtcVSRf+PDI3MSELdXX90ju70xug3Jb9 wan7eyAAaFMXF4AkLbL/B6xdsyWZVbSsWtzmow+NhDKhbp/V3uLk+GK0DQy/olJ7i1VD EW5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103520; x=1732708320; 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=TiqeCFZDqhipWUnEYmgeXzyRKYNgnuZZpfW5Y2b78gg=; b=HEuC0DCk0460e2tCui19ibTrQWo1wa9EpVDhi4pIhQL32CyvVL6hodRpKzVMmBBnJH TOTdScEimHpXYGdY1V4IHXqnhqHo8maE+UcGK+90zUYv+tXWAmTKWHWZwXmq+1EdHFmZ aOxu+cN8d2wX59XGU9BXdeN+HQKHi8uL41usACy3DT4EwdRwuk63qHnhsIAjzbsdtW/l RAy75RekPLtSJulyK0Yp2HQXcXpk444WLKOgj06Zp9lz1iFg4wbtFEGfem2OaWZuuqTx xeaEtzK9gsW7V9X3U6YZ8jvTgoVkdGE/jeDRX2VfOcYj2+S6y7SqT0x5pirQeQffA8KC KMfw== X-Gm-Message-State: AOJu0Yxtog8kvqtmJUmuj+22bfj3OazgNam7nTx97rMhmUi6LJKJWt9V dErTeoZgRz0//vS0BgsvqzNaQFqnBFIjL1rOFk1IqpmVrPDOOGgR9U5aKA== X-Google-Smtp-Source: AGHT+IFsNYwuZKnfnbUDBD74cWR1VNXWQRwwhfy3v68Ur2i8K/+KVpLLfej+TD1nNC7iZPMjhQ/wtg== X-Received: by 2002:a17:902:f705:b0:211:ebd:e370 with SMTP id d9443c01a7336-2126aea6566mr26362095ad.25.1732103520512; Wed, 20 Nov 2024 03:52:00 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2120ac38dbasm61054785ad.60.2024.11.20.03.51.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:52:00 -0800 (PST) Date: Wed, 20 Nov 2024 19:52:09 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 8/9] ref: check whether the target of the symref is a ref 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: Ideally, we want to the users use "git symbolic-ref" to create symrefs instead of writing raw contents into the filesystem. However, "git symbolic-ref" is strict with the refname but not strict with the referent. For example, we can make the "referent" located at the "$(gitdir)/logs/aaa" and manually write the content into this where we can still successfully parse this symref by using "git rev-parse". $ git init repo && cd repo && git commit --allow-empty -mx $ git symbolic-ref refs/heads/test logs/aaa $ echo $(git rev-parse HEAD) > .git/logs/aaa $ git rev-parse test We may need to add some restrictions for "referent" parameter when using "git symbolic-ref" to create symrefs because ideally all the nonpseudo-refs should be located under the "refs" directory and we may tighten this in the future. In order to tell the user we may tighten the above situation, create a new fsck message "symrefTargetIsNotARef" to notify the user that this may become an error in the future. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 9 +++++++++ fsck.h | 1 + refs/files-backend.c | 14 ++++++++++++-- t/t0602-reffiles-fsck.sh | 29 +++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index dcea05edfc..f82ebc58e8 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -183,6 +183,15 @@ git@vger.kernel.org mailing list if you see this error, as we need to know what tools created such a file. +`symrefTargetIsNotARef`:: + (INFO) The target of a symbolic reference points neither to + a root reference nor to a reference starting with "refs/". + Although we allow create a symref pointing to the referent which + is outside the "ref" by using `git symbolic-ref`, we may tighten + the rule in the future. Report to the git@vger.kernel.org + mailing list if you see this error, as we need to know what tools + created such a file. + `trailingRefContent`:: (INFO) A loose ref has trailing content. As valid implementations of Git never created such a loose ref file, it may become an diff --git a/fsck.h b/fsck.h index 5227dfdef2..53a47612e6 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(SYMREF_TARGET_IS_NOT_A_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 f4342e3f3e..c2b99fdf40 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3513,6 +3513,7 @@ static int files_fsck_symref_target(struct fsck_options *o, struct fsck_ref_report *report, struct strbuf *referent) { + int is_referent_root; char orig_last_byte; size_t orig_len; int ret = 0; @@ -3521,8 +3522,17 @@ static int files_fsck_symref_target(struct fsck_options *o, orig_last_byte = referent->buf[orig_len - 1]; strbuf_rtrim(referent); - if (!is_root_ref(referent->buf) && - check_refname_format(referent->buf, 0)) { + is_referent_root = is_root_ref(referent->buf); + if (!is_referent_root && + !starts_with(referent->buf, "refs/") && + !starts_with(referent->buf, "worktrees/")) { + ret = fsck_report_ref(o, report, + FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF, + "points to non-ref target '%s'", referent->buf); + + } + + if (!is_referent_root && check_refname_format(referent->buf, 0)) { ret = fsck_report_ref(o, report, FSCK_MSG_BAD_REFERENT_NAME, "points to invalid refname '%s'", referent->buf); diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index ee1e5f2864..692b30727a 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -366,6 +366,35 @@ test_expect_success 'textual symref content should be checked (aggregate)' ' test_cmp expect sorted_err ' +test_expect_success 'the target of the textual symref should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" + do + printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err || return 1 + done +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && From patchwork Wed Nov 20 11:52:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13881089 Received: from mail-pg1-f174.google.com (mail-pg1-f174.google.com [209.85.215.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 428951A01D4 for ; Wed, 20 Nov 2024 11:52:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103532; cv=none; b=ha4sfI13dB+XTAO5MZTZVo3o4D2Fx5BSk3EBQ/NLPsWVo3y2nFW3/TFBGr+2mu2tAfMh+I0jGFqVeiABku8nFzgDRkMqIHUYNynkfiRena0tGs50eZgIEMvnnFII5DhaI++cyNcfAlpDxQOxejXxsRqsAK4jk5xb2iWU20HBAdo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1732103532; c=relaxed/simple; bh=Ii92LAF0JIPuAo4gZX3jRN8De3iey6UerhVehKXOAx0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=leR5cCh/tgXY0LNf+GZIzGKaC0IHf2tIqty8zA5Nh1kRWZpbAAl8chlk2cLz7UOEnLwFuoLFE6WcTyjUb2+ZhiPqM0TGOgR7bTdd6fUPvOO0s5J8Ix4atPU8nRusgy7hzqPZLSI9MjSOeMIkyH1E0NpwEz0q8dmnZ6CJ12UtJi4= 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=ePKEBhqX; arc=none smtp.client-ip=209.85.215.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ePKEBhqX" Received: by mail-pg1-f174.google.com with SMTP id 41be03b00d2f7-7edb6879196so1473133a12.3 for ; Wed, 20 Nov 2024 03:52:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732103529; x=1732708329; 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=GeWHLfS53DVdQjI9rauCGMiEsC9cKzziMR6yFX8GjjA=; b=ePKEBhqXbb5/0iK8rFTdyW6FIXCerd/jm+2m5O3bcbWsi2hqHgsYNdq0ATtHLJrdSK 07FAr2VHhH7GCGc2oVWo3nQfb2xNuAt5H6Ik+ddcG7flWKCWfy2hGycj7xD7By10xmML P04lKXzGUVrOwwFweVNdxHlm8IyvmmJuIUhUGtmrNlRYoX7IgiAJpdci7HqSGQqSe345 9hZZ8OlE72XyNbbXjvJ8FJ7xsjjexCWU4Jh3MiOb9Gj/7KqZ7Ro7G3VGmKXOvu9XViZh b0E81YeTPEJY0yuJoXNxWvEg/feoaOGxw6dHT9lPvJXv4voRgk0rbo4GS7LIZOAq4Eog hluw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732103529; x=1732708329; 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=GeWHLfS53DVdQjI9rauCGMiEsC9cKzziMR6yFX8GjjA=; b=xPBAgCc4VZTB0inhkBKxWv9Wy6X9seAJZRKUAqZJvhRz4y8jtPjl6PvNq6iJaqRCiZ F1ZabAGbZj5elogYD8HkhQ3PrvUmhGDCTiqIaiIDJiyH/J95NGsDDZMmlkosEHVykVYX ColRDOynY9MjZb1CEo+VIOz67cCOHT+6/WYCo9cxV3d/rQlGWpe3Gy6HqW3j2FaMGUhD lqiLxQW78ov0bGra9ypEaOVj/62drsbDX0iaqJBAXKylj6nZ+gKrcMCUXQKY0GjgCxtB Wnx98O5cYbVTrC9EJpw04D9fCYStJenAnFkJ/ShQApGcERJSeJJclS+720qSol8ViSYp 01gQ== X-Gm-Message-State: AOJu0Ywhu0G3VCUKg0ru6ORjkrYYiVvZGpV0aE8VVmHsFbqJ01xQYQdB R3lZGb2eixu6QTItIF+2xNn44rFipq39WEKQS8jwaN6mMVYEImaYkr3rAA== X-Google-Smtp-Source: AGHT+IHJ5yRNDopre90JNrLqKzoAHTi+35+9ASYf+rhROSuf2KGmX48AHzhRc8Z8kAiQzsPwwz6L7g== X-Received: by 2002:a05:6a20:12c6:b0:1db:ed4d:fa90 with SMTP id adf61e73a8af0-1ddae3eb113mr3408481637.10.1732103529040; Wed, 20 Nov 2024 03:52:09 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-724befeaa4dsm1414956b3a.200.2024.11.20.03.52.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 03:52:08 -0800 (PST) Date: Wed, 20 Nov 2024 19:52:18 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v9 9/9] 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: Besides the textual symref, we also allow symbolic links as the symref. So, we should also provide the consistency check as what we have done for textual symref. And also we consider deprecating writing the symbolic links. We first need to access whether symbolic links still be used. So, add a new fsck message "symlinkRef(INFO)" to tell the user be aware of this information. 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. And we need to also generate the "referent" parameter for reusing "files_fsck_symref_target" by the following steps: 1. Use "strbuf_add_real_path" to resolve the symlink and get the absolute path "ref_content" which the symlink ref points to. 2. Generate the absolute path "abs_gitdir" of "gitdir" and combine "ref_content" and "abs_gitdir" to extract the relative path "relative_referent_path". 3. If "ref_content" is outside of "gitdir", we just set "referent" with "ref_content". Instead, we set "referent" with "relative_referent_path". Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 6 ++ fsck.h | 1 + refs/files-backend.c | 38 ++++++++- t/t0602-reffiles-fsck.sh | 141 ++++++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 4 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index f82ebc58e8..b14bc44ca4 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -183,6 +183,12 @@ git@vger.kernel.org mailing list if you see this error, as we need to know what tools created such a file. +`symlinkRef`:: + (INFO) A symbolic link is used as a symref. Report to the + git@vger.kernel.org mailing list if you see this error, as we + are assessing the feasibility of dropping the support to drop + creating symbolic links as symrefs. + `symrefTargetIsNotARef`:: (INFO) The target of a symbolic reference points neither to a root reference nor to a reference starting with "refs/". diff --git a/fsck.h b/fsck.h index 53a47612e6..a44c231a5f 100644 --- a/fsck.h +++ b/fsck.h @@ -86,6 +86,7 @@ enum fsck_msg_type { FUNC(MAILMAP_SYMLINK, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ + FUNC(SYMLINK_REF, INFO) \ FUNC(REF_MISSING_NEWLINE, INFO) \ FUNC(SYMREF_TARGET_IS_NOT_A_REF, INFO) \ FUNC(TRAILING_REF_CONTENT, INFO) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index c2b99fdf40..ea5961e48c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1,6 +1,7 @@ #define USE_THE_REPOSITORY_VARIABLE #include "../git-compat-util.h" +#include "../abspath.h" #include "../config.h" #include "../copy.h" #include "../environment.h" @@ -3511,7 +3512,8 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, static int files_fsck_symref_target(struct fsck_options *o, struct fsck_ref_report *report, - struct strbuf *referent) + struct strbuf *referent, + unsigned int symbolic_link) { int is_referent_root; char orig_last_byte; @@ -3520,7 +3522,8 @@ static int files_fsck_symref_target(struct fsck_options *o, orig_len = referent->len; orig_last_byte = referent->buf[orig_len - 1]; - strbuf_rtrim(referent); + if (!symbolic_link) + strbuf_rtrim(referent); is_referent_root = is_root_ref(referent->buf); if (!is_referent_root && @@ -3539,6 +3542,9 @@ static int files_fsck_symref_target(struct fsck_options *o, goto out; } + if (symbolic_link) + goto out; + if (referent->len == orig_len || (referent->len < orig_len && orig_last_byte != '\n')) { ret = fsck_report_ref(o, report, @@ -3562,6 +3568,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, struct dir_iterator *iter) { struct strbuf ref_content = STRBUF_INIT; + struct strbuf abs_gitdir = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct fsck_ref_report report = { 0 }; const char *trailing = NULL; @@ -3572,8 +3579,30 @@ static int files_fsck_refs_content(struct ref_store *ref_store, report.path = target_name; - if (S_ISLNK(iter->st.st_mode)) + if (S_ISLNK(iter->st.st_mode)) { + const char *relative_referent_path = NULL; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_SYMLINK_REF, + "use deprecated symbolic link for symref"); + + strbuf_add_absolute_path(&abs_gitdir, ref_store->repo->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(&ref_content, iter->path.buf); + skip_prefix(ref_content.buf, abs_gitdir.buf, + &relative_referent_path); + + if (relative_referent_path) + strbuf_addstr(&referent, relative_referent_path); + else + strbuf_addbuf(&referent, &ref_content); + + ret |= files_fsck_symref_target(o, &report, &referent, 1); goto cleanup; + } if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { /* @@ -3611,13 +3640,14 @@ static int files_fsck_refs_content(struct ref_store *ref_store, goto cleanup; } } else { - ret = files_fsck_symref_target(o, &report, &referent); + ret = files_fsck_symref_target(o, &report, &referent, 0); goto cleanup; } cleanup: strbuf_release(&ref_content); strbuf_release(&referent); + strbuf_release(&abs_gitdir); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 692b30727a..f8f27cfc6c 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -395,6 +395,147 @@ test_expect_success 'the target of the textual symref should be checked' ' done ' +test_expect_success SYMLINKS 'symlink symref content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + 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 ../../logs/branch-escape $branch_dir_prefix/branch-symbolic && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./"branch " $branch_dir_prefix/branch-symbolic-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref + error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch '\'' + EOF + rm $branch_dir_prefix/branch-symbolic-bad && + 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 invalid refname '\''refs/tags/.tag'\'' + EOF + rm $tag_dir_prefix/tag-symbolic-1 && + test_cmp expect err +' + +test_expect_success SYMLINKS 'symlink symref content should be checked (worktree)' ' + test_when_finished "rm -rf repo" && + git init repo && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + main_worktree_refdir_prefix=.git/refs/heads && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $worktree1_refdir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $worktree2_refdir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_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 $main_worktree_refdir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' + EOF + rm $worktree1_refdir_prefix/branch-symbolic && + test_cmp expect err && + + for bad_referent_name in ".tag" "branch " + do + ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err || return 1 + done +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo &&