From patchwork Mon Oct 21 13:34:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844153 Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) (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 33FDE1E5705 for ; Mon, 21 Oct 2024 13:34:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.43 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517654; cv=none; b=UcR1md1vLPzZ1a83b1d4hZGE/9g9D4As+o5Tw2Bk56smStOAmP1ulYq+buUG0Rac5mY0BAw4Cvx6+VtgH8aGz44FpRjXtJHR+zE9PP3r/uwVUfB9mBlZHjsWQ3gSocbEG+tdCx1DDIJS55pQ+LuucmC0qLdLB5zDBQT3Ze5Vnwk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517654; c=relaxed/simple; bh=Iq240slf6F8USCuNlFVvJmIT6fjkny4AKF4dW34UQj8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=aETogHumILefYh2UggeDzX10bolwBnNtBtlgKO0OKVAWBnXaiSOfqfIGUrSNBNnArtRgmE0d2EJz0Y3VBLWKC5JK5+CQznpLxoLuS7WwBAc3+Jmd0s5hsPTA2WgbnwS6g232cqWQJRgWo/s2qNEAfUu3ih18AEPfhzsIEixldGY= 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=b2bU/jqF; arc=none smtp.client-ip=209.85.216.43 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="b2bU/jqF" Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-2e2e88cb0bbso3251622a91.3 for ; Mon, 21 Oct 2024 06:34:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517651; x=1730122451; 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=b2bU/jqFf2iymXfWvqw7XDiDtEjazzMPovAsB2A0mFnxL6LoCQQTfVakvO9QT1CNLi bIMuuNhEJiRYgvpU31GHzfiT3fkTHdAD8R5A0M27mY+ShjkyyfnDvMUvN7f2FjZpmkMs n4hDFeRck+Cd7Vqp9TFR/f6uyoHme1xgvmeyfrDrhIsuZV0IJLyhMsKnQfKClZf66sP+ P+LJ+6vQqQHBKIi0kHRQJHPAYpp4dm53XR8vLs1JuEHu0LmzhuE2oTCN3jePmilPMOfV 2fP+sVLQwYVrvyRlgZqitoIxm+NrR/+krQL0733qCnnbreUhEUu6m/JLdtx4LLbop0ZW h0IQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517651; x=1730122451; 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=YDGWZKjIbSNhEMwlAJoiGJwgnjoNIOOJQy0zKAe1TVv+EevNDLSU1W2zltBtmx8EhN CM9HUg2+fwobE9/SQAbMvIbc0ptTrNEi7AHqxZmElYye/rKITbZsu5lJ9fUmzRgzOTST oDYgkmXnNR7qhhQVQeeRyqJOpznH8NZ1+6UT6bVnS+MApZD9NE9Sh3uj1AJf5rzVZgBI OebY4xCPwAoC6gfw+sT/JWeIWAeruy37tWJ7aIgTAGPa0kezk0NqraD8WfjuteKl6LZ4 5/llW43d7mNqOyrrhyooZkh+lrbaTAajekhgFbogT7ZL7OiWcMceMAGt1jFgXopjxXaF hhTw== X-Gm-Message-State: AOJu0Yz0YfzpkS9v2UA3GilZ3w83kJ9t79AG760MWVauBoOOGxMecM7s Zku3ZRVIWJHiw8YxY18qsceMyzwNX8tdOfM89RfFTvyzw5OOe/jlroaKHkWq X-Google-Smtp-Source: AGHT+IGdyW2bZBzUnCAwH5R+nFf9gEsSIIY1eHwCPi1YIb+kmmWTAhx5y/eNLE9tkb3OF7NM3wnUKA== X-Received: by 2002:a17:90b:4b0c:b0:2db:89f0:99a3 with SMTP id 98e67ed59e1d1-2e5616e6f9cmr12408301a91.26.1729517650861; Mon, 21 Oct 2024 06:34:10 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2e5ad4ed127sm3727306a91.42.2024.10.21.06.34.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:34:10 -0700 (PDT) Date: Mon, 21 Oct 2024 21:34:13 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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 Mon Oct 21 13:34:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844154 Received: from mail-pg1-f177.google.com (mail-pg1-f177.google.com [209.85.215.177]) (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 A90951F8F04 for ; Mon, 21 Oct 2024 13:34:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517662; cv=none; b=KATgX9xtw8WnZW08JaEVT4GGBJP9nej72fxFTAXc6AxO/OZIuvgIUQdDv4li+lTIM+pKjCsW5xUPwaRHzXNM7eHXew7h8fqxDRQDcYuac4HUr8i0VmI1DFSr5PZd10JKZaomqvT4iy58UEmhNJ6IidCy1ElwxufgR+0ZaoqoaME= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517662; c=relaxed/simple; bh=1KOcCGoj/mE52PBsfVdWF/eoGNTmQzd4/kD5fYxFbhg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=qcGXUzVePcIrGnPpu+JwxKiFvkOGouO1l5FNnXq55aj+PzVfs3W69em/sjY1PEFHzUWDAlY8hLI7aGFBzubqR+UpB0L5yJXnka2DaEfEADu8V+84j60tGW6+6a5xKYP7oN/JCwoyMZ8QSh3D3rmNlfIIi9EryrjB/AYQzJZIEDs= 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=kxpciIW+; arc=none smtp.client-ip=209.85.215.177 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="kxpciIW+" Received: by mail-pg1-f177.google.com with SMTP id 41be03b00d2f7-7ea76a12c32so3654291a12.1 for ; Mon, 21 Oct 2024 06:34:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517659; x=1730122459; 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=cOeiroyRx3P8gRdL/jdaG+9kuj82WbmB1b9QRImAsF8=; b=kxpciIW+BXMBy4cC9SB0ckeZgxRRGmZg8cx/Aml1PDxvnQ+NO9i2/SjdgZXAiSqY2p GknYdxYum27ZhJzfXLrlt56W0QYjB8JQC8PSP9xqgqoPBU8XzMtNDXVKFlsGFQqZrXlm OgzCmkk7REKe+E2Q+v8jDHyYH26MRun3v1tUjnfdHqxEHgG3evT0xijcoyQzqOyJ9wOu KyxUwYVy6zinZrVk0LehR7bAPm6Fw132h0pMn3cLeol9i5NcF02hjp0sigpcosFL9O6w XQMM5X/7jN8szjv8uZXv7XphURhkXlX1B6HHyamsN1R0Yc+l39xplV6h8ANSVKstEGpH /djg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517659; x=1730122459; 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=cOeiroyRx3P8gRdL/jdaG+9kuj82WbmB1b9QRImAsF8=; b=CR0KtP9MMpcqus+aHspxi0EgKdGII/s+3Nzfhh66FtvaxA7pfB+bCG4qwQpBNcdBh0 BCPBHXKYNW274uxdHAtMnVua3Ewwrx+rEOxBbAv6GtCAkMSoiytGkh2IXTEtBDzbm7ms GMEP4AOBVvdbJro01KRj2W6Es9843NqGnuVWJOQ8A5uRO9GtSSefigBu9iCMFsSpWjUW mUH8AK0jhUHTjw1W3+S7dfbmoxdCI9jasc5/F/X5eWcHLuiv8CcFNuZia9NuSoBahpRA 9OGVC2laO8q+xvRlpZg3gEIJA52MH6u52Uzuri9UbWJDMyfHNIhuEEia+1ko2fXKqwRk ZSuA== X-Gm-Message-State: AOJu0YySAzQFvOVmRVAqXuFZ+ywKVATmhmobw/kZVjV9byR8+fjVfk00 m7o66PUYQ/OhETXBHXqf0z9HLX7nLZ/g9Esm162tYoUFzmyCutCSc9zOSmLp X-Google-Smtp-Source: AGHT+IF4wMiNMNKALOckbbrzG1MLa32ipKGFZ670rG/PTp+NGOg5K18woLNYR21EozlQyvVUIu586w== X-Received: by 2002:a05:6a21:3a94:b0:1d9:2335:a891 with SMTP id adf61e73a8af0-1d92c5755a0mr15449637637.36.1729517659391; Mon, 21 Oct 2024 06:34:19 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71ec135663asm2847770b3a.86.2024.10.21.06.34.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:34:18 -0700 (PDT) Date: Mon, 21 Oct 2024 21:34:22 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 4 ++-- t/t0602-reffiles-fsck.sh | 30 +++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 03d2503276..f246c92684 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3519,10 +3519,10 @@ 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)) { + 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..0aee377439 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -25,6 +25,13 @@ test_expect_success 'ref name should be checked' ' git tag tag-2 && git tag multi_hierarchy/tag-2 && + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ && + git refs verify 2>err && + cat >expect <<-EOF && + EOF + test_must_be_empty err && + rm $branch_dir_prefix/@ && + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && test_must_fail git refs verify 2>err && cat >expect <<-EOF && @@ -33,20 +40,20 @@ test_expect_success 'ref name should be checked' ' 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'\'' && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/@: badRefName: invalid refname format + error: refs/heads/ branch-1: badRefName: invalid refname format EOF - rm $branch_dir_prefix/@ && + rm $branch_dir_prefix/'\'' branch-1'\'' && test_cmp expect err && - cp $tag_dir_prefix/multi_hierarchy/tag-2 $tag_dir_prefix/multi_hierarchy/@ && + cp $tag_dir_prefix/multi_hierarchy/tag-2 $tag_dir_prefix/multi_hierarchy/'\''~tag-2'\'' && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: refs/tags/multi_hierarchy/@: badRefName: invalid refname format + error: refs/tags/multi_hierarchy/~tag-2: badRefName: invalid refname format EOF - rm $tag_dir_prefix/multi_hierarchy/@ && + rm $tag_dir_prefix/multi_hierarchy/'\''~tag-2'\'' && test_cmp expect err && cp $tag_dir_prefix/tag-1 $tag_dir_prefix/tag-1.lock && @@ -60,6 +67,15 @@ test_expect_success 'ref name should be checked' ' error: refs/tags/.lock: badRefName: invalid refname format EOF rm $tag_dir_prefix/.lock && + test_cmp expect err && + + mkdir $tag_dir_prefix/'\''~new-feature'\'' && + cp $tag_dir_prefix/tag-1 $tag_dir_prefix/'\''~new-feature'\''/tag-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/~new-feature/tag-1: badRefName: invalid refname format + EOF + rm -rf $tag_dir_prefix/'\''~new-feature'\'' && test_cmp expect err ' @@ -84,7 +100,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 Mon Oct 21 13:34:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844155 Received: from mail-pf1-f180.google.com (mail-pf1-f180.google.com [209.85.210.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 08CAE1F4726 for ; Mon, 21 Oct 2024 13:34:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.180 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517670; cv=none; b=Vi4Aw6T/4bjwkaZDO5My9P/FoMV/SDbSVy6XLRY0EzOdC9RNifz1CZsHI2/xBA8fmKg7WIcoNAM7JNlvzeWbVxSEB73dUOjBkqqF9uZ9s1y7o2ryiw6WS3z19cnn8YVJQFWhURzG/+xWqQ3lKdh6Jd5BdF8qkMoB4ToCHUYBIOA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517670; c=relaxed/simple; bh=wMNCWf6s29KpHZcxEWiYo/DoNvHpeTy11CEa+EXGFCM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=O6wGNDwL5QruCy9A2OOB+WoRrr9DMqIDl2tEm8xor/jSTWcDOE6626P5KndEVDmHAfukBDd3LATjTupVJRVzdPw8WGol0aCy0zY0ZYz4BgjckJ7wKEtZSnnXIJBezLeWR1jjuplqn3PGe70BXP1+NYAJ0UnXtQxklnOcWCor6MM= 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=JKo83tir; arc=none smtp.client-ip=209.85.210.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="JKo83tir" Received: by mail-pf1-f180.google.com with SMTP id d2e1a72fcca58-71ea2643545so2679594b3a.3 for ; Mon, 21 Oct 2024 06:34:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517668; x=1730122468; 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=UcrIzPe6eAmshJw/eCNKdp+NDct1XU910ErxjPGCOGY=; b=JKo83tirZIe5IOb9IY2mRyKSKtC82n32th3htK/wNAY5O8Vwv6KFHD6dR2mm3TRyDk xSAlnzVXGsDT0E5xvPqqRG6VxA1UTbGjs8dtBzZ+QVdy216CJypTJ0iXr6ZSIQbdjqiF n81O/XcYEGzDiYUdBCKZKB1pAos/6Sm/ETiO2VZmVkf91MGtLpJx0GsF4uCCgYVf+TxB xvxxzKXRVlvbaM2fLsK0VPj5XVMwkkjNOZcUizLkqHJEr7xVSB0rGwDUqh+LmxK+Xof+ 3cZL0N97tT1Tzr3EhHZq7811V4/fW3/XnNAQTRWneHe00NB1BthxDJ49jYAXrG/lj4dT Y88A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517668; x=1730122468; 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=UcrIzPe6eAmshJw/eCNKdp+NDct1XU910ErxjPGCOGY=; b=mpmKYLA7PP5DIKd70y+NX9VcHqPETmQrWB6cUHtu/BJWA1LtRSuQx8j2MArBpvSimx ukmfN3WRZJNc73azbvMKbxf1Nm8rqUFpUFLmf/97SE8xez/89WYa3b3NGB7frdjthr6R YPgpD7xveUiaMu9S5A6xrwbmDNhpideELGyF4jpPFactR6pcos4bumLvc8+YT50n2KUj swjmT4mSRvzVQc8G6u0gFZLdUxYwoLjpE33XWd+WpETvKyzWgTspNRDdckKDghEJM04t kSckMOBEAWezK2JDKxtN84Hehn0BgBtf2/uox2WWAt4lYeHHLmrlCUJyr0Uw0kdVfaLq TOIw== X-Gm-Message-State: AOJu0YzhT3tIzicbJtH+ezSZwXLz0m8wc4fnMXBNgaY21sRssDFFrWuJ 1bLI8MdNtOg5IYT+Wi/48fwkNteEUfz3TPXToIh3RF0tAPrOSiYKQJ8DgRtA X-Google-Smtp-Source: AGHT+IFKpQlegprdqlr3OvCR9CgpJydbz2d13AqiB54lMjyOYUmPNCm6MuO9yCGT3dzAQxfjsoPhUw== X-Received: by 2002:a05:6300:4044:b0:1d8:d600:2c6b with SMTP id adf61e73a8af0-1d92c4a1e9amr14992625637.3.1729517667805; Mon, 21 Oct 2024 06:34:27 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71ec131399esm2947300b3a.13.2024.10.21.06.34.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:34:27 -0700 (PDT) Date: Mon, 21 Oct 2024 21:34:31 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 3/9] ref: initialize target 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 re-calculate the target name. It's bad for us to do repeat calculation. Instead, we should calculate it only once and pass the target name to the check functions. In order not to do repeat calculation, rename "refs_check_dir" to "target_name". And in "files_fsck_refs_dir", create a new strbuf "target_name", thus whenever we handle a new target, 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 f246c92684..fbfcd1115c 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 *target_name, 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 *target_name, struct dir_iterator *iter) { struct strbuf sb = STRBUF_INIT; @@ -3519,11 +3519,10 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock")) goto cleanup; - strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); - if (check_refname_format(sb.buf, 0)) { + if (check_refname_format(target_name, 0)) { struct fsck_ref_report report = { 0 }; - report.path = sb.buf; + report.path = target_name; ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_NAME, "invalid refname format"); @@ -3539,6 +3538,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 target_name = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; struct dir_iterator *iter; int iter_status; @@ -3557,11 +3557,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(&target_name); + strbuf_addf(&target_name, "%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", target_name.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, target_name.buf, iter)) ret = -1; } } else { @@ -3578,6 +3582,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, out: strbuf_release(&sb); + strbuf_release(&target_name); return ret; } From patchwork Mon Oct 21 13:34:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844156 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 875101EB9F2 for ; Mon, 21 Oct 2024 13:34:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517680; cv=none; b=ONAUtsau071f42WUyovSmBGN27WzEKbfjL9AnDgUlNB/Bvvusfn0E0mY/6y80vTXSGhhWOqMSp492MqCYlXwAGF0YdACwVKDGnO8+mL2PHnkJllqA6AniHiN48Rljn2Jv0qz++hRBN/SXh3sOrjuZ83nOAjYG9ne6nvtmMQ+d/A= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517680; c=relaxed/simple; bh=ZE2KSJw49THT5RdcbGkKV5fEEA4fm87U9afV3dsW+G0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Y48/ITIW7l2SU+G2jnOGeflrnLNg6mKDUH2TPu2NHy35FEUj+jbKYLPUZ0mDG0UG6evCXx8IB9zzcHyda2lkt0twrblNzjZ5HoHPSxib9vPRLNHaMdkhpYiHr1V9PjZizIh7uwkhVY55ejSAIbia3KQZMKQcUbdjhFrQzx4hADI= 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=fcn3vn/W; arc=none smtp.client-ip=209.85.214.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fcn3vn/W" Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-20cb89a4e4cso30535395ad.3 for ; Mon, 21 Oct 2024 06:34:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517677; x=1730122477; 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=ZE3kkVNGtxIiVPEnCVFwFowPumHB1qlzjMLlu4ndFCI=; b=fcn3vn/WDcmUZ3BQMhG3pNQ0v5Q2Z1R2UfWEVjrOsE4QrkiOa472ZIoBP9w5HfPh2D kNKw1yTWFACKu7Y1HxJuogvQn7NaQgzVGonFSpLG6oWPVX0PLGcXqFppxEUO1OECKwpc Ijy+YbCB1Zqn2mi9jX5XPZh1B5/QLNlMJJVMf8QsUyxJYvVD3Z0p9on7Q9/raC23Gjvb hGxf8bB8kKCV1uA6YZNVr3rKp74Qjwhkzd0g2KpaUywq4EYogy8/Vl6ftnNQJmoQRe6b RFoVyfjPmlxKqxt/pAsg3RXy7DQo40HVkuwqq7LxnTH1TfRRWmCYL2izaxNImdP+jQE/ rSgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517677; x=1730122477; 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=ZE3kkVNGtxIiVPEnCVFwFowPumHB1qlzjMLlu4ndFCI=; b=U+SJw3m09v5T7v+ztI6Wgw2qRqSDMyE4OtwE89ReaN+BW6iigNYYfF4LNSc5fJdXj/ lyE53fNkkPTQb8dmbJh+J5wlvB2dZTiuSV6rAcwngGRFrAmVdYbKP2cA1BlDvj4etAGR 1YwnBZ0oFlh07cbx59aps2AoqRkYfgK4zN29O/BOiHlXte6hcjnIj1NxGz5kqrlZTHVI ss9TMi2i44eDWmbNwdkXb8CtMFtbNsPikv2SHj9kOgN+RcswxMAAbnsQDTVQKa6Z5PtF QJP3uVpenjmgNvdgpw6W8daCogKWzHNZVu3H/uC0HMCGkwN3fvzLaWDagPFujyr7WmOZ /zIw== X-Gm-Message-State: AOJu0Yyx2pHQ8rdvpLtFl+3Gar9PHX7ypb0EFcu5kDTUA9y3OPFPPVY7 3czyCOJUPZkZjJhllPpBBQSfRRQaQwhqEaT2M6JM/fWlHNO1FROq4eQAxB2P X-Google-Smtp-Source: AGHT+IGxgD/JasnglCJ18xLeaFRTXQjNvXr09NmYQ5fehvk5/475jmPf8X4ba3zItyKrNImqAEE3Kw== X-Received: by 2002:a17:902:e5cd:b0:20c:b3d9:f5bd with SMTP id d9443c01a7336-20e5a75a9a7mr165079495ad.18.1729517677147; Mon, 21 Oct 2024 06:34:37 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-20e7f0db2desm25653465ad.203.2024.10.21.06.34.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:34:36 -0700 (PDT) Date: Mon, 21 Oct 2024 21:34:40 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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. As we decide to add more checks for ref content, 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 | 12 ++++++-- 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 | 59 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 100 insertions(+), 15 deletions(-) diff --git a/builtin/refs.c b/builtin/refs.c index 24978a7b7b..886c4ceae3 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, **p; 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,15 @@ 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 (p = worktrees; *p; p++) { + struct worktree *wt = *p; + ret |= refs_fsck(get_worktree_ref_store(wt), &fsck_refs_options, wt); + } + 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 fbfcd1115c..24ad73faba 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" @@ -3536,6 +3537,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 target_name = STRBUF_INIT; @@ -3558,6 +3560,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(&target_name); + + if (!is_main_worktree(wt)) + strbuf_addf(&target_name, "worktrees/%s/", wt->id); strbuf_addf(&target_name, "%s/%s", refs_check_dir, iter->relative_path); @@ -3587,7 +3592,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, @@ -3596,17 +3602,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 0aee377439..6eb1385c50 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -105,4 +105,63 @@ 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 && + + ( + cd worktree-1 && + 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 + ) && + + ( + cd worktree-2 && + 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 + ) +' + test_done From patchwork Mon Oct 21 13:34:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844157 Received: from mail-pg1-f178.google.com (mail-pg1-f178.google.com [209.85.215.178]) (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 8C35B1E5705 for ; Mon, 21 Oct 2024 13:34:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517689; cv=none; b=CaVsOTNSjE5ZL9QgdXaRtU+jZHhrren5d8mK8qigTZ5+yyidaOSvnuofrW2/fxiJqwRWHN1kTjY+JP3zA7uFtdtMhL5G2QPwp+Es7ZeElCArwwZyqkHKWGOoKzZP72au3RG/dCHwJSbUS1QZaXGvRVr2fhK8dwigYmapQtZjCqY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517689; c=relaxed/simple; bh=GJLazU+dCdSgE10awpnKVBoQQUzQ6/IqD6H194iqA+s=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=D+YS+capySJle13y/aO0K2XSv0oyfZRHES0LD1iZvRntM6odlitciqP7MnnIz/iRh2rtn5ARzplz1+iJKDgPmcRpynv5UGEB473H/7PNldn/nKCMHZfOx4q1Ylo+6I2hzEP+CcMaX/SKyMGEt57ygBeHNa0xN/c1AHhf4PHwTrA= 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=DMzcMTfD; arc=none smtp.client-ip=209.85.215.178 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="DMzcMTfD" Received: by mail-pg1-f178.google.com with SMTP id 41be03b00d2f7-7eae96e6624so873477a12.2 for ; Mon, 21 Oct 2024 06:34:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517686; x=1730122486; 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=jFU0TdtTS78gcnNCH1jRtNx6PdVqgtzc6vnVLLxx0Tg=; b=DMzcMTfDUzNjJbzCARXkNYGTG6+ZQ7OoLMhnvJpLk4B1t98q0dyPdU5/R6V/CNPEpH c6mH3BjMULsbQbSEphsUgKxdljM7cUsrz0/Fu9+jXUa6M6QIEE4IyEPGEtRayavCJq9o xj2fwKbMvhDDGUBSDdC9JYn6nIcsqOTyMWhJiiWZGLEU7SIfSN7qrdo0BWB0aTKBGSZI JK9TatvwS58WyZzCFwf2Rm+Qoc1hLoEY/jScIQNLjn7L7KLyVLl6kz8C0Zjndc2ERMQs EUTzx1vv2uRrDe9ruCb7Mhexikf883FcOvLaXu+txlyRaJO41JkxEgm7AvaGYeQFTdqA Piew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517686; x=1730122486; 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=jFU0TdtTS78gcnNCH1jRtNx6PdVqgtzc6vnVLLxx0Tg=; b=wK8ndmLN8z+pjvA3pIDC5w2T9S6VPkMbzJqS56Stoua2VZ8AefOCBjxRTqILQ4rUEd hQqpYAAoqZ14AWJfFIF3VxZsxB1d9Ca/GaCYqgg9snEsHLW6HEDdeoRrB01M2/eEErvb 7YqO4VNayh9cLBOzVRMwGiZJ+Z83NfcBZIoqgc+X0rgZl88Ni3Ej9pxwr7Ya7R3NXFnb qerGkK5xRwdam0kA75uD9y8fhsrhLd0C9W3LnETwQKNal9Q5+Ci4a2sVxm/kuDmmjl6/ MCXUPwR6Cvpx0jTkmaVWr7YsSI6AS6u6PTcfxb3VUEVXLFd2wZDvCMpE3ZuPWrTi8e3S yqgg== X-Gm-Message-State: AOJu0Yx3cCqyAGpJXE7CwWBksYGl6vxjPWsAI6vgSwUHl4uSHqpQHure MMnyuFDePB69LW86lUYEYJuiyY+kVg2GXQgeHjCqNMCZfeRJIkB+uUksgwqw X-Google-Smtp-Source: AGHT+IG0ijOh+ws9ylH5jIlfQiStKL30DWnehkR/5F/DwiM7wuIaMsD1j4jk0oO7+qbKMeYqtPSNmA== X-Received: by 2002:a05:6a21:1304:b0:1d8:d3b4:7a73 with SMTP id adf61e73a8af0-1d92c4baaa2mr17014495637.4.1729517684731; Mon, 21 Oct 2024 06:34:44 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71ec13ea1c4sm2834847b3a.152.2024.10.21.06.34.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:34:44 -0700 (PDT) Date: Mon, 21 Oct 2024 21:34:47 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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 | 43 +++++++++++++ t/t0602-reffiles-fsck.sh | 117 ++++++++++++++++++++++++++++++++++ 4 files changed, 164 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 24ad73faba..2861980bdd 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3505,6 +3505,48 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *target_name, 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) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "cannot read ref file '%s': (%s)", + iter->path.buf, strerror(errno)); + 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 *target_name, @@ -3597,6 +3639,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 6eb1385c50..29bdd3fc01 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -164,4 +164,121 @@ test_expect_success 'ref name check should work for multiple worktrees' ' ) ' +test_expect_success 'regular ref content should be checked (individual)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + git refs verify 2>err && + test_must_be_empty err && + + bad_content=$(git rev-parse main)x && + printf "%s" $bad_content >$tag_dir_prefix/tag-bad-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-bad-1: badRefContent: $bad_content + EOF + rm $tag_dir_prefix/tag-bad-1 && + test_cmp expect err && + + bad_content=xfsazqfxcadas && + printf "%s" $bad_content >$tag_dir_prefix/tag-bad-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-bad-2: badRefContent: $bad_content + EOF + rm $tag_dir_prefix/tag-bad-2 && + test_cmp expect err && + + bad_content=Xfsazqfxcadas && + 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 +' + +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 + ) && + + bad_content_1=$(git rev-parse HEAD)x && + bad_content_2=xfsazqfxcadas && + bad_content_3=Xfsazqfxcadas && + + printf "%s" $bad_content_1 >$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_1 + EOF + rm $worktree1_refdir_prefix/bad-branch-1 && + test_cmp expect err && + + printf "%s" $bad_content_2 >$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_2 + EOF + rm $worktree2_refdir_prefix/bad-branch-2 && + test_cmp expect err && + + printf "%s" $bad_content_3 >$worktree1_refdir_prefix/bad-branch-3 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/bad-branch-3: badRefContent: $bad_content_3 + EOF + rm $worktree1_refdir_prefix/bad-branch-3 && + test_cmp expect err +' + test_done From patchwork Mon Oct 21 13:34:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844158 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (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 ED0D21EF94D for ; Mon, 21 Oct 2024 13:34:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517696; cv=none; b=OPdp7AR7v4tfWmh7s7rUg2enAsYaOmdhQtFdyDnlOzhYpaeN/5n4Od5dIxpCo4gTEQ5/3Cw16HWHvpKnwiSruWaJk+wmVzzIiSidCVepRPBr9oWD2OU9vIoaSoW482WNjYwUw9w8RH033SFZ1fx6KrLEvlehXUqHu/a3FdkN6D8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517696; c=relaxed/simple; bh=bRHjAi8ugNjBRHYo8qv0a1+pH8OtcCSLKLA/ovD4mzs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JIttXYtpYBj14FmCPY1CXj3bFZNmkuMDXgD61Dh6iYWiGEiI3m7Zs+5GRo00JIyKLqyCUl2VzvKr24f0jE0kYUl0KsxN7zJTPFJd0Cvy2DpwS18HWueYJ2JA2kKFBXs0Qxl4osoQZPbqsXIdOwS0FyTLyhCs1eJrBue7eQfKx+k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=jAvQwMZ5; arc=none smtp.client-ip=209.85.214.178 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="jAvQwMZ5" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-20cb7139d9dso39916735ad.1 for ; Mon, 21 Oct 2024 06:34:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517693; x=1730122493; 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=675DPDPoM4nQq+UGQICE+895pF3EK7hKdTsEyM4S+Vg=; b=jAvQwMZ53R+rVw9m23+iHktTIyj3IcIEFYeotICsx4duMwzZDEm0aZpJ9zzgylTCa+ gpc+4uvw3yx19HgGAFeP8xFH7eL5BUI3MAkDIwMFG4I9w8ldbsm3/6KWIsxvtRIuMry3 VodvlIqbW4+TCyVXiuKe/rk122bix5+3eAWKc1EyfDsmtIj8A4FUApjLE6HSZHT0M/tA ZEB7tQoMsZlqQ6jo1JC9SrlvrUpIEgJbNndlmEYcA1VwtUOuyj93mpbygGtTJvPSTm01 xaj2e1xlJYFSjqsg2ftlO0nDLvk1C+XsgMEX2LP2nWtLBIYMLj5FUpaXHB26nZYCCPHX RByQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517693; x=1730122493; 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=675DPDPoM4nQq+UGQICE+895pF3EK7hKdTsEyM4S+Vg=; b=wM65gWWC1e6XMV+cnyrNtcgz20b+K4gK2ALBH6hNLnjHfV41xMCrgs0m3U673RH6Af r9CvzAXj99eDtJA4qwZt574RuyFN1AHg/HgxrgNzwvx9G24j4/gXCZmFXhQm7AT51gSG f0Hy5Gw9uJLroMjjOhlSN5qf6pnY+VS1+8jmCSuy8ev2U+wwg9rQE4ja+OwXObEItsSQ kZFeDn4Ojk+ivFMNxxHMvkz1begdEDoFiBamDnDdKH6HGlg5hepSnFsa6wQ6zKq3uHMu qHtKTX/aVyOdZnOHMJ0RuXqnw6K3nVmMEhLuUq6T2y5uyeHi++uOhU1fUHabH7ti7kyc 5tsQ== X-Gm-Message-State: AOJu0Yx0S0Ilx396jb5mc3eyAwnUeN8Gm8Wp57Zo1VmeFwfuz2NK7vcI UdRaqOiBCX9E0jhvdoQwIxywg7vH7NrwfVWWCTe88Imju4mSoSblflbPTvBQ X-Google-Smtp-Source: AGHT+IG3y0Z57j05JKo/m8fhx0GowbjmGe4bxakfuUfEjuLnOe7Pr4KiWgDCRG2qRp4wCjLlzlVvZw== X-Received: by 2002:a17:903:184:b0:20c:7181:51c7 with SMTP id d9443c01a7336-20e5a923bdfmr156756465ad.52.1729517692584; Mon, 21 Oct 2024 06:34:52 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-20e7ef08d1dsm25769515ad.89.2024.10.21.06.34.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:34:52 -0700 (PDT) Date: Mon, 21 Oct 2024 21:34:55 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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 | 67 +++++++++++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 5 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 2861980bdd..b1fba92e5f 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; @@ -3533,7 +3538,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, @@ -3541,6 +3546,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 29bdd3fc01..0418d79c4f 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -201,6 +201,61 @@ test_expect_success 'regular ref content should be checked (individual)' ' error: refs/heads/a/b/branch-bad: badRefContent: $bad_content EOF rm $branch_dir_prefix/a/b/branch-bad && + test_cmp expect err && + + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err && + + printf "%s\n\n\n" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-garbage-1: trailingRefContent: has trailing garbage: '\'' + + + '\'' + EOF + rm $tag_dir_prefix/tag-garbage-1 && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-2 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-garbage-2: trailingRefContent: has trailing garbage: '\'' + + + garbage'\'' + EOF + rm $tag_dir_prefix/tag-garbage-2 && + test_cmp expect err && + + printf "%s garbage\na" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-garbage-3: trailingRefContent: has trailing garbage: '\'' garbage + a'\'' + EOF + rm $tag_dir_prefix/tag-garbage-3 && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse main)" >$tag_dir_prefix/tag-garbage-4 && + test_must_fail git -c fsck.trailingRefContent=error refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-garbage-4: trailingRefContent: has trailing garbage: '\'' garbage'\'' + EOF + rm $tag_dir_prefix/tag-garbage-4 && test_cmp expect err ' @@ -219,12 +274,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 @@ -278,6 +337,14 @@ test_expect_success 'ref content checks should work with worktrees' ' error: worktrees/worktree-1/refs/worktree/bad-branch-3: badRefContent: $bad_content_3 EOF rm $worktree1_refdir_prefix/bad-branch-3 && + test_cmp expect err && + + 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 ' From patchwork Mon Oct 21 13:35:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844159 Received: from mail-pf1-f173.google.com (mail-pf1-f173.google.com [209.85.210.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7B24A1EF94D for ; Mon, 21 Oct 2024 13:35:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517703; cv=none; b=NyL/FA+Cbf+hJ+KcBXf75hyaMfmHaRaFa0csQGFjUIJCq4pssCzlhmq5BeRmWIB9KqBPWfWVhRVRVlkuA9xcavIvLKZ+jYbpIg5cNP82ory0KqBNg7t/YjoK+EV3+cE27EfQmB8LDRrQb6RBKjK/Sx3zjsdEvvMTED6KIm09rR4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517703; c=relaxed/simple; bh=QfuNUTRrs2Gx5SkVlvy0Ldy5NTJ6EMqkydhLbZ8cHAo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JzdLvdo1wvVvvdGm+ZJIVcnRdtfPLxmwnBrXHkycZWYVdaRPYrqIGZ8sTcHVM5srZ5uCDsL9wl092BbMmqwLk0vxhhW/MKNMv3mt5pXrVNf0bcKQaN2nFBBfxgacZvd1Tp32qes6HOL2Pg3xPhvYmN6sFwaz3KSntv8JphLg0xU= 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=RgSgAEx9; arc=none smtp.client-ip=209.85.210.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RgSgAEx9" Received: by mail-pf1-f173.google.com with SMTP id d2e1a72fcca58-71e4c2e36daso3819438b3a.0 for ; Mon, 21 Oct 2024 06:35:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517700; x=1730122500; 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=0tT/lz67lU/kANzw4Uj/P1Cw9w9C2nNdBC8V4J4bhMQ=; b=RgSgAEx9bqqxv5gAAEuJXd/LwkdidJ/KV5Kt6lg0Bm5L9Y8xjPjbcNGuZjbxTf0CKK t9Y+b8jV1TiMtfOypny0ykPD/81l1oAZ4JYCRpP4arp1d6Gs9T6UMn1gt0FEZTFV9954 8F8Sk2X1v8IPut5tnFSF7J8eC5Uov176iMoCH5+wCb7TWLELhfMoDG+muXcsjJED62RA 5rzkP3qi2q8ME+hDUMdKcIDceMrtDQWGh9PNuMWjEWqe1s8UKIZ8dEWmkklOOZVk1zaU Yu0U6W+pEAjTqG9merkiperYHTLcCigJeetFhgB+ge7KBz+svH29bsG8i3LVy/IgxABQ yx/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517700; x=1730122500; 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=0tT/lz67lU/kANzw4Uj/P1Cw9w9C2nNdBC8V4J4bhMQ=; b=Gcn/gfPEPoHmLEIOBd5CBvUMCurdUNg2P1WUAgXjwpL6CQ07zF1nDGXONBhKvIRRL+ b1af2CIeEQ0i204u4VZ0F3P7ChAvvRyROC5eCE5pVRFVGFp3xYkmVSnawhUHm7l+P+rk IiWmXuT3+Aw5pQNFLMwo9QwVNYRo6RpdgPZ1xYEWruM12suXQwPxQwMvG9I0LPN0WuHz ucD1BYhJZAeNzhK+EdQ4EnKF+e6E23tDtfYgvySNjKbFP+faLUInhDnBLIn2Jre6WVPQ nSNhz+IrgfXKA1DZmYtp8szldvrs/IgK+xg7HNJnVwut6CoaaJOc4BayugRVQXZX8Dph zDBQ== X-Gm-Message-State: AOJu0YxeQCA6tF4Rx2Vjk+vMCwLLS/guhHcF0drbuR5pxS05QNLE/6r3 lPrqT/zFIOlj7SKkvuvoNsHi8+wZ4VQL8U8pf+3QI+yYB5/ORBA6hXfZb5Or X-Google-Smtp-Source: AGHT+IGL1383zQf5dofiRGJ/g+O8rMYtk58I7cp69R890OJfWFR7ivRSMO2b+50Pl+t/v5qW4bEiiw== X-Received: by 2002:a05:6a00:180f:b0:71e:6fcb:7693 with SMTP id d2e1a72fcca58-71ea4297ce7mr15114319b3a.12.1729517700021; Mon, 21 Oct 2024 06:35:00 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71ec13eb2c0sm2935162b3a.142.2024.10.21.06.34.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:34:59 -0700 (PDT) Date: Mon, 21 Oct 2024 21:35:03 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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 b1fba92e5f..1a267547f2 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 *target_name, 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, @@ -3559,6 +3596,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 0418d79c4f..f475966d7b 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -289,6 +289,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 && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err && + + printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && + git refs verify 2>err && + rm $branch_dir_prefix/branch-head && + test_must_be_empty err && + + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: 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 && + + 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'\'' + EOF + rm $branch_dir_prefix/branch-bad-1 && + 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 && @@ -345,6 +448,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 Mon Oct 21 13:35:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844160 Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7A99A1F8936 for ; Mon, 21 Oct 2024 13:35:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.182 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517711; cv=none; b=YzotMr7Spa4wacdKUuYkiv1ci9KWoLGaWYIbaM34EorjJvrFG9NXY4IEyVXGLpTISIbQFwS2x11o2VhWyZdGELl60xJeZ+yMsZ7LVKDUIiIqr+mv0Iav5ALSXGaNHbxjaTfY0v8MDUTXsPaXEW1QQZC3gZ1rD9UcMOfBcFqWFM0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517711; c=relaxed/simple; bh=uJaUhkoiCFEP35d4+ZcI+0GnHZNSU9pQFCvCZYa7XJs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LpdsPWcgNFjPTnLUyOophhHd1GiJfqzP4OV857l5cfPo7Th8eCMJEcQyMn5z5D+OhPOiYGcaOFXa6NujXoJXn9yEH3wRNl/9wAZ/NNwQfI/B5lLb4HA4xlhA3cqc4uHvrOYg9stTzk0IDB/hFuohQCb0u/DN93+Zt+E00cU4qFQ= 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=V0XrLLD8; arc=none smtp.client-ip=209.85.214.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="V0XrLLD8" Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-20cbca51687so45398525ad.1 for ; Mon, 21 Oct 2024 06:35:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517708; x=1730122508; 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=sHSLQnIOzYfm4N+F3fe22TSH+Q/W51zDBhzlnzDilYE=; b=V0XrLLD8sKSlHuSSTUczdSKQImbsTVBUhAAoOj7UM1s2kq4Dmqfvbl7zZppLR3TWWw yKIlRoI/BaaZnxzteC5AgSMn/gI0rBRmfk/mRXn3WVeREdEHBaB2jDCYwh5pwJU4tu6o LJQyZabh2J0eOqnYVNyUcBP3i7NRKMZSu/fq7f9RCXqrd3fYW/tau4xExp6bUf1MU7J5 C3HdmaCfbwQ8nBMNJaAOpEPfdm2DISMf3gbldf8f+VmwoDgoyNoGJVYTSqqciwWNKLrj lyVUQ+XOhg9JVfYJDW+cHQpWgFl7AxlsNIkhvBnTFnflsY4rA+Euwl/7GuqKWIkmHgJJ M5dQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517708; x=1730122508; 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=sHSLQnIOzYfm4N+F3fe22TSH+Q/W51zDBhzlnzDilYE=; b=ZwqN18tm1yxGRAk1SoBdnn/7bHWPdqF1a4vcKEt4dDHj7dWCxKwOz/wQjuW6UsVv+r A+HmXKEOBDhTA3YblYrT/kIAtOZZECVPyaT0Ua9irh2bUGuAk/aWbKspZ1JgS0BY+uWL fRyDasraYkB41KcSJzYqbUOelp+6Cy+AJBSESxXHXaGkwbPrItTI5qFocV7Ei//iJoSB H4vWPxjTGT8zqZxW12hX8mvBbs/i5xkZqbpPKC1+tgZ5E2K6aNYQgP7rctSeWq5btcpe Y4Pai9zz5BSTmLcjAnWeQnd5s6gDMLC747ibgRNmyheyVwi08y0WDhK5hWZYUVdC5967 dCUA== X-Gm-Message-State: AOJu0YyjyJNScZ699k+r/RIBja9N6tbgCSdhDdzbJIc7PJQjZ6K/3plh 0ypqLH/VH35X9vkE28xCCJlv07cHZuO8f4rYe9YAitS+H9Qfnr6Wx1B0U2ua X-Google-Smtp-Source: AGHT+IF4J9WWZgyd06n+HqtabmvehtxjbPm0YbyaEeW/qCg7LD5KtYGVdu2uNd71/CUgegCCGzTjRg== X-Received: by 2002:a17:903:1ca:b0:20c:8907:902 with SMTP id d9443c01a7336-20e5a923d33mr143199475ad.49.1729517707838; Mon, 21 Oct 2024 06:35:07 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-20e7f0f6d05sm25624125ad.275.2024.10.21.06.35.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:35:07 -0700 (PDT) Date: Mon, 21 Oct 2024 21:35:10 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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 | 28 ++++++++++++++++++++++++++++ 4 files changed, 50 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 1a267547f2..b4912af3b5 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 f475966d7b..c6d40ce9a1 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -392,6 +392,34 @@ 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" && + + printf "ref: HEAD\n" >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err && + + printf "ref: refs/foo\n" >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err && + + printf "ref: refs-back/heads/main\n" >$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 '\''refs-back/heads/main'\'' + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && From patchwork Mon Oct 21 13:35:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13844161 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) (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 8B4F51F893D for ; Mon, 21 Oct 2024 13:35:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.44 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517718; cv=none; b=WjITsbPtRR44WkPiSq82B1jGgPaMJO4B/b03xoT2/vvQ79KkndD+cxYe1LnhNYou2RLo5wPq146RZOfCy2Y15c04c9hTNrz/81qionOO6s3B/RyivbdG/pxUCpz7z/sbMrl0iHhtwMGqpU4gBxCW4vVhA7hKU527E0wVQk5R93A= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729517718; c=relaxed/simple; bh=LJdMvWyOACkIxE8jNxh0L6t2P0rmY02pmnxrwSUgRpc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Jih+Z12p0raKpquV3lWQjvDpWDi5QAEgFUOsnV+von7nRwWdqfYXTAdBrXD8vuD4JyJlsA/uo1qr/ijphbIiQbZpwO96LovJn3J5cPWIrZseGkK4F8oCL3YZcikAre36P+76T1vXBTB6VzotFufzzZGrFO4JoyWQm6aSZQe73/M= 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=LV7hwYuL; arc=none smtp.client-ip=209.85.216.44 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="LV7hwYuL" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2e30fb8cb07so3262632a91.3 for ; Mon, 21 Oct 2024 06:35:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729517715; x=1730122515; 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=ZrCJXlXENvmHEw7QE3vGYpjN6R05VoZ4y6A3f5c73vo=; b=LV7hwYuLugU9hYRfCfw49gmfDhGedcglcKpIYxjlsZlT8vASG6tlaiyG7VdF3UHz9c erpc7aZOKBNhgtukwWhY4XUdDulrKKyklo/STp+KLR2U6q5A9/0fdTuCnC5pTHIAiPXW Y7Z/c5SUGfXrFzw+AZiJeOYXiDvHk4hjKwXYy1v8+bxyUx18R3xGLhHGUVxj++7pgTa6 tt+Uo7N0ngZG/SuLhBkcLPoFSBjm/XfsocL5+Dgd7+GsyT2NfH5NNnXdORXXI8k4VjH0 fFF2Cl0HTkF100r4INs1HKBpX/k99EaNeENNaQywD+3izUdrTvTM5Lrz3EignyUBWPVV KeJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729517715; x=1730122515; 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=ZrCJXlXENvmHEw7QE3vGYpjN6R05VoZ4y6A3f5c73vo=; b=X2gpdok5wsirxCyZsPSZo09I7ZxixjL2HFOS2PasdLsTEhxoCb3fDxwQupjm0VUvLp BJW7XphA0/6dgjmshPYwz5kB5ol0BM2PpRKCYd8Jl6mkb207Sc0lhu4xI0s7vsiSdIgn p5ZFsUGsba0dUsYTYrffDH9BwP/ykHH8fJunPqLgm2kacsyXfk9CVLlye/+HumZd6nOd oYL/ErLuddYcKdzDrrgbB4boIkaLNTa1eBKNVQZn/tJvgC82/ZKFoOS/r9lwMrj5Yxbo JXrd29V2hmOrQSVjZcIybxZdpEuCxJCk4i6M5aCAKLuSP8Cthd/5/vPlgIYrCi7CiA3R c0cg== X-Gm-Message-State: AOJu0YxL6C1oD6a5Dl6urUssnPkfuU/3i7J9azUNL9z4xBudo8qej2Wc 13Vp/zHMtJG37olg1OkwJhYCW+7hkC5RPhCAmzjYfzQ5XnJ5Hkl7FCesTLub X-Google-Smtp-Source: AGHT+IFj3f8oq7xeriYzO19auWFTK53OJ6MEk8ME2O/6LWXp2bLQ0O1G0JTRG8iYdhJ1hnV/17NUVQ== X-Received: by 2002:a17:90a:f6d7:b0:2e3:171e:3b8c with SMTP id 98e67ed59e1d1-2e561901f11mr12624922a91.25.1729517715289; Mon, 21 Oct 2024 06:35:15 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2e5ad388297sm3750947a91.30.2024.10.21.06.35.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 06:35:14 -0700 (PDT) Date: Mon, 21 Oct 2024 21:35:18 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v6 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 | 45 +++++++++++++++++++++++++++++++++++ 4 files changed, 86 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 b4912af3b5..180f8e28b7 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->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) { ret = fsck_report_ref(o, &report, @@ -3607,13 +3636,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 c6d40ce9a1..aee7e04b82 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -420,6 +420,51 @@ test_expect_success 'the target of the textual symref should be checked' ' test_cmp expect err ' +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 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo &&