From patchwork Sun Jan 5 13:49:09 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926521 Received: from mail-pj1-f54.google.com (mail-pj1-f54.google.com [209.85.216.54]) (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 70F90849C for ; Sun, 5 Jan 2025 13:49:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084957; cv=none; b=JpJ0dVMa82XS6YpURvb+wcw+ZRKe8hA3zirplwBhOEnZ4uzxQsCfCNZUcrFcqVcFWgKYGJmA8ocAbd73AWM3oBAPnk8JXfHf9+HACKS+wdOJRhpl7wYGoK+9/G1Zv74pGjeSpir5TVKMbgsf0OmKp5nVgVJ5UkO7O7d6xDR2aDc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084957; c=relaxed/simple; bh=WAWnmXpVNRml7p2qjdvfC6Ocv2EN15hGeSPwmqahlQk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Ja5kP74cMGtB84MonZX/orqzBXyczCov3iZDiYv/INopyWnl3EtitZLPVmFYSuzR8V13v2WgpFCS+xwFzwEZAzylt+QD+m8Z7kFfYH0mt+FtiCjfNVmVX91erYLbLj5WdkgtuuRHbFeyjfBK+rZFLnb1J6OIZz1+1TS6BmoAMOM= 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=MdcxdozM; arc=none smtp.client-ip=209.85.216.54 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="MdcxdozM" Received: by mail-pj1-f54.google.com with SMTP id 98e67ed59e1d1-2ef6c56032eso14281425a91.2 for ; Sun, 05 Jan 2025 05:49:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736084954; x=1736689754; 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=C+fVAmzLQ4Om50kyTFGW++t5pk9Lgq8uMnlI+JyO7Hg=; b=MdcxdozMMOAg2spWQ7SwViwQFPHrl7d0wfoAvwR6P+5BDyGfKR/ytLPTPseAIhtXi2 lkzIZOjJt7XTf/LSqaTRCJ9vchcdRcz2YyzdsRYp1yQG+I0Op1sjupY8pht5SYRcwrdL XJtxhvEphocI8t16b4kvoKcoMFlSrslbrWUyXlLv/OPIr/Xq3tWwa78dgjqRwd0i2389 kBOGKiwWSTz436glSBLLZ7pAD9Mn9eSOxGYggOYm6ZaF3kYPnNj38oJVi7Ws/nzLCXRN t12kThO1SOVJbHZqi6WqM0crFE5u3cdmxHCLFZInsjZt9RZKdQELSa9o8mxn1kmqtGWR h10Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736084954; x=1736689754; 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=C+fVAmzLQ4Om50kyTFGW++t5pk9Lgq8uMnlI+JyO7Hg=; b=Dh9An5RtG/yBGdqcR9K+VbnBk+qVv88YnKwJUExTLpHVTbLzAPBGA7s107L/bk43QY oFVVLRVocz5ZhhsayFpJPAojcMbfx8gJTcsToRiKbbtuFtzyc48dZysmhn+kNFqyxn/l WyOqpt/6E9AO7afzMTiMu7xz3SsIRoKIoqbcakEXh0DJCcj+mXCq6d/WIkT7AttvXX/t Kd25WgpMNsrLxgn83jD/NZh3eyP4b5oUeBsaE8DNoJIvbonJi6P97eDd5zjJATNyiUvQ 5Z9sY2ZWhTmat2PzPhJPSqNMRamsQO8hwkQXjhm5Nxeq8Ey6oa3spdrZdMSxPZrhON2b GOng== X-Gm-Message-State: AOJu0Yz8F/6JU8fxev8D3aszsuPs0uszfmPBx3qe15htKWkbNW9b8WxU swVgtfAvwuazhJ3+5mlzNeNwvgwRGzi+JUA6WDAIFx3gCISockqYx4LhNA== X-Gm-Gg: ASbGncsl/JBLdXLE4/M0RZ0BX5sjyJQfPuhCRhKSj1Era8e11h5L5IaU9En9Ci3bp/Q q8Qeao1VJCKjluodoR3+nC/jnFIZLTgL7Nh12ISK9424tXkcUw8T+KJxyjTylHslPlauoHbUxSu iOZ23mb2AMU/IEMxDjSesoCwIwxoQmlcZ59EmAxUE9eDRzl91fgYBXnPRI2NEdcmRoLIBjECfFp iIirnsxXn4QyyidDUyQLxDCspoYDgkBZCU= X-Google-Smtp-Source: AGHT+IEH+5/WWJQHDU801v1xcBBkFboutv0epXrh1ekCR1zYNXv9pHq+MGIRet3WjXiznkfyc/gMKw== X-Received: by 2002:a17:90a:da8e:b0:2ee:fdf3:38dd with SMTP id 98e67ed59e1d1-2f452ec2919mr72851492a91.23.1736084954134; Sun, 05 Jan 2025 05:49:14 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dca02e91sm275612925ad.274.2025.01.05.05.49.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:49:13 -0800 (PST) Date: Sun, 5 Jan 2025 21:49:09 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 01/10] files-backend: add object check for regular 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: Although we use "parse_loose_ref_content" to check whether the object id is correct, we never parse it into the "struct object" structure thus we ignore checking whether there is a real object existing in the repo and whether the object type is correct. Use "parse_object" to parse the oid for the regular ref content. If the object does not exist, report the error to the user by reusing the fsck message "BAD_REF_CONTENT". Then, we need to check the type of the object. Just like "git-fsck(1)", we only report "not a commit" error when the ref is a branch. Last, update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 50 ++++++++++++++++++++++++++++++++-------- t/t0602-reffiles-fsck.sh | 30 ++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 64f51f0da9..0a4912c009 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -20,6 +20,7 @@ #include "../lockfile.h" #include "../object.h" #include "../object-file.h" +#include "../packfile.h" #include "../path.h" #include "../dir.h" #include "../chdir-notify.h" @@ -3589,6 +3590,34 @@ static int files_fsck_symref_target(struct fsck_options *o, return ret; } +static int files_fsck_refs_oid(struct fsck_options *o, + struct ref_store *ref_store, + struct fsck_ref_report report, + const char *target_name, + struct object_id *oid) +{ + struct object *obj; + int ret = 0; + + if (is_promisor_object(ref_store->repo, oid)) + return 0; + + obj = parse_object(ref_store->repo, oid); + if (!obj) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "points to non-existing object %s", + oid_to_hex(oid)); + } else if (obj->type != OBJ_COMMIT && is_branch(target_name)) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "points to non-commit object %s", + oid_to_hex(oid)); + } + + return ret; +} + static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_options *o, const char *target_name, @@ -3654,18 +3683,19 @@ static int files_fsck_refs_content(struct ref_store *ref_store, } if (!(type & REF_ISSYMREF)) { + ret |= files_fsck_refs_oid(o, ref_store, report, target_name, &oid); + 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; + ret |= fsck_report_ref(o, &report, + FSCK_MSG_REF_MISSING_NEWLINE, + "misses LF at the end"); + } else if (*trailing != '\n' || *(trailing + 1)) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_TRAILING_REF_CONTENT, + "has trailing garbage: '%s'", trailing); } + + goto cleanup; } else { ret = files_fsck_symref_target(o, &report, &referent, 0); goto cleanup; diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index d4a08b823b..75f234a94a 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -161,8 +161,10 @@ 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 && + git branch branch-1 && mkdir -p "$branch_dir_prefix/a/b" && git refs verify 2>err && @@ -198,6 +200,28 @@ test_expect_success 'regular ref content should be checked (individual)' ' rm $branch_dir_prefix/branch-no-newline && test_cmp expect err && + for non_existing_oid in "$(test_oid 001)" "$(test_oid 002)" + do + printf "%s\n" $non_existing_oid >$branch_dir_prefix/invalid-commit && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/invalid-commit: badRefContent: points to non-existing object $non_existing_oid + EOF + rm $branch_dir_prefix/invalid-commit && + test_cmp expect err || return 1 + done && + + for tree_oid in "$(git rev-parse main^{tree})" "$(git rev-parse branch-1^{tree})" + do + printf "%s\n" $tree_oid >$branch_dir_prefix/branch-tree && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-tree: badRefContent: points to non-commit object $tree_oid + EOF + rm $branch_dir_prefix/branch-tree && + test_cmp expect err || return 1 + done && + for trailing_content in " garbage" " more garbage" do printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && @@ -244,15 +268,21 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' bad_content_1=$(git rev-parse main)x && bad_content_2=xfsazqfxcadas && bad_content_3=Xfsazqfxcadas && + non_existing_oid=$(test_oid 001) && + tree_oid=$(git rev-parse main^{tree}) && 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 && + printf "%s\n" $non_existing_oid >$branch_dir_prefix/branch-non-existing-oid && + printf "%s\n" $tree_oid >$branch_dir_prefix/branch-tree && test_must_fail git refs verify 2>err && cat >expect <<-EOF && error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 + error: refs/heads/branch-non-existing-oid: badRefContent: points to non-existing object $non_existing_oid + error: refs/heads/branch-tree: badRefContent: points to non-commit object $tree_oid 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'\'' From patchwork Sun Jan 5 13:49:19 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926522 Received: from mail-pl1-f181.google.com (mail-pl1-f181.google.com [209.85.214.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C98671EA80 for ; Sun, 5 Jan 2025 13:49:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084966; cv=none; b=hr8lyffdHPBml2Je6EAhOl3oS+WI04e232l/+z1dbA/eZ6uAKDgS+F+AgFJoAG7eCDHmg0XC+Qrwcn7eggdLUvff15AP00plnXYGDl6/d2iUeqaCgy9BVtgzJcMhTJU8N+3k3axsPKdvnKGYnfsVCNXfrsCOezbL9ay4awVcIfI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084966; c=relaxed/simple; bh=d3qR3ZFxaeWlmXlPs5P841hWHemZiaBfqce58HHAYqA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MP1MiQiMriTap1Zva6Xq3SJW1Ez9TW+UcBt2ES70eG0+snKF6JcpxPoVWF/p93qP/NbD91Vv38CrmnNNg+NQOf5Qk6sQgzgEPAKRbRtLIolvZflGmlqYAo2RWD7mXg8aoGH6wVMwJ/QqRPbBHGaSYKu6Zysyo55xBPVheLCu4s4= 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=TrV2aTHT; arc=none smtp.client-ip=209.85.214.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="TrV2aTHT" Received: by mail-pl1-f181.google.com with SMTP id d9443c01a7336-2164b1f05caso187905285ad.3 for ; Sun, 05 Jan 2025 05:49:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736084963; x=1736689763; 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=XKGrfoIJOmc3wq5aj4fqGaPQgHGOBpQN7ZgkCX+fcNc=; b=TrV2aTHTjnUKJxbtAMTfPX458HPpZpA1LIjA+2xk0kJOVi6gYHbl7sPIt3VjbpjqUg WD4moJPyEvML7FMNy3FaU8QSyvARHzp1kzWlcI95uIS1qwvrE1ViKU+UjTUpB5GcceEW FKVpbbmlw5/7r6utI6fOGcKOi4xAYHAVS42Oo51KREpy0NF9HoTvzRkqshAFKZH1oBAP 48xc7c6DiqV6BA2h+eZvuDFYA9DyP7mpxLn2folczaFblSn76erWcgoJp1pAsR804wq5 ab0hvpagLgW5ODt9bbl23z5zdx3pCVIGBJQ4KIE0Z82GbkdiDiQP77IcPCcFnTpEbqxG OmbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736084963; x=1736689763; 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=XKGrfoIJOmc3wq5aj4fqGaPQgHGOBpQN7ZgkCX+fcNc=; b=qKY+SXjemCO9MhHEU8GH4mlKfDh55W25QRNkGh97R3PMKmnD+Amm8adJj0cxaLGE5r Q9xucePp8OeEoyPt1PmltTSwVhlRCjzcMn+VIIv/g46j2GPSAaYYYEqZeUGxoy1vQUG5 IxGblrStunG8q9LQNnAKtrLl9h5L7ABo780Wxt8w+MVSCep3pCwP4kVS8J19K01bLd4P YifYXvYPPPyY3RL28h8f4RDAx2AI5ej/dP9mzK0kAbEYITQnubY24fBD5J56WxmG6ydQ LXW2/6Bzq1DMTw9gZAJLAOHEyLzm+VZNB9g41kx8qQHw1opUOkaEZwCtoN7dtOlsobB3 ZYOA== X-Gm-Message-State: AOJu0YwAcZTYL+PIGnu54UZsCp/h8Vik/tosP9kqToYB2yvaDGk6mSRt f2vVInQUefbslgmOKHfUSFRrP4i2EyknHLQ6nJvMk8cmbnW8iQGwqmsQEw== X-Gm-Gg: ASbGncs4faAp1zyrxEouZ2K9gXaY+xBPhF3rPxgaybsL/m+gw+lgARd4Z/k1snYzkeh F5UaYGixd4I5MzTgOaPq0edO+Qz64GAPNK6W4WJdZhuM7imIUyRu5ltfZ0WPj5q7XWoh4S3zRdd SYcMuK/bWEf56txt6iH5nJhjMvHUBPyYICPNHDzjkZnRuB6L71MV3fHIfIQlml1ccqrhlq2vkMP p3ismljR9a67qq/4EY7DSZ1mJkVQ+8YQAo= X-Google-Smtp-Source: AGHT+IECb0q7jhuMaQrd9ZDuzD29bDj8FfoBinA7bSA8+1p1D8aGu1Fx/7WkSmUNWunJP5p1iZFNTQ== X-Received: by 2002:a17:902:ecd2:b0:215:89a0:416f with SMTP id d9443c01a7336-219e6ebd0a4mr745748295ad.30.1736084963393; Sun, 05 Jan 2025 05:49:23 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dca029ccsm274509155ad.260.2025.01.05.05.49.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:49:22 -0800 (PST) Date: Sun, 5 Jan 2025 21:49:19 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 02/10] builtin/refs.h: get worktrees without reading head info 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 "packed-backend.c", there are some functions such as "create_snapshot" and "next_record" which would check the correctness of the content of the "packed-ref" file. When anything is bad, the program will die. It may seem that we have nothing relevant to above feature, because we are going to read and parse the raw "packed-ref" file without creating the snapshot and using the ref iterator to check the consistency. However, when using "get_worktrees" in "builtin/refs", we will parse the head information. If the referent of the "HEAD" is inside the "packed-ref", we will call "create_snapshot" and "next_record" functions to parse the "packed-ref" to get the head information. And if there are something wrong, the program will die. Although this behavior has no harm for the program, it will short-circuit the program. When the users execute "git refs verify" or "git fsck", we don't want to simply die the program but rather show the warnings or errors as many as possible to info the users. So, we should avoiding reading the head info. Fortunately, in 465a22b338 (worktree: skip reading HEAD when repairing worktrees, 2023-12-29), we have introduced a function "get_worktrees_internal" which allows us to get worktrees without reading head info. Create a new exposed function "get_worktrees_without_reading_head", then replace the "get_worktrees" in "builtin/refs" with the new created function. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/refs.c | 2 +- worktree.c | 5 +++++ worktree.h | 6 ++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/builtin/refs.c b/builtin/refs.c index a29f195834..55ff5dae11 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -88,7 +88,7 @@ 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); - worktrees = get_worktrees(); + worktrees = get_worktrees_without_reading_head(); for (size_t i = 0; worktrees[i]; i++) ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), &fsck_refs_options, worktrees[i]); diff --git a/worktree.c b/worktree.c index af68b24f9d..74cb463e51 100644 --- a/worktree.c +++ b/worktree.c @@ -174,6 +174,11 @@ struct worktree **get_worktrees(void) return get_worktrees_internal(0); } +struct worktree **get_worktrees_without_reading_head(void) +{ + return get_worktrees_internal(1); +} + const char *get_worktree_git_dir(const struct worktree *wt) { if (!wt) diff --git a/worktree.h b/worktree.h index 38145df80f..1ba4a161a0 100644 --- a/worktree.h +++ b/worktree.h @@ -30,6 +30,12 @@ struct worktree { */ struct worktree **get_worktrees(void); +/* + * Like `get_worktrees`, but does not read HEAD. This is useful when checking + * the consistency, as reading HEAD may not be necessary. + */ +struct worktree **get_worktrees_without_reading_head(void); + /* * Returns 1 if linked worktrees exist, 0 otherwise. */ From patchwork Sun Jan 5 13:49:28 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926523 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) (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 E126A14D456 for ; Sun, 5 Jan 2025 13:49:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084975; cv=none; b=hX2XBkPsK24vQEdvO/E9JEQpnNbsMknvq8K35IklQNgJMyR9cnKaqvb/a4FlFIBNfN6REO27UFT1TE0I2Bfm3czFxcf3brAItov4FB++tLwYRPG4H6oh8SI7FbdgKFd1aT+AsUBPbyh8asR/eowoM1yd6nk7tzfZvcCpYayxVes= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084975; c=relaxed/simple; bh=gDS12VvN2487cMUDFjbmhcqNvLF2y1VJb9gKEygp1J8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=u+QG/2kROhbWXSlUPuSzCAjiNxHhzvXWbj28/9ggUn8NanUL2N8RxRQv3/ZDWYi2U1IQkLi3mpAEXOXxxFFblRTnaeYYVAsLf3WSeiWy9lPAoNOElozQBtxWGdN2j2LiGc28FxF4BlYcXoYBTC65bF1wh5A2o06vxaJvwtXxqRU= 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=SG2OzHH+; arc=none smtp.client-ip=209.85.216.41 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="SG2OzHH+" Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-2ee8aa26415so19483010a91.1 for ; Sun, 05 Jan 2025 05:49:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736084972; x=1736689772; 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=jnFAsS4DH7o78Lcy8qKUYeCwUDDeg88O9RhW1h+7zzo=; b=SG2OzHH+FiOs0uW/smQ5FjPUfWUafgyEAfanvAYb8LzxtZkk6H5l9clO7ycPhVPDr1 yY8ZEzLdcuu1bf4NoaJ4kHg15ipCvdC1Kqr7Vwx7ouwtOUMQlJFWGy/ad2I3SpelM+OK szHjYaOnvpvPxG5P0ejr3Bs5b+C1HV9BUrxhFsMc006TX22ZeWp0CBArO5doJrGEURLi I2MNJkEzhyFrIG5QirS0dB5N4KRx+Jn9LjqWQl7jzf0CtgRS9KpuHRJPuDrqd8D2vQ7P dTLiEK8rvs/42HkneiIsUB2ncRPK160fnhzKLtW2rjag31Jb3nlfnOFUXztmAerf8R7a cIBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736084972; x=1736689772; 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=jnFAsS4DH7o78Lcy8qKUYeCwUDDeg88O9RhW1h+7zzo=; b=FTduIhFP0VRiuUezN0t3PiIo5liZOls5XTb7frEg+8D2XgwhJTX/wTQeikBx+SJOB6 9GWjFW7N8ndeJen36WFUPd36ZIQiwwJOioP0J5xojNIK7KNZDtIW7UbnIGO9VyVn3HnE 713+LZ9zNzWeD3DKJfBcVJP8msIFSBw3aN24H9H2cKxfj3zKCPAi4j03G7NNkIhmd6X7 bizRngP5Ajci8p1iG5SkoJTDHLiW4nCVCPQqPWPGTAQOwLdUxaUufbmJYP72PwtCbrwV i8r0TlsZf6wki9oUS8o7uEjpuRQYzFa79TUJuOCi8kxeTRI+qzqZz1eo+9xCC8TzgHsD pJ8Q== X-Gm-Message-State: AOJu0YwReI2ZqZkcR3qbg6B/FJc2ycP7hekbohk64sytjsDoVnv9U1Z0 4y/X/mAXoQLvafvNWofCNJcgMJffO8r0URAqQITDVtGyJcYJmE+1Fwt+/g== X-Gm-Gg: ASbGncv2HFjMaAUXM6qgonWO+TbVc3i8OP/v4X6Ypks2cru0EIaeTPAan80Xf9UHmdO vO8lqyF3Q1e0C/m47rklNMhNxCr4T0SYwglNqWLNUqUTtXjmqQxsU4quvOu3YRGI5c7sPLTGghw 51rSOsYH9yvL+SmYiN1z6tjYbfsvMWhZu5u6qKGTXzy7cUqaecy7yP1ytVBCsFq6YOaMFg+M4hV 91szOs0ayU35z6ps3AEssv8Rg/zluGyQm0= X-Google-Smtp-Source: AGHT+IHbiG5/TwelSCLzYlaGAr0O0ALFrStClSIGkb6Mmc9fH2tZl7qa1G8VsfzaPgBgfJvC0UCOxA== X-Received: by 2002:a17:90b:544f:b0:2ee:d433:7c54 with SMTP id 98e67ed59e1d1-2f452e372e2mr84008054a91.19.1736084972351; Sun, 05 Jan 2025 05:49:32 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dc9f51a0sm273954625ad.187.2025.01.05.05.49.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:49:31 -0800 (PST) Date: Sun, 5 Jan 2025 21:49:28 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 03/10] packed-backend: check whether the "packed-refs" is regular 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: Although "git-fsck(1)" and "packed-backend.c" will check some consistency and correctness of "packed-refs" file, they never check the filetype of the "packed-refs". The user should always use "git packed-refs" command to create the raw regular "packed-refs" file, so we need to explicitly check this in "git refs verify". Use "lstat" to check the file mode. If we cannot check the file status, this is OK because there is a chance that there is no "packed-refs" in the repo. Reuse "FSCK_MSG_BAD_REF_FILETYPE" fsck message id to report the error to the user if "packed-refs" is not a regular file. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 33 +++++++++++++++++++++++++++++---- t/t0602-reffiles-fsck.sh | 20 ++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 3406f1e71d..d9eb2f8b71 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -4,6 +4,7 @@ #include "../config.h" #include "../dir.h" #include "../gettext.h" +#include "../fsck.h" #include "../hash.h" #include "../hex.h" #include "../refs.h" @@ -1747,15 +1748,39 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } -static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED, +static int packed_fsck(struct ref_store *ref_store, + struct fsck_options *o, struct worktree *wt) { + struct packed_ref_store *refs = packed_downcast(ref_store, + REF_STORE_READ, "fsck"); + struct stat st; + int ret = 0; if (!is_main_worktree(wt)) - return 0; + goto cleanup; - return 0; + /* + * If the packed-refs file doesn't exist, there's nothing to + * check. + */ + if (lstat(refs->path, &st) < 0) + goto cleanup; + + if (o->verbose) + fprintf_ln(stderr, "Checking packed-refs file %s", refs->path); + + if (!S_ISREG(st.st_mode)) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs"; + + ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_FILETYPE, + "not a regular file"); + goto cleanup; + } + +cleanup: + return ret; } struct ref_storage_be refs_be_packed = { diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 75f234a94a..307f94a3ca 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -626,4 +626,24 @@ test_expect_success 'ref content checks should work with worktrees' ' test_cmp expect err ' +test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' + 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 pack-refs --all && + + mv .git/packed-refs .git/packed-refs-back && + ln -sf packed-refs-bak .git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs: badRefFiletype: not a regular file + EOF + rm .git/packed-refs && + test_cmp expect err +' + test_done From patchwork Sun Jan 5 13:49:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926524 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 AF93814A4C1 for ; Sun, 5 Jan 2025 13:49:43 +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=1736084985; cv=none; b=nu334GjrfYRi2GJwmkdrSkDqAFnlPIMm6fTicg51iPN76Xup5wz7vXhWZbtgNAG38Iy/+9p6+Q4eXiJ4EZRcdSt1s6HjJljNSC7gUOLtdatbrWzxVWYjWacdX/C7aWkoEtQYJZMC2rhOGRzXMNXBuRrZh7k1qR4+XB5qHFO3fIU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084985; c=relaxed/simple; bh=+7c9w+GpS6mhDuVvUP51QNBmQvWqADOTvknjLXvUIac=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=REV+U+gm+pVu11IZxgKXPWM0wg249RTL5H5zwmv2WfwTrbcYHg0aYIAqs9L1bBBtoSnPIEpMio+iGOAS0A9xYhE6evWia35/hS40mQ3QwB+uU7R5JWX9W25fmVUESMozY7fU4m805iv+7SxF2UONgu3wEjkFY5Mz/Spg2wWpv8w= 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=cYfYVbEV; 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="cYfYVbEV" Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-2162c0f6a39so199659845ad.0 for ; Sun, 05 Jan 2025 05:49:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736084982; x=1736689782; 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=Qd9PdyPvUdMAp9AL6bTD/L6KHWZAvmW2XCBtNzWrwZw=; b=cYfYVbEVf2thGCU+QxHkhX6haxg9MKnUEUSKNDFerbmcdcfPI/VreJB8bcOvTY8+/w KkunVh2IvXW/RUT4PF+bZXZkjtxRYeaRgveZMc/707IE8UVTVpfIJEpGmnTS85bf4g9P hDDjUCcFCxNe37CSJFFJQFfIC3vkCek45kpaPawFVexapjMTpCHtotM2nkz2JT9Rkd8s UqM3y76Zlm1Ntn4pZMQ4opek2sw3No39u34IyCZpH3klCbS/Zq13CPwx9Qzwk8Q0bkVc gn6liKhOXNxXk5DnbZIGYcqGsItJwmVWj1v/ACsKSoXjTnrraumyEu4xFGRugKe2h+Qq rJeg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736084982; x=1736689782; 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=Qd9PdyPvUdMAp9AL6bTD/L6KHWZAvmW2XCBtNzWrwZw=; b=cDXkVm0dFN3IpB1NB6I3xjMulpnPaIFUY3xARoivJzHx9eM2wOs4j4dt6+xiMSUxTO L5ijNfnU/JeWHxpAEHqFNa3iWLNKr5Dl+86gQ7ydolgkE3NHkqb6uoNfwdu/giQthJ8d ShZvFhXqmvk4QjBc53rM9BjwQI7zchYkMNPnrFr1jCSY932LI7X2Y23cM1VPq3DvIwm3 ME89Rq5KOANfMdMeY15hjD/4D+/dzzRwGTYLyaFhvQ02040jqkzmifuJwE160OmXOv3f nD3DVK+LlyrkecAU2gdtxAJIel7dMm+gPySAKXm3MZiUU6BbYGQwXBD47QPbuxbd+Yx2 6Hhw== X-Gm-Message-State: AOJu0YxjeMyZlhEMku61m9Js/79+VBexxdIeTYVfjbtIvV3x2SaPW9Qj p0ykTE8EGzf/uUDrJUnBd2sSxcBf8GEXSU2b8eFhfTt7j7gOZ0WfpNgQww== X-Gm-Gg: ASbGnctplDQeuQnGzg4qGLL9DKjik3dh0NvxldLkqL22YUHti/GOaPzO6eQjjCFJD8/ 2b8ARawrEF1n54wdpYa7D2H7cxKD9JH5TTbarxAwyhYozQHrVUqM0r8g0xSdEpeWKC1/d4tfPf9 qGqmS4Lt50bpWGh1/aoSUACYh2Up2OKlbs0XN2Ml1G2aYqRHtU71a0YirzFrzrieTFvcUoY1u7K L6vCoLwRu8GNnkYJ6AWbb5HPOwITNJ1k8g= X-Google-Smtp-Source: AGHT+IGdaz95OlT7fcb90XbrMqZDLXp2XplI00a+IdpNjFORVzGX8a6+0sIrrnFLsYv67kW2nGly9Q== X-Received: by 2002:a05:6a00:8009:b0:728:f266:cb09 with SMTP id d2e1a72fcca58-72aa9b03a0bmr95426399b3a.13.1736084981818; Sun, 05 Jan 2025 05:49:41 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72aad90e439sm29650168b3a.178.2025.01.05.05.49.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:49:41 -0800 (PST) Date: Sun, 5 Jan 2025 21:49:37 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 04/10] packed-backend: add "packed-refs" header consistency check 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 "packed-backend.c::create_snapshot", if there is a header (the line which starts with '#'), we will check whether the line starts with "# pack-refs with:". As we are going to implement the header consistency check, we should port this check into "packed_fsck". However, the above check is not enough, this is because "git pack-refs" will always write "PACKED_REFS_HEADER" which is a constant string to the "packed-refs" file. So, we should check the following things for the header. 1. If the header does not exist, we may report an error to the user because it should exist, but we do allow no header in "packed-refs" file. So, create a new fsck message "packedRefMissingHeader(INFO)" to warn the user and also keep compatibility. 2. If the header content does not start with "# packed-ref with:", we should report an error just like what "create_snapshot" does. So, create a new fsck message "badPackedRefHeader(ERROR)" for this. 3. If the header content is not the same as the constant string "PACKED_REFS_HEADER", ideally, we should report an error to the user. However, we allow other contents as long as the header content starts with "# packed-ref with:". To keep compatibility, create a new fsck message "unknownPackedRefHeader(INFO)" to warn about this. We may tighten this rule in the future. In order to achieve above checks, read the "packed-refs" file via "strbuf_read_file". Like what "create_snapshot" and other functions do, we could split the line by finding the next newline in the buf. If we cannot find a newline, this is an error. So, create a function "packed_fsck_ref_next_line" to find the next newline and if there is no such newline, use "packedRefEntryNotTerminated(INFO)" to report an error to the user. Then, parse the first line to apply the above three checks. Update the test to excise the code. However, when adding the new test for a bad header, the program will still die in the "create_snapshot" method. This is because we have checked the files-backend firstly and we use "parse_object" to check whether the object exists and whether the type is correct. This function will eventually call "create_snapshot" and "next_record" method, if there is something wrong with packed-backend, the program just dies. It's bad to just die the program because we want to report the problems as many as possible. We should avoid checking object and its type when packed-backend is broken. So, we should first check the consistency of the packed-backend then for files-backend. Add a new flag "safe_object_check" in "fsck_options", when there is anything wrong with the parsing process, set this flag to 0 to avoid checking objects in the later checks. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 16 ++++++ fsck.h | 6 ++ refs/files-backend.c | 6 +- refs/packed-backend.c | 105 ++++++++++++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 44 ++++++++++++++ 5 files changed, 174 insertions(+), 3 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index b14bc44ca4..34375a3143 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,10 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefHeader`:: + (ERROR) The "packed-refs" file contains an invalid + header. + `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. @@ -176,6 +180,13 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`packedRefEntryNotTerminated`:: + (ERROR) The "packed-refs" file contains an entry that is + not terminated by a newline. + +`packedRefMissingHeader`:: + (INFO) The "packed-refs" file does not contain the header. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref @@ -208,6 +219,11 @@ `treeNotSorted`:: (ERROR) A tree is not properly sorted. +`unknownPackedRefHeader`:: + (INFO) The "packed-refs" header starts with "# pack-refs with:" + but the remaining content is not the same as what `git pack-refs` + would write. + `unknownType`:: (ERROR) Found an unknown object type. diff --git a/fsck.h b/fsck.h index a44c231a5f..026ad1d537 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ @@ -53,6 +54,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE, ERROR) \ FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ + FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ @@ -90,6 +92,8 @@ enum fsck_msg_type { FUNC(REF_MISSING_NEWLINE, INFO) \ FUNC(SYMREF_TARGET_IS_NOT_A_REF, INFO) \ FUNC(TRAILING_REF_CONTENT, INFO) \ + FUNC(UNKNOWN_PACKED_REF_HEADER, INFO) \ + FUNC(PACKED_REF_MISSING_HEADER, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) @@ -163,6 +167,7 @@ struct fsck_options { fsck_error error_func; unsigned strict; unsigned verbose; + int safe_object_check; enum fsck_msg_type *msg_type; struct oidset skip_oids; struct oidset gitmodules_found; @@ -198,6 +203,7 @@ struct fsck_options { } #define FSCK_REFS_OPTIONS_DEFAULT { \ .error_func = fsck_refs_error_function, \ + .safe_object_check = 1, \ } /* descend in all linked child objects diff --git a/refs/files-backend.c b/refs/files-backend.c index 0a4912c009..66eae36184 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3599,7 +3599,7 @@ static int files_fsck_refs_oid(struct fsck_options *o, struct object *obj; int ret = 0; - if (is_promisor_object(ref_store->repo, oid)) + if (!o->safe_object_check || is_promisor_object(ref_store->repo, oid)) return 0; obj = parse_object(ref_store->repo, oid); @@ -3819,8 +3819,8 @@ static int files_fsck(struct ref_store *ref_store, struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ, "fsck"); - return files_fsck_refs(ref_store, o, wt) | - refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt); + return refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) | + files_fsck_refs(ref_store, o, wt); } struct ref_storage_be refs_be_files = { diff --git a/refs/packed-backend.c b/refs/packed-backend.c index d9eb2f8b71..3b11abe5f8 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1748,12 +1748,100 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +static int packed_fsck_ref_next_line(struct fsck_options *o, + int line_number, const char *start, + const char *eof, const char **eol) +{ + int ret = 0; + + *eol = memchr(start, '\n', eof - start); + if (!*eol) { + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + + strbuf_addf(&packed_entry, "packed-refs line %d", line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_ENTRY_NOT_TERMINATED, + "'%.*s' is not terminated with a newline", + (int)(eof - start), start); + + /* + * There is no newline but we still want to parse it to the end of + * the buffer. + */ + *eol = eof; + strbuf_release(&packed_entry); + } + + return ret; +} + +static int packed_fsck_ref_header(struct fsck_options *o, const char *start, const char *eol) +{ + const char *err_fmt = NULL; + int fsck_msg_id = -1; + + if (!starts_with(start, "# pack-refs with:")) { + err_fmt = "'%.*s' does not start with '# pack-refs with:'"; + fsck_msg_id = FSCK_MSG_BAD_PACKED_REF_HEADER; + } else if (strncmp(start, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER))) { + err_fmt = "'%.*s' is not the official packed-refs header"; + fsck_msg_id = FSCK_MSG_UNKNOWN_PACKED_REF_HEADER; + } + + if (err_fmt && fsck_msg_id >= 0) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs.header"; + + return fsck_report_ref(o, &report, fsck_msg_id, err_fmt, + (int)(eol - start), start); + + } + + return 0; +} + +static int packed_fsck_ref_content(struct fsck_options *o, + const char *start, const char *eof) +{ + int line_number = 1; + const char *eol; + int ret = 0; + + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + if (*start == '#') { + ret |= packed_fsck_ref_header(o, start, eol); + + start = eol + 1; + line_number++; + } else { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs"; + + ret |= fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_MISSING_HEADER, + "missing header line"); + } + + /* + * If there is anything wrong during the parsing of the "packed-refs" + * file, we should not check the object of the refs. + */ + if (ret) + o->safe_object_check = 0; + + + return ret; +} + static int packed_fsck(struct ref_store *ref_store, struct fsck_options *o, struct worktree *wt) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); + struct strbuf packed_ref_content = STRBUF_INIT; struct stat st; int ret = 0; @@ -1779,7 +1867,24 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } + if (strbuf_read_file(&packed_ref_content, refs->path, 0) < 0) { + /* + * Although we have checked that the file exists, there is a possibility + * that it has been removed between the lstat() and the read attempt by + * another process. In that case, we should not report an error. + */ + if (errno == ENOENT) + goto cleanup; + + ret = error_errno("could not read %s", refs->path); + goto cleanup; + } + + ret = packed_fsck_ref_content(o, packed_ref_content.buf, + packed_ref_content.buf + packed_ref_content.len); + cleanup: + strbuf_release(&packed_ref_content); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 307f94a3ca..6c729e749a 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -646,4 +646,48 @@ test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' test_cmp expect err ' +test_expect_success 'packed-refs header should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + cd repo && + test_commit default && + + git refs verify 2>err && + test_must_be_empty err && + + printf "$(git rev-parse main) refs/heads/main\n" >.git/packed-refs && + git refs verify 2>err && + cat >expect <<-EOF && + warning: packed-refs: packedRefMissingHeader: missing header line + EOF + rm .git/packed-refs && + test_cmp expect err && + + for bad_header in "# pack-refs wit: peeled fully-peeled sorted " \ + "# pack-refs with traits: peeled fully-peeled sorted " \ + "# pack-refs with a: peeled fully-peeled" + do + printf "%s\n" "$bad_header" >.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs.header: badPackedRefHeader: '\''$bad_header'\'' does not start with '\''# pack-refs with:'\'' + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done && + + for unknown_header in "# pack-refs with: peeled fully-peeled sorted garbage" \ + "# pack-refs with: peeled" \ + "# pack-refs with: peeled peeled-fully sort" + do + printf "%s\n" "$unknown_header" >.git/packed-refs && + git refs verify 2>err && + cat >expect <<-EOF && + warning: packed-refs.header: unknownPackedRefHeader: '\''$unknown_header'\'' is not the official packed-refs header + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done +' + test_done From patchwork Sun Jan 5 13:49:51 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926525 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.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 53747849C for ; Sun, 5 Jan 2025 13:49:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084997; cv=none; b=BRrsiwUMJFXoNfp58zjvUG7t3cTXQxVJ14UfJhIUB3gwlR2KSAIF1IipYcBnf12CbQ87PcktYBiAsgu4ph2Ui/afPVMFIioOBHRiNyRL9CinKcMMPZbvLkJZZb/wJjHkbMamosGAn8ZEUZMqMguro388O2rFF2wtij2JABAjNsY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736084997; c=relaxed/simple; bh=bTgRovIqaZN61VnZ5aaBXmpGOS31+O0dH+hWRiP9w+g=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=m5Glz32h+hkE+uiFxKtSBZKJXf0CO8fZC69vFWsnQOlTiBXw02Qck3H82RIkDFO3VCrjmPPcD2beWfILaESTQq6sQ3NgKsQ/MtDhpatsjDqIPA2Xl/0sCDoY5gMYb7Veekk6blSu8MsFfhH80k6vbPMunNI56y9rwMlXDxzrH54= 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=XxHkaZ7Z; arc=none smtp.client-ip=209.85.214.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="XxHkaZ7Z" Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-21628b3fe7dso186218445ad.3 for ; Sun, 05 Jan 2025 05:49:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736084995; x=1736689795; 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=ZAHbQzHtWCo0OMNmKvqoMEqGnVC1+2onyD119bGj7dg=; b=XxHkaZ7ZIa1YMIXPWQIlozv/floQ1HFT7A0GM8FH+49gR11kaVb6Q1sQP6jMT6I47J WAs0WbF9sxDz6mg5XByQjAKYVK9QeAPaSwkyS0pFHvuymGGq2F8HLdgoyNBNQ27XOjls Sn2fGfYkaw78UQyPQ202/ywtgtRRaRjDzwAsVRJz1XM+B56dBWhgm5y+/nBrv6fSBZ8L ytOeQbMFm41aK2h5N7EvIrO+btFay+4KjLTW/pJMjWZK97XTPM+7+JSYT8LVk7jQ4BiK YKA3tl4OVO8s407sPBWmR7RCiYRjfj3eRQnhIisUY+5QbF7VS/n4N+yX7sHpq5ByyQDi ukWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736084995; x=1736689795; 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=ZAHbQzHtWCo0OMNmKvqoMEqGnVC1+2onyD119bGj7dg=; b=fU8B9cdgxAZFTqtAhHUJhXHzX5cvndEQrPD9Pwk1vc6evRvDt5Rk7ILbnvPVUXoRxS A7Ypujc2reb4kePePawtq6Om7QyvFdWbZxKrH+TgAAHzR4RQE/QoiP/bqAX9Xpbcrtvw sAHfAbKC7dqMCXarutVXSPzYjt10ZUEzT3nbVSwWoDOVEF1iC/rH3mioYBwx77FYbYdW vkcLxBm7d+KAGQftuEX+JWdfJy2pCJEyPu7ZLtkdaXiQgQGZ5xa45KsoU0vXkmimidDn rtio7hLvkfzGja+xJnHFlMalwEDqLuna1pp2RiieAoY9i8SkhIngZfQ9Q80QgZti4R0d g15g== X-Gm-Message-State: AOJu0Yyd3uPu+YJnvjOVHXyJJXx8T2BbwNOEnLFPgvKskc1Rdzm3Op8P NgvC21ee27tsed1X4uRiczFii+TAHdh2DeAWQ+fOVoVLRcLcd6QcMp1zEA== X-Gm-Gg: ASbGnctnIpIaYA9WgWtUP34r0oJVdtdrGzrE97V3sUZ5kk5sGJSr6KMPCKw3CVJXhXM Shy72dbgqjhLsP4FBjl8q6ctGVDq/ZseZHJkNhvtAZmkKNrBCVZWpb8eZFLGCBSZ42D2GYEQhQ8 hvap7P17WkBIG5M7aiI9q8+HTyB8MwDxCZMFcM22WI6bXAqVGhgmK11MPq4Tf6ROMiuI/wc3bXA Iue3QbVs7v2s1H3qQw/3JS1RcTcTb9M7ak= X-Google-Smtp-Source: AGHT+IEI6bSUD9xiz6JqYGaHeNU5GrE1I04Yg2/uLpQClSD3YJIlyLh4spb0TDqxsEsONlUHbkCNHw== X-Received: by 2002:aa7:88cc:0:b0:725:df1a:285 with SMTP id d2e1a72fcca58-72abddff55cmr75574440b3a.12.1736084995012; Sun, 05 Jan 2025 05:49:55 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72aad81617csm29519100b3a.39.2025.01.05.05.49.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:49:54 -0800 (PST) Date: Sun, 5 Jan 2025 21:49:51 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 05/10] packed-backend: check whether the refname contains NULL binaries 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 implemented the header consistency check for the raw "packed-refs" file. Before we implement the consistency check for each ref entry, let's analysis [1] which reports that "git fsck" cannot detect some binary zeros. "packed-backend.c::next_record" will use "check_refname_format" to check the consistency of the refname. If it is not OK, the program will die. So, we already have the code path and we must miss out something. We use the following code to get the refname: strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf In the above code, `p` is the start pointer of the refname and `eol` is the next newline pointer. We calculate the length of the refname by subtracting the two pointers. Then we add the memory range between `p` and `eol` to get the refname. However, if there are some NULL binaries in the memory range between `p` and `eol`, we will see the refname as a valid ref name as long as the memory range between `p` and the first occurred NULL binary is valid. In order to catch above corruption, create a new function "refname_contains_null" by checking whether the "refname.len" equals to the length of the raw string pointer "refname.buf". If not equal, there must be some NULL binaries in the refname. Use this function in "next_record" function to die the program if "refname_contains_null" returns true. [1] https://lore.kernel.org/git/6cfee0e4-3285-4f18-91ff-d097da9de737@rd10.de/ Reported-by: R. Diez Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 3b11abe5f8..f6142a4402 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -493,6 +493,23 @@ static void verify_buffer_safe(struct snapshot *snapshot) last_line, eof - last_line); } +/* + * When parsing the "packed-refs" file, we will parse it line by line. + * Because we know the start pointer of the refname and the next + * newline pointer, we could calculate the length of the refname by + * subtracting the two pointers. However, there is a corner case where + * the refname contains corrupted embedded NULL binaries. And + * `check_refname_format()` will not catch this when the truncated + * refname is still a valid refname. To prevent this, we need to check + * whether the refname contains the NULL binaries. + */ +static int refname_contains_null(struct strbuf refname) +{ + if (refname.len != strlen(refname.buf)) + return 1; + return 0; +} + #define SMALL_FILE_SIZE (32*1024) /* @@ -894,6 +911,9 @@ static int next_record(struct packed_ref_iterator *iter) strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf; + if (refname_contains_null(iter->refname_buf)) + die("packed refname contains embedded NULL: %s", iter->base.refname); + if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) { if (!refname_is_safe(iter->base.refname)) die("packed refname is dangerous: %s", From patchwork Sun Jan 5 13:49:59 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926526 Received: from mail-pj1-f54.google.com (mail-pj1-f54.google.com [209.85.216.54]) (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 B8313AD2C for ; Sun, 5 Jan 2025 13:50:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085008; cv=none; b=e0Ak30NGox8x9f+XUButBpyPSdn3ALXASUY4fyyQerdkOLsZjI1B/trtbYyYzYuH0a4O+B8575M90RIq7PgZbTL/3C1KSXB9aZIvR8HxtK+8W4eN5ORJQaAUBAXgsLn+qaz40167WOfRRN7iuu6Ec+vPGqzEAVplHoQMUPBFS7I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085008; c=relaxed/simple; bh=N3JZ9gZIy3ZYKhXHN5BwF0X0C+Uv0ODbUJ9UhSBHZk0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=FALRvKlY8s+sBKqdi/mldTgyHDo8PIiJSDiZok5zWjksXVehdm92Lbw9EmnPi+bhVoGrf97t0g/bh1fSRTz0yx9QCl6EoRciFZvy9t0ERy4bnbDd0iMwcKYVB16BF1Ah/q82gbq3xAdmBtr3DFgVGbpidagjuKWCOMAsT1a83SY= 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=im/+jnPm; arc=none smtp.client-ip=209.85.216.54 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="im/+jnPm" Received: by mail-pj1-f54.google.com with SMTP id 98e67ed59e1d1-2ef6c56032eso14281760a91.2 for ; Sun, 05 Jan 2025 05:50:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736085004; x=1736689804; 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=XrRLwEWQRzDCNpfcICQfK4NfVFOVApWxmXNVi2/Tn5A=; b=im/+jnPm45QW/pM4x8J6vdFqiqvK9euQt2L/v7wMx7z8kOFqDu4EhiG58eDT9RuapD 5oyVyEOzAdCLprwGr7Ss+nrngrEC1w/7UQQmXNhWUz5O3I3M06b8en48e0k2mAwdph3p GqYqDRR1h/W5t4E8XFE8LOlv3g6Yd8Ydb9cPfwIxMX+HOyVCc8W03znk9VlFRuw3u5ad AdyzuzFhnGOuLFsaOZFlQHuGVupWD33rZXiYozacLlBwHJsrVYexPqs4IKTVWULRFLGJ 3xpiFYpysuy8t24eHM/fP/7g4opvVWB7G0ksETUVQp1el5JCQS9NN74oBYib0KWZEI4e g6fQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736085004; x=1736689804; 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=XrRLwEWQRzDCNpfcICQfK4NfVFOVApWxmXNVi2/Tn5A=; b=u/Sradr/CDB6khcuWRdkpSa+GZgSoBkjdgButlDJJ+HNRHlwsIbT/7sSovbha6QxlE IO3PI0NA7w2q/vzlixOQVpAesejPatrZJTXVO07bfR6ErnPYmlpT0cGi/6G3WDKyZl3p YYCT8XLGxiAIW/r/S/3BU7X/LZwTqWNsgFLxMxPNUYmkWH/JDo7b3NtOlMCrlW4S8F5N lCFepfOQd7/bwac8BzgVvEHWgE0I7oAI8YsyD4HWZf7I+2dK9g/hZxeoMXZLcW235Cjx feK3Y1W9Y2+zNtt2d1Xn6u8njS84mf8U5apIupoq9sifwdZzSFMU9SNH/sxRLa+b/1GA nOkA== X-Gm-Message-State: AOJu0YwHd7SfutrStenkBjEj/fh3f3tpm3+hOTlkgILN5yPsDNpy+XkK xgIku/ncBm9LBBmG4y5339Zg0W1nzlrbyyss878iKeTcux50y78eBhdSPw== X-Gm-Gg: ASbGncuHxUnkyzSpYZ15bWt5bz/3XQMgcWASBbxqLeCx+zi+qHh94G8AfnniE6r28XS YP7rDc4Vp2sgoTiRew8bU12EqskFYgk15ELxbS5Z6yeQbWERDFnHd7/TbjjxiiueaZcq2R4YpTo i5UyTWDnqzv4gru30jFQ2IANpC1pXWiLf/TexpezeS2XmSjruWW83pXz32UcymjtYdDZN3T9od/ Cb+a5xeQIMwFuLPwp0HRdNIKbMmP+l37eE= X-Google-Smtp-Source: AGHT+IGJK+Ss79dkjKxStjzbPMccKL7+P9PktVURccumjRo00ckRigKB0w0mrzSfsEJEneYTkMjXcQ== X-Received: by 2002:a17:90b:2745:b0:2ee:3cc1:793a with SMTP id 98e67ed59e1d1-2f452ec37aemr84079291a91.29.1736085004171; Sun, 05 Jan 2025 05:50:04 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dc9f4fbcsm275786505ad.174.2025.01.05.05.50.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:50:03 -0800 (PST) Date: Sun, 5 Jan 2025 21:49:59 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 06/10] packed-backend: add "packed-refs" entry consistency check 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: "packed-backend.c::next_record" will parse the ref entry to check the consistency. This function has already checked the following things: 1. Parse the main line of the ref entry, if the oid is not correct. It will die the program. And then it will check whether the next character of the oid is space. Then it will check whether the refname is correct. 2. If the next line starts with '^', it will continue to parse the oid of the peeled oid content and check whether the last character is '\n'. We can iterate each line by using the "packed_fsck_ref_unterminated_line" function. Then, create a new fsck message "badPackedRefEntry(ERROR)" to report to the user when something is wrong. Create two new functions "packed_fsck_ref_main_line" and "packed_fsck_ref_peeled_line" for case 1 and case 2 respectively. Last, update the unit test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 + fsck.h | 1 + refs/packed-backend.c | 105 +++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 40 +++++++++++++ 4 files changed, 148 insertions(+), 1 deletion(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 34375a3143..2a7ec7592e 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,9 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefEntry`:: + (ERROR) The "packed-refs" file contains an invalid entry. + `badPackedRefHeader`:: (ERROR) The "packed-refs" file contains an invalid header. diff --git a/fsck.h b/fsck.h index 026ad1d537..4fca304b72 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_ENTRY, ERROR) \ FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index f6142a4402..6e521a9f87 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1822,7 +1822,96 @@ static int packed_fsck_ref_header(struct fsck_options *o, const char *start, con return 0; } +static int packed_fsck_ref_peeled_line(struct fsck_options *o, + struct ref_store *ref_store, int line_number, + const char *start, const char *eol) +{ + struct strbuf peeled_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct object_id peeled; + const char *p; + int ret = 0; + + strbuf_addf(&peeled_entry, "packed-refs line %d", line_number); + report.path = peeled_entry.buf; + + start++; + if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid peeled oid", + (int)(eol - start), start); + goto cleanup; + } + + if (p != eol) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has trailing garbage after peeled oid '%.*s'", + (int)(eol - p), p); + goto cleanup; + } + +cleanup: + strbuf_release(&peeled_entry); + return ret; +} + +static int packed_fsck_ref_main_line(struct fsck_options *o, + struct ref_store *ref_store, int line_number, + const char *start, const char *eol) +{ + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct strbuf refname = STRBUF_INIT; + struct object_id oid; + const char *p; + int ret = 0; + + strbuf_addf(&packed_entry, "packed-refs line %d", line_number); + report.path = packed_entry.buf; + + if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid oid", + (int)(eol - start), start); + goto cleanup; + } + + if (p == eol || !isspace(*p)) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has no space after oid '%s' but with '%.*s'", + oid_to_hex(&oid), (int)(eol - p), p); + goto cleanup; + } + + p++; + strbuf_add(&refname, p, eol - p); + if (refname_contains_null(refname)) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "refname '%s' contains NULL binaries", + refname.buf); + goto cleanup; + } + + if (check_refname_format(refname.buf, 0)) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_NAME, + "has bad refname '%s'", refname.buf); + goto cleanup; + } + +cleanup: + strbuf_release(&packed_entry); + strbuf_release(&refname); + return ret; +} + static int packed_fsck_ref_content(struct fsck_options *o, + struct ref_store *ref_store, const char *start, const char *eof) { int line_number = 1; @@ -1844,6 +1933,20 @@ static int packed_fsck_ref_content(struct fsck_options *o, "missing header line"); } + while (start < eof) { + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + ret |= packed_fsck_ref_main_line(o, ref_store, line_number, start, eol); + start = eol + 1; + line_number++; + if (start < eof && *start == '^') { + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + ret |= packed_fsck_ref_peeled_line(o, ref_store, line_number, + start, eol); + start = eol + 1; + line_number++; + } + } + /* * If there is anything wrong during the parsing of the "packed-refs" * file, we should not check the object of the refs. @@ -1900,7 +2003,7 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } - ret = packed_fsck_ref_content(o, packed_ref_content.buf, + ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf, packed_ref_content.buf + packed_ref_content.len); cleanup: diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 6c729e749a..7e8b329425 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -690,4 +690,44 @@ test_expect_success 'packed-refs header should be checked' ' done ' +test_expect_success 'packed-refs content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + git tag -a annotated-tag-2 -m tag-2 && + + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_2_oid=$(git rev-parse annotated-tag-2) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + tag_2_peeled_oid=$(git rev-parse annotated-tag-2^{}) && + short_oid=$(printf "%s" $tag_1_peeled_oid | cut -c 1-4) && + + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s\n" "$short_oid refs/heads/branch-1" >>.git/packed-refs && + printf "%sx\n" "$branch_1_oid" >>.git/packed-refs && + printf "%s refs/heads/bad-branch\n" "$branch_2_oid" >>.git/packed-refs && + printf "%s refs/heads/branch.\n" "$branch_2_oid" >>.git/packed-refs && + printf "%s refs/tags/annotated-tag-3\n" "$tag_1_oid" >>.git/packed-refs && + printf "^%s\n" "$short_oid" >>.git/packed-refs && + printf "%s refs/tags/annotated-tag-4.\n" "$tag_2_oid" >>.git/packed-refs && + printf "^%s garbage\n" "$tag_2_peeled_oid" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: badPackedRefEntry: '\''$short_oid refs/heads/branch-1'\'' has invalid oid + error: packed-refs line 3: badPackedRefEntry: has no space after oid '\''$branch_1_oid'\'' but with '\''x'\'' + error: packed-refs line 4: badRefName: has bad refname '\'' refs/heads/bad-branch'\'' + error: packed-refs line 5: badRefName: has bad refname '\''refs/heads/branch.'\'' + error: packed-refs line 7: badPackedRefEntry: '\''$short_oid'\'' has invalid peeled oid + error: packed-refs line 8: badRefName: has bad refname '\''refs/tags/annotated-tag-4.'\'' + error: packed-refs line 9: badPackedRefEntry: has trailing garbage after peeled oid '\'' garbage'\'' + EOF + test_cmp expect err +' + test_done From patchwork Sun Jan 5 13:50:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926527 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) (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 617D8144D1A for ; Sun, 5 Jan 2025 13:50:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.48 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085018; cv=none; b=XdtBNCnkWHHpWpLR42CaLwoGHgpQqnA1x260aHP7kMdb1WYXIC0+8y6fM3I0Vjw7BvQuMea9eO+ew+Z5XMa1UZiVD9ZLx9K0M9KhIFpznMRmPh/qddNC2Br+w0B6C6Qs0hSKLdVnJH4T8NEEakAIaG51UG9rlZ7ZOeIMz1NZfIE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085018; c=relaxed/simple; bh=/1wk9FVuDhAceWUi1iD+Cb6+KsC98czVFNF7l8Xiiok=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pSPYd6/aNWeAU4kuG4ccrMICq43VFPhru5bwX7BrNlBhr/qkrSf+nm/kLgGcW7+vmGpEMDJg77Fh98ouk+lpeLWSOy9UhGRxXhd7JcWzw8xg9NXtHkefyOIGWGWGZk0Ky+SS9eyncgxUJRmVsvH835w3VhIzVBHVlzRcQSVLWcc= 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=ag7zefcX; arc=none smtp.client-ip=209.85.216.48 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="ag7zefcX" Received: by mail-pj1-f48.google.com with SMTP id 98e67ed59e1d1-2ee67e9287fso19491904a91.0 for ; Sun, 05 Jan 2025 05:50:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736085015; x=1736689815; 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=pmTV6X5dSaUAm3v/sYENUmrKOuFogU3fE/8gcUcu23w=; b=ag7zefcXzl0vTrQSu0gkYZOgSgiYcYqBfBbWr4NJgX+bgRx/IP9xLQXNuAXPp8HDxL 2YjijhpEAYmo5dkDly+qc86mkJBX0r0x5YTA6aqEDPIz4iR4VE9qZ5Tkn3N2ixpKx+7Y W9WyZ7Eif28EYqxUidE6w1Dm+iVqKSVNkbTdtTGuGOh/P7nIBDlztLifCviMSkQjgmYB GtkzEFVaWfBeeYkiG8MPOGUPbE3iF37SiL5om7e2x02+bHhicdH3x6ZWbsrcHMTKqlvD azKRGbXbI6W7axRbVLARqfEkGfuFfrYAiJurGJ/BN8OvfDcKCXqSepZUxlLP5bpr6/sw MMSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736085015; x=1736689815; 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=pmTV6X5dSaUAm3v/sYENUmrKOuFogU3fE/8gcUcu23w=; b=RRuIBpcXcPMwjwvzIkgxSOAVmOw7JsHWBO43gcGByPR6OacRvulhTKuRA01RNLHCKx v8wPPWDUV6Ta5oAsKVHdld08YZac11eHpi3LuAb19+jOTYRcDZKVuvyZ5jgOxyOxA/yQ l6m+hnRTgetuEPjlROyS827IisRegcleOBK0V1oUZf4aU9nG+w2kY5wmlk5jQoCq4BOP 1M3kIQXJ6dwsbhQXFOtRrnvATnM4nxjRA3Y8fBR4Y4v91BXr4Mmi/1xYsVDOYeo5CY16 J+i8znSgcYMsPNF9IbszyStygOeWGhT7wjMH3MdC90qC/Y+Ti0woAkaKi3x2PRM3MEOQ gsZQ== X-Gm-Message-State: AOJu0YymBh4mCbnVwafa2hgvhg64lvrsdHgl7gzS5sIT7kxlT07FHpQ0 EbcfGZtweE9q8vl1DSEbeSSIfY0SvcHu++sPID6wR4lN5rVzP9KVTcAYVA== X-Gm-Gg: ASbGncvA44Y7s7VS6ojbqGdt/alCujJca0h2XlJNBPK2xO8AdnkfwqlaBl2hr9+21GK rpyTj+jHxWTb70g8Zee+2zK3VYQkotURoNMUFZLs7dYu9emy/5Yw489vIBYXfueO8EEd33OsSaa G1E4Tlo9hgiFPlKAzbk6kizodF9Ri8TgwoZVZ9+thqtPrK26v2Asjr065+4dFNbip/AAgg19c5R vg6CUJh85aiMl1M6T9dJAVeYEv0XE4Cnho= X-Google-Smtp-Source: AGHT+IHGhVhi68Kx7tSPgLohZiXveClTvFeBfDAT4B6s+usxt3o77iXCXVC6/qVedsLZkGBCq/tOgw== X-Received: by 2002:a17:90b:2e4a:b0:2ee:fa0c:cebc with SMTP id 98e67ed59e1d1-2f452e38d0emr85734856a91.20.1736085014871; Sun, 05 Jan 2025 05:50:14 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dc962e0csm275631005ad.22.2025.01.05.05.50.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:50:14 -0800 (PST) Date: Sun, 5 Jan 2025 21:50:10 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 07/10] packed-backend: create "fsck_packed_ref_entry" to store parsing info 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 check whether the oid hash is correct by using `parse_oid_hex_algop`. However, we doesn't check whether the object exists. It may seem that we could do this when we are parsing the raw "packed-refs" file. But this is impossible. Let's analysis why. We will use "parse_object" function to get the "struct object". However, this function will eventually call the "create_snapshot" and "next_record" function in "packed-backend.c". If there is anything wrong, it will die the program. And we don't want to die the program during the check. So, we should store the information in the parsing process. And if there is nothing wrong in the parsing process, we could continue to check things. So, create "fsck_packed_ref_entry" to do this. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 56 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6e521a9f87..7386e6bfce 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1768,6 +1768,29 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +struct fsck_packed_ref_entry { + int line_number; + + int has_peeled; + struct object_id oid; + struct object_id peeled; +}; + +static struct fsck_packed_ref_entry *create_fsck_packed_ref_entry(int line_number) +{ + struct fsck_packed_ref_entry *entry = xcalloc(1, sizeof(*entry)); + entry->line_number = line_number; + entry->has_peeled = 0; + return entry; +} + +static void free_fsck_packed_ref_entries(struct fsck_packed_ref_entry **entries, int nr) +{ + for (int i = 0; i < nr; i++) + free(entries[i]); + free(entries); +} + static int packed_fsck_ref_next_line(struct fsck_options *o, int line_number, const char *start, const char *eof, const char **eol) @@ -1823,20 +1846,20 @@ static int packed_fsck_ref_header(struct fsck_options *o, const char *start, con } static int packed_fsck_ref_peeled_line(struct fsck_options *o, - struct ref_store *ref_store, int line_number, + struct ref_store *ref_store, + struct fsck_packed_ref_entry *entry, const char *start, const char *eol) { struct strbuf peeled_entry = STRBUF_INIT; struct fsck_ref_report report = { 0 }; - struct object_id peeled; const char *p; int ret = 0; - strbuf_addf(&peeled_entry, "packed-refs line %d", line_number); + strbuf_addf(&peeled_entry, "packed-refs line %d", entry->line_number + 1); report.path = peeled_entry.buf; start++; - if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) { + if (parse_oid_hex_algop(start, &entry->peeled, &p, ref_store->repo->hash_algo)) { ret |= fsck_report_ref(o, &report, FSCK_MSG_BAD_PACKED_REF_ENTRY, "'%.*s' has invalid peeled oid", @@ -1858,20 +1881,20 @@ static int packed_fsck_ref_peeled_line(struct fsck_options *o, } static int packed_fsck_ref_main_line(struct fsck_options *o, - struct ref_store *ref_store, int line_number, + struct ref_store *ref_store, + struct fsck_packed_ref_entry *entry, const char *start, const char *eol) { struct strbuf packed_entry = STRBUF_INIT; struct fsck_ref_report report = { 0 }; struct strbuf refname = STRBUF_INIT; - struct object_id oid; const char *p; int ret = 0; - strbuf_addf(&packed_entry, "packed-refs line %d", line_number); + strbuf_addf(&packed_entry, "packed-refs line %d", entry->line_number); report.path = packed_entry.buf; - if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) { + if (parse_oid_hex_algop(start, &entry->oid, &p, ref_store->repo->hash_algo)) { ret |= fsck_report_ref(o, &report, FSCK_MSG_BAD_PACKED_REF_ENTRY, "'%.*s' has invalid oid", @@ -1883,7 +1906,7 @@ static int packed_fsck_ref_main_line(struct fsck_options *o, ret |= fsck_report_ref(o, &report, FSCK_MSG_BAD_PACKED_REF_ENTRY, "has no space after oid '%s' but with '%.*s'", - oid_to_hex(&oid), (int)(eol - p), p); + oid_to_hex(&entry->oid), (int)(eol - p), p); goto cleanup; } @@ -1914,7 +1937,10 @@ static int packed_fsck_ref_content(struct fsck_options *o, struct ref_store *ref_store, const char *start, const char *eof) { + struct fsck_packed_ref_entry **entries; + int entry_alloc = 20; int line_number = 1; + int entry_nr = 0; const char *eol; int ret = 0; @@ -1933,14 +1959,21 @@ static int packed_fsck_ref_content(struct fsck_options *o, "missing header line"); } + ALLOC_ARRAY(entries, entry_alloc); while (start < eof) { + struct fsck_packed_ref_entry *entry + = create_fsck_packed_ref_entry(line_number); + ALLOC_GROW(entries, entry_nr + 1, entry_alloc); + entries[entry_nr++] = entry; + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); - ret |= packed_fsck_ref_main_line(o, ref_store, line_number, start, eol); + ret |= packed_fsck_ref_main_line(o, ref_store, entry, start, eol); start = eol + 1; line_number++; if (start < eof && *start == '^') { + entry->has_peeled = 1; ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); - ret |= packed_fsck_ref_peeled_line(o, ref_store, line_number, + ret |= packed_fsck_ref_peeled_line(o, ref_store, entry, start, eol); start = eol + 1; line_number++; @@ -1955,6 +1988,7 @@ static int packed_fsck_ref_content(struct fsck_options *o, o->safe_object_check = 0; + free_fsck_packed_ref_entries(entries, entry_nr); return ret; } From patchwork Sun Jan 5 13:50:19 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926528 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9792129422 for ; Sun, 5 Jan 2025 13:50:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085026; cv=none; b=eu7O6tJX2lCDa3p/OqxTJkEMYei2s/4dnhC20AwUPXLrSWRLUdndxyhnK6Lf5ryieBB9mCQ/W5TfG+zlDVsiHMRPZV7KEdh3cPrPaL88qrwBECMK+WH20jlM1nkTFMsGt7TVKcL9+NflGB6DvqvlaDGC0mfLX907CbrkyUkQKx0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085026; c=relaxed/simple; bh=MGBAbfPXQMkaj71bQvQb3M2OJdVad7Sve0TmRZ99xQ0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lZNrxS9gKxSkeat/S2YAhlqamPmBYx8sLp9yqfjAoLAfTCTzu9k5OhuJOS1NoMHcoB3QJ5bc940PSmBndgohvLGnFG2Y6ORRTOixd32SBUUx3vU0c42vOSmW5emY0GhmdiBDLlAl0Gf4bwb0+7ch4BzQGpKpkHp6TW40mJcc4Bo= 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=hYbZhVhz; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hYbZhVhz" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-2166f1e589cso234091315ad.3 for ; Sun, 05 Jan 2025 05:50:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736085023; x=1736689823; 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=m0olQPNWODyaICpwvuNCYPEwqsltI4AxpVg6U27nxNg=; b=hYbZhVhzpmO2l1T1epdMPA7dWkEayZE7kZjd2gOc0Q+OJPatmB8FLsmPpPaWQPT1x3 L51dYe2Ic8Lvef959gT8uuLnMsiJI1fBK6eT6bWvPNbtg2qGynJCBVsMAb24lY9oLYeS HOTXbA5ciM74NCTCSLYR/aiE17SpPPvIH6u9fHJo5mz1qD19PwR6nnJ2McUK1kp6v7VZ 6G0i6/3DbLvrrBiaMHrOTU4ZrLSb4NgtQz/DrWBpE3yEuovLNYaJ3y2WyRw8/oJ1WkbX 1qciJeX8gbMj8/d44Oz3MdiPABazglBT3r4C2rNHTP6Ou6K5cDy+PqPT24kYHp3Nim8e 5n8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736085023; x=1736689823; 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=m0olQPNWODyaICpwvuNCYPEwqsltI4AxpVg6U27nxNg=; b=dRE0wUjXRNs5WQ/omGWMnyhygcyW0trZFB2QW1539bKE4CGMjiUb40hOfTpWc+fGbX +7esmO7gasnx3eH4AncuGQ2S4D0msoWP2r9r+5EnG2B9uhy5LKHiPh0UXGtO8rwcnie2 anuAHRal+6fvPspGkB6gPsYhJNB06ueUCmp4vJsBbbAOhP3pBR5K2qsMblbqU6Jhmes1 RIoDO3BcFm7q5VRUkhUsU55v1FaQO4rcD2HtplgXHNzxXUKXp7fDjprW+TO0I/gT24mX iQWK9l8miNyrTjN5PwU96qLQDnXuVyqBhLeyKRb3zu++1S5CtBug3wOlDqV34YpeUw5X vLMw== X-Gm-Message-State: AOJu0Yy2UUyI5YSB5Kd6Kjmm+ClQEWQUAzY0fKV43w68rg00f8POAPMV 3c0zB/mcOt55mW0vDb907/z8K3yx+vgMewXwzrZS1cnmAxDlL/p6sV3iZw== X-Gm-Gg: ASbGnct3NFPAxTdpiWjCVH9FmXRJq1m5w1DuTWDECFWYVQtTuVpgR8ZXJv0KfaKEH0g XZmV9Ih5Vv+PAzmCql8rlEYvihO+gTi9D32yWKRJh1N4PDOy4Y583JKcdsZFE0P7/JgDdg9/2ln TQRROrZCgqHQXvThoAUEBRcI0WIe0mnr57BgxNIlpdq/s1DSZBerp2HDTGqrVDPwI/F2R2fFyWb p8i3K7VYlulgDSq1i8UlCtM++cnofwRyeU= X-Google-Smtp-Source: AGHT+IGWjISzYlfGG1eHwSWX5IcVXZ3t6R1eZHbsr/Vws2RPhuwLBWJx46vsvyPYbN6bgP+xw+kM6A== X-Received: by 2002:a05:6a21:3a82:b0:1e1:9f77:da92 with SMTP id adf61e73a8af0-1e5e07f0057mr77917194637.33.1736085023261; Sun, 05 Jan 2025 05:50:23 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72aad8dbb09sm30406177b3a.121.2025.01.05.05.50.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:50:22 -0800 (PST) Date: Sun, 5 Jan 2025 21:50:19 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 08/10] packed-backend: add check for object consistency 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: If there is nothing wrong when parsing the raw file "packed-refs", we could then iterate the "entries" to check the object consistency. There are two kinds of ref entry: one is the normal and another is peeled. For both situations, we need to use "parse_object" function to parse the object id to get the object. If the object does not exist, we will report an error to the user. Create a new function "packed_fsck_ref_oid" to do above then update the unit test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 50 +++++++++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 35 ++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 7386e6bfce..d83ce2838f 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -13,6 +13,7 @@ #include "../iterator.h" #include "../lockfile.h" #include "../chdir-notify.h" +#include "../packfile.h" #include "../statinfo.h" #include "../worktree.h" #include "../wrapper.h" @@ -1933,6 +1934,52 @@ static int packed_fsck_ref_main_line(struct fsck_options *o, return ret; } +static int packed_fsck_ref_oid(struct fsck_options *o, struct ref_store *ref_store, + struct fsck_packed_ref_entry **entries, int nr) +{ + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct object *obj; + int ret = 0; + + for (int i = 0; i < nr; i++) { + struct fsck_packed_ref_entry *entry = entries[i]; + + strbuf_release(&packed_entry); + strbuf_addf(&packed_entry, "packed-refs line %d", entry->line_number); + report.path = packed_entry.buf; + + if (is_promisor_object(ref_store->repo, &entry->oid)) + continue; + + obj = parse_object(ref_store->repo, &entry->oid); + if (!obj) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%s' is not a valid object", + oid_to_hex(&entry->oid)); + } + if (entry->has_peeled) { + strbuf_reset(&packed_entry); + strbuf_addf(&packed_entry, "packed-refs line %d", + entry->line_number + 1); + report.path = packed_entry.buf; + + obj = parse_object(ref_store->repo, &entry->peeled); + if (!obj) { + ret |= fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%s' is not a valid object", + oid_to_hex(&entry->peeled)); + } + } + + } + + strbuf_release(&packed_entry); + return ret; +} + static int packed_fsck_ref_content(struct fsck_options *o, struct ref_store *ref_store, const char *start, const char *eof) @@ -1986,7 +2033,8 @@ static int packed_fsck_ref_content(struct fsck_options *o, */ if (ret) o->safe_object_check = 0; - + else + ret |= packed_fsck_ref_oid(o, ref_store, entries, entry_nr); free_fsck_packed_ref_entries(entries, entry_nr); return ret; diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 7e8b329425..faa7c80356 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -730,4 +730,39 @@ test_expect_success 'packed-refs content should be checked' ' test_cmp expect err ' +test_expect_success 'packed-refs objects should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + cd repo && + test_commit default && + git tag -a annotated-tag-1 -m tag-1 && + + tag_1_oid=$(git rev-parse annotated-tag-1) && + + for non_existing_oid in "$(test_oid 001)" "$(test_oid 002)" + do + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s refs/heads/foo\n" "$non_existing_oid" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: badPackedRefEntry: '\''$non_existing_oid'\'' is not a valid object + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done && + + for non_existing_oid in "$(test_oid 001)" "$(test_oid 002)" + do + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s refs/tags/foo\n" "$tag_1_oid" >>.git/packed-refs && + printf "^$non_existing_oid\n" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 3: badPackedRefEntry: '\''$non_existing_oid'\'' is not a valid object + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done +' + test_done From patchwork Sun Jan 5 13:50:31 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926529 Received: from mail-pj1-f46.google.com (mail-pj1-f46.google.com [209.85.216.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 45E831EA80 for ; Sun, 5 Jan 2025 13:50:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085040; cv=none; b=Ax6lLIe47s14myvNqBsimX7X6Qgfb7sjHjSfrGMJ24r7y4wjKGtENaYEKC7yxMe7PT6h/u8nERLc1wmSDvYpfNLVuPRvpQKF40H7IZZIp2ccyoQnzP4Ceu80apN2/I2vzJyUb5ltmMpPNNWA1yi8885VvRrNIWagTByb8RFD/og= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085040; c=relaxed/simple; bh=VvRxamiKwuKPhhV8sNfkQ8WSiJCGSXgZ5ZL8Rycyloc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=t4AoVVxOuWqx/fZ/BQtQx7X9nlffr3G4FykWKZBoTJqnTEnuSr/tbFScleDMxRG3N0HLb2F9ECH7oS+WaVf5jwfrUVaClMLS/D42bPfn1MiYRuujWakTbxYyMKkT6VXpIMNheprVPkUgU3gS2AAIC2b0bCe7a6jL6nApbPKdlE8= 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=Hnf1TClo; arc=none smtp.client-ip=209.85.216.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Hnf1TClo" Received: by mail-pj1-f46.google.com with SMTP id 98e67ed59e1d1-2ef28f07dbaso15200051a91.2 for ; Sun, 05 Jan 2025 05:50:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736085038; x=1736689838; 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=BxGuNZP1trsHqydU8+fTfXhvbua/VZ9m1ONmU6l+Mwg=; b=Hnf1TCloC/XviX0yhq+d7PM9bSBf6OfJ0pz6xZ+VLEt4MPgmzTIKAF+cdUbnCQe5kY Bq+CE8oTVXXSxzwa+lDcG2KDvmlqtPDE05lZcnhzCVt5Le4dYgwv/ppOQpeaw3ZiZrSn mqNZFHDWs0ngq5DhncYDRdoGzDOpIly9J8Kf933PCVZc95bZWLkTIlTRywr8bValZeV2 HxdhfDyly99ON6K6aUmQsVKJQfVMI646Mvp/rNMFYHpNSldORmmPANK2dUlE9ppNIK9u nRXAZ1Qpdrg7VrkurBZyujQsfwC0d6pesuDGsbypVwwNaUFJt3yss4k+hgRits9aiBbp ZepA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736085038; x=1736689838; 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=BxGuNZP1trsHqydU8+fTfXhvbua/VZ9m1ONmU6l+Mwg=; b=u5qp9MlpbB7sqwRpWSE/fsckK6SOwfYXKuxaGr4eTd7TtLlImALcErxycDVU54Ogla w63EuhfFd2xcRzUgproQIG57odzMYNN9Z77orTHJWtNQwUkIkdxWu3WUe6EfVz7dkdAV Buwt2P08zrqSgJQzgLpjbE0lQVDNoI+kQaZLnykhNEN6FKBsSd1D//QMBuUdQ+lSaizR wpxPAdnA6DOb+wLxk+HxYzYRZDyY11Gm0Ix1sJBKrBvcU42AGWeQOF3kF5VdsTyuqxcF D1g8KjGBJZ39Vg6ZYMJqFDru8mKAk0Hjs9VUMac5FOoNPFBr1jFXgrOgkwl4Bhz/o8An alBg== X-Gm-Message-State: AOJu0YyCy7h13GTnl/c703t4VkBB45D24+p/1STn1UPljzS40ad78m6g hhFLzmvpLH+IXeryhx/zafUDZhvilpxwHkwMqq/fbGjBl4/LYN2DrWsUGw== X-Gm-Gg: ASbGncslJLSjcRh5sJgyEBmKatV0s3hFRB2g8m8IksPiGXcUZoWk9Jq0/MBQeEQfkNr b72PTjntfWkq7QddqQJ0loDR/QtqfLhbvmin5vJMT490ubFPB0+LAO/kk2A5UNbczZ7apfz/ndq vLCzuLzRYH0XUS3sEz4sYcIZHh68bpEpkJNJa+JAZNMWDoVFfY+BjE7T12u9NMNInklktiPaTys 2Cauuei6nMctF+KMbbm4QVa7RPh9nLSfbQ= X-Google-Smtp-Source: AGHT+IFXgTDEMoKqqpty65Z/gTYCIrLl7VWKbDcOHN1+AmTw8Wpap1BDDLP7EfCGvfy9V31ko9oxww== X-Received: by 2002:a17:90b:2d44:b0:2ee:df57:b194 with SMTP id 98e67ed59e1d1-2f452e3c70amr74177858a91.21.1736085038128; Sun, 05 Jan 2025 05:50:38 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2f4be2b1f43sm13987109a91.17.2025.01.05.05.50.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:50:36 -0800 (PST) Date: Sun, 5 Jan 2025 21:50:31 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 09/10] packed-backend: check whether the "packed-refs" is sorted 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 will always try to sort the "packed-refs" increasingly by comparing the refname. So, we should add checks to verify whether the "packed-refs" is sorted. It may seem that we could add a new "struct strbuf refname" into the "struct fsck_packed_ref_entry" and during the parsing process, we could store the refname into the entry and then we could compare later. However, this is not a good design due to the following reasons: 1. Because we need to store the state across the whole checking lifetime, we would consume a lot of memory if there are many entries in the "packed-refs" file. 2. The most important is that we cannot reuse the existing compare functions which cause repetition. So, instead of storing the "struct strbuf", let's use the existing structure "struct snaphost_record". And thus we could use the existing function "cmp_packed_ref_records". However, this function need an extra parameter for "struct snaphost". Extract the common part into a new function "cmp_packed_ref_records" to reuse this function to compare. Then, create a new function "packed_fsck_ref_sorted" to use the new fsck message "packedRefUnsorted(ERROR)" to report to the user. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 ++ fsck.h | 1 + refs/packed-backend.c | 78 ++++++++++++++++++++++++++++++----- t/t0602-reffiles-fsck.sh | 40 ++++++++++++++++++ 4 files changed, 111 insertions(+), 11 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 2a7ec7592e..7a11d35c5e 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -190,6 +190,9 @@ `packedRefMissingHeader`:: (INFO) The "packed-refs" file does not contain the header. +`packedRefUnsorted`:: + (ERROR) The "packed-refs" file is not sorted. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref diff --git a/fsck.h b/fsck.h index 4fca304b72..1be7402eb9 100644 --- a/fsck.h +++ b/fsck.h @@ -56,6 +56,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ + FUNC(PACKED_REF_UNSORTED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index d83ce2838f..df65fec5a5 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -300,14 +300,8 @@ struct snapshot_record { size_t len; }; -static int cmp_packed_ref_records(const void *v1, const void *v2, - void *cb_data) +static int cmp_packed_refname(const char *r1, const char *r2) { - const struct snapshot *snapshot = cb_data; - const struct snapshot_record *e1 = v1, *e2 = v2; - const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1; - const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1; - while (1) { if (*r1 == '\n') return *r2 == '\n' ? 0 : -1; @@ -322,6 +316,17 @@ static int cmp_packed_ref_records(const void *v1, const void *v2, } } +static int cmp_packed_ref_records(const void *v1, const void *v2, + void *cb_data) +{ + const struct snapshot *snapshot = cb_data; + const struct snapshot_record *e1 = v1, *e2 = v2; + const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1; + const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1; + + return cmp_packed_refname(r1, r2); +} + /* * Compare a snapshot record at `rec` to the specified NUL-terminated * refname. @@ -1775,13 +1780,17 @@ struct fsck_packed_ref_entry { int has_peeled; struct object_id oid; struct object_id peeled; + + struct snapshot_record record; }; -static struct fsck_packed_ref_entry *create_fsck_packed_ref_entry(int line_number) +static struct fsck_packed_ref_entry *create_fsck_packed_ref_entry(int line_number, + const char *start) { struct fsck_packed_ref_entry *entry = xcalloc(1, sizeof(*entry)); entry->line_number = line_number; entry->has_peeled = 0; + entry->record.start = start; return entry; } @@ -1980,6 +1989,50 @@ static int packed_fsck_ref_oid(struct fsck_options *o, struct ref_store *ref_sto return ret; } +static int packed_fsck_ref_sorted(struct fsck_options *o, + struct ref_store *ref_store, + struct fsck_packed_ref_entry **entries, + int nr) +{ + size_t hexsz = ref_store->repo->hash_algo->hexsz; + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct strbuf refname1 = STRBUF_INIT; + struct strbuf refname2 = STRBUF_INIT; + int ret = 0; + + for (int i = 1; i < nr; i++) { + const char *r1 = entries[i - 1]->record.start + hexsz + 1; + const char *r2 = entries[i]->record.start + hexsz + 1; + + if (cmp_packed_refname(r1, r2) >= 0) { + const char *err_fmt = + "refname '%s' is not less than next refname '%s'"; + const char *eol; + eol = memchr(entries[i - 1]->record.start, '\n', + entries[i - 1]->record.len); + strbuf_add(&refname1, r1, eol - r1); + eol = memchr(entries[i]->record.start, '\n', + entries[i]->record.len); + strbuf_add(&refname2, r2, eol - r2); + + strbuf_addf(&packed_entry, "packed-refs line %d", + entries[i - 1]->line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_UNSORTED, + err_fmt, refname1.buf, refname2.buf); + goto cleanup; + } + } + +cleanup: + strbuf_release(&packed_entry); + strbuf_release(&refname1); + strbuf_release(&refname2); + return ret; +} + static int packed_fsck_ref_content(struct fsck_options *o, struct ref_store *ref_store, const char *start, const char *eof) @@ -2009,7 +2062,7 @@ static int packed_fsck_ref_content(struct fsck_options *o, ALLOC_ARRAY(entries, entry_alloc); while (start < eof) { struct fsck_packed_ref_entry *entry - = create_fsck_packed_ref_entry(line_number); + = create_fsck_packed_ref_entry(line_number, start); ALLOC_GROW(entries, entry_nr + 1, entry_alloc); entries[entry_nr++] = entry; @@ -2025,16 +2078,19 @@ static int packed_fsck_ref_content(struct fsck_options *o, start = eol + 1; line_number++; } + entry->record.len = start - entry->record.start; } /* * If there is anything wrong during the parsing of the "packed-refs" * file, we should not check the object of the refs. */ - if (ret) + if (ret) { o->safe_object_check = 0; - else + } else { ret |= packed_fsck_ref_oid(o, ref_store, entries, entry_nr); + ret |= packed_fsck_ref_sorted(o, ref_store, entries, entry_nr); + } free_fsck_packed_ref_entries(entries, entry_nr); return ret; diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index faa7c80356..800a19e4e6 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -765,4 +765,44 @@ test_expect_success 'packed-refs objects should be checked' ' done ' +test_expect_success 'packed-ref sorted should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + + refname1="refs/heads/main" && + refname2="refs/heads/foo" && + refname3="refs/tags/foo" && + + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s %s\n" "$branch_2_oid" "$refname1" >>.git/packed-refs && + printf "%s %s\n" "$branch_1_oid" "$refname2" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: packedRefUnsorted: refname '\''$refname1'\'' is not less than next refname '\''$refname2'\'' + EOF + rm .git/packed-refs && + test_cmp expect err && + + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s %s\n" "$tag_1_oid" "$refname3" >>.git/packed-refs && + printf "^%s\n" "$tag_1_peeled_oid" >>.git/packed-refs && + printf "%s %s\n" "$branch_2_oid" "$refname2" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: packedRefUnsorted: refname '\''$refname3'\'' is not less than next refname '\''$refname2'\'' + EOF + rm .git/packed-refs && + test_cmp expect err +' + test_done From patchwork Sun Jan 5 13:50:43 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13926530 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 897DF29422 for ; Sun, 5 Jan 2025 13:50:50 +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=1736085052; cv=none; b=aNQcMyEdZYf7TJu1mHOi4OoAtdCo38GN/tmSETzZ/Ih3ZUXZ1YexX4gYTkAdn6RRnMeQ7dE7ZwdRoX+HRNAzDVX+aGK7s6O6h/ixuQ0fgHUSz9NYuV5+gccxKRcAwwMgzTfoI3GAov9cyfaf2hqbKlAWetH6F+6bTX2P/kBU+rU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736085052; c=relaxed/simple; bh=F4LG85nl+6wBH+9dohPyZffmT0eVuQMfe9EWhPZgz6I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZQ1Vs3WVFdDzaaT8zXmJ+d7VzqYC3jrZAPsV1cl/qo7krl71NuUDrPSEFDUJmszZWRCaKJeujCTSBy3HfLRXfKYnkZtGvKMswCb0zhZuYXJRa3qSQ00gi8pUif59tEVddg73D17BMy67A+okkAJgUchdoWyvnPca3c1gFtjfBhs= 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=gY1piaEE; 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="gY1piaEE" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2f4409fc8fdso16484727a91.1 for ; Sun, 05 Jan 2025 05:50:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736085049; x=1736689849; 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=J5GMzWkDL0xz4uI9h9Baucv8NbKSiIrB9AezjA30CVM=; b=gY1piaEEm1EJNaAnFLhWunuurbovpVlklNwPUeah6VtWsf3JOBVmaQEGAVFnwqcOcU dDiKIsXxwdoX7Oys/qis9HtFZEWCHRo2bn0D3BdTypta2/ZPepDDvlgF2+2WDqgc82xt GL5MXE+ZF9Rn0q5kFnInBV5ZHYuFM13y32pxK8NUlxOoBijFSP8egskkgPeT0haSrP7k a/Tm6Us9omE0exloOcE7eKhjd8a1c5q2ELa8VafcyLaTDomnkBN3AzHlm0tlA0i5FDTB tji1BsZKX7zRUvQ4fPN6OTNDqNGg1ZR11GIIj4bv5j17ejTsGo3n2RFr4sXm337ncmhk ndlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736085049; x=1736689849; 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=J5GMzWkDL0xz4uI9h9Baucv8NbKSiIrB9AezjA30CVM=; b=h8gVbn/gz2QoFpVhmHmlBzNecbLr2xDMO52j6CmQDCWcKFdVgTs4fEtOdfT+hhsYYr v6EkeuwB0H6jM2OU3htSFb55nSRvrn00UmR1fE5bL8hH6lQ6AlB3z7j6tAvLqe2KHoRC PGCyrDuBe5ZXopJmaiV5oJWWQus85lAGUw5MeciEsJwdKyFyb4BzsY08gTOZ2MwETpYz zhK28cZjYMLRI508tZEzDV4jtsHzmvUWC9McOW5CubXeOMWownu/b65zgrK+A/6LoME3 ttEHJW4pxl7YHjwgeT3Z22XmJcbuUIf2t2W2744CrdFCt19TnV5xoUEtoAY8iV0Zyf3h qTkg== X-Gm-Message-State: AOJu0YzR8Zh5ACBVmOEfPWcysXZg/YDHif0qu8uAzuKU59rXVnzG+BuZ ahoAaT31Oc/6RIu5ZaHYmQHwv0E+VoEhPJ/Y6A9fXsImLsrZAqlknA3gjg== X-Gm-Gg: ASbGnctDO+sh3mXaERamzkFCDDobgbCyTe5iI/U39+HP1xRmifMLyCPauy65ZA7+czM 02daYZ07uKB3WEI6GviJlRft699MF0xfA1wex5Q4o/Ga06tfsfJ/d5lcZevibK0XlTL6A4MgFyv QEhV80zUKgS5QOaspv7oy5xIJPk/LyyBLBg7qq5Q0POlQ+Akj46bnSucQmNgAtubXhqEhi562ZQ n6UM+Va+8ZMMcrCJPYDgBujar26GuOn+Oo= X-Google-Smtp-Source: AGHT+IH3crBNkwc9aMnDgSqEM6FElcb56eACZGkoY9Ivd7L8XTbioqVIeXEnV7dPu6i8L0QU+hwMww== X-Received: by 2002:a17:90b:54d0:b0:2f1:2e10:8160 with SMTP id 98e67ed59e1d1-2f4536d193bmr81501211a91.11.1736085049008; Sun, 05 Jan 2025 05:50:49 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2f454f58087sm33227281a91.11.2025.01.05.05.50.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jan 2025 05:50:48 -0800 (PST) Date: Sun, 5 Jan 2025 21:50:43 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH 10/10] builtin/fsck: add `git refs verify` child process 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: At now, we have already implemented the ref consistency checks for both "files-backend" and "packed-backend". Although we would check some redundant things, it won't cause trouble. So, let's integrate it into the "git-fsck(1)" command to get feedback from the users. And also by calling "git refs verify" in "git-fsck(1)", we make sure that the new added checks don't break. Introduce a new function "fsck_refs" that initializes and runs a child process to execute the "git refs verify" command. In order to provide the user interface create a progress which makes the total task be 1. It's hard to know how many loose refs we will check now. We might improve this later. And we run this function in the first execution sequence of "git-fsck(1)" because we don't want the existing code of "git-fsck(1)" which implicitly checks the consistency of refs to die the program. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/fsck.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/builtin/fsck.c b/builtin/fsck.c index 0196c54eb6..a10e52b601 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -902,6 +902,32 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress) return res; } +static void fsck_refs(void) +{ + struct child_process refs_verify = CHILD_PROCESS_INIT; + struct progress *progress = NULL; + + if (show_progress) + progress = start_progress(_("Checking ref database"), 1); + + if (verbose) + fprintf_ln(stderr, _("Checking ref database")); + + child_process_init(&refs_verify); + refs_verify.git_cmd = 1; + strvec_pushl(&refs_verify.args, "refs", "verify", NULL); + if (verbose) + strvec_push(&refs_verify.args, "--verbose"); + if (check_strict) + strvec_push(&refs_verify.args, "--strict"); + + if (run_command(&refs_verify)) + errors_found |= ERROR_REFS; + + display_progress(progress, 1); + stop_progress(&progress); +} + static char const * const fsck_usage[] = { N_("git fsck [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]\n" " [--[no-]full] [--strict] [--verbose] [--lost-found]\n" @@ -967,6 +993,8 @@ int cmd_fsck(int argc, git_config(git_fsck_config, &fsck_obj_options); prepare_repo_settings(the_repository); + fsck_refs(); + if (connectivity_only) { for_each_loose_object(mark_loose_for_connectivity, NULL, 0); for_each_packed_object(the_repository,