From patchwork Tue Aug 27 16:07:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13779774 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 66E3A64A for ; Tue, 27 Aug 2024 16:07:00 +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=1724774821; cv=none; b=TjIGbQsWvUQ7dMvMe6WQRFAKmUXovzaFeEN1Fx6kcfox1FtMqVmoAzpufdpl30bME07Sh5ax/73wB+FhNnxqf3cEXSuE+E1rU0uqaY/7XQj4o6/TcT01KlkSlLDWqDYyJDmatjA7FN3SR/0Q6VzZeBAGpX0FJWeVP0AgDCpMAKg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724774821; c=relaxed/simple; bh=V4DDs/hClwAYYIhD/5WHCWPBJlyb6/CZ1ijOn8MqQ6I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uEaKdoCF2BUVTwEB7omBWkllFLZzP5/8jvpgBGAszyrtxi5evLrhapUZF3Itk8LEgUH6hEtg8GzyA9WEtDXZrrjU0iLGJhZzVtwG0n3RqqEY4ins66LPZbXEq/lDNT3c5DIZSw9epWqpzlRANTN6X217Xgyo3J8bEo63KjyHVAE= 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=TUW3tvpv; 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="TUW3tvpv" Received: by mail-pj1-f48.google.com with SMTP id 98e67ed59e1d1-2d3eda6603cso4547919a91.3 for ; Tue, 27 Aug 2024 09:07:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724774819; x=1725379619; 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=i8uyMImbvTYD4N/MAG3h6aPl+rkH5Fvu9BeFpyy8WmA=; b=TUW3tvpvX/HXx7avQ7vYBI6QQpCcx0u4RoZWaN090vbWwkrxB7SiSdNR2WkAB1X9Zd BeQj2PX6b8wDcVxsS9xDvnC/gK5h1docTwnIGpBIC/g6ETwFJhErnjgM2kVchbLFrwfA Kip10g0ybYN/O8rTlSvd/RC5+NkHgqmOl44e1BDBQYDoAC/hdj3SVpZWBIu8mxKN5ZxX lN9c4qXCp0Onbsm4jfUwMcDbqE07TEoJnSO97aN4HlEjhr7+W0up3f49hk5xrxPvfUjt JJr0/hU8I8KPs8AbS/NIft+xP+CiQRGIaW9wtnDIPDV+H+AQ6sLVkzt5EOXP0NDrvY8z gKdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724774819; x=1725379619; 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=i8uyMImbvTYD4N/MAG3h6aPl+rkH5Fvu9BeFpyy8WmA=; b=X4l2wsDNOY8zxmaHRZ50eP1v5j2W5VfWZKKVCjaqDpdBKGzANvSl8zHvaP5SjbVVZy ap+WwvqXW3oiEBgt0sUL7Knv6R3FctLSBMOqCgbAn2HW75a7XpS9UOkehVQPsmj3hnF6 TWnyHmIlwiWSyFZr8IOwdjeXq5hwckUP68TE4EHtw1xFPYg2OR3Eg/CmU6/sGMzS664G OcSKWnXaLnRkzrfz8U6+SZPjmiJE81bQXkDCLSWD/t7RfnPZVqxO3j52gi8Tca/nxNUn 3OuYn3KrZc1s0tPQ2egMO6b7/F90fhXiGRi5uWsLSljRIHSjJCv4Vw2rWdbbLHs7fZiF pR/g== X-Gm-Message-State: AOJu0YwhfEay+xaMehbRNbdLkggepL9NjcgMsQkHavQtwd1NVUYOHxzc zfDLAC1QhklyPiR910cv+oI+0ktgKl+HeBYEkYJ4RiVJGsh8WjDWy10Skw== X-Google-Smtp-Source: AGHT+IEP9kI3iJoyiSc5v/D4987kKR5ts31vPi1aszBQe90dF1rXLNilduKBqubjD7fLm3iZaSFG2g== X-Received: by 2002:a17:90a:2c49:b0:2c9:cbdd:acd with SMTP id 98e67ed59e1d1-2d8259ea0f2mr3774267a91.35.1724774819114; Tue, 27 Aug 2024 09:06:59 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2d5ebbef1edsm14680598a91.47.2024.08.27.09.06.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Aug 2024 09:06:58 -0700 (PDT) Date: Wed, 28 Aug 2024 00:07:50 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v2 1/4] ref: initialize "fsck_ref_report" with zero Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "fsck.c::fsck_refs_error_function", we need to tell whether "oid" and "referent" is NULL. So, we need to always initialize these parameters to NULL instead of letting them point to anywhere when creating a new "fsck_ref_report" structure. The original code explicitly specifies the ".path" field to initialize the "fsck_ref_report" structure. However, it introduces confusion how we initialize the other fields. In order to avoid this, initialize the "fsck_ref_report" with zero to make clear that everything in "fsck_ref_report" is zero initialized. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 8d6ec9458d..d6fc3bd67c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3446,7 +3446,7 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, goto cleanup; if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) { - struct fsck_ref_report report = { .path = NULL }; + struct fsck_ref_report report = {0}; strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); report.path = sb.buf; From patchwork Tue Aug 27 16:07:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13779775 Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.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 F15F91BFE00 for ; Tue, 27 Aug 2024 16:07:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724774831; cv=none; b=He+PLlplbLyulr3rUnghCaDVkLjhyF+QGxR1CdRNd+SxBLWbUbHaEn0sWKC67xsY46Z2wR0TO3q+3rEJBGWJvCcaUJSrfmi0g1fcg5YZ7fALWohovzULtJMzq5eiZXlnKTOZCnUPqsADbsqBzp0wMnq1DIWkjIH4POQlPDjTjJg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724774831; c=relaxed/simple; bh=TDvfFTLf3vQWEaPKHpXQqc2VPvyqeFVL9HqH0H0Qjqo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=eu15PL84Yuiv8mW9NnKMQG1T2mrrrYYXM2p2WEa/Sgpf1Ui5u4ihU2Ax7VjLOyihxp9r0aat5/iMWqeuSZwyhM7TE3BseYClmZuVXmGp3lVAlZWVLYhmReNmAxbnIk3EiFHqKprRl0VQtX6/Ib5eTpRAPDs4HCZp50xSfQreYsk= 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=bcic3GR/; arc=none smtp.client-ip=209.85.210.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="bcic3GR/" Received: by mail-pf1-f181.google.com with SMTP id d2e1a72fcca58-70cec4aa1e4so3956863b3a.1 for ; Tue, 27 Aug 2024 09:07:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724774829; x=1725379629; 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=L4GVbGUDr/SIkIAzGDiiG/B9lqyJg1hN0ces4Q/fxqk=; b=bcic3GR/+4Pa5h5UEvipOxXdsTYpJCO74dr5ohuej9Ry80ATIq5A9zaF79nxLGESO7 058S4EZW/TCgnni5yTnYUfSmvTT1Iti8ifiJM8q7m04pn/4m87OxRBBucw9ltxfo0IyB t7i4/sOeHQwvNvPi8ovytwlZAzz5gDJQNKFP3Z3bCbJOdUlGNLrAS37t7sYU3+ixT422 WautNryMaKBC6X7aDs5WLkBnvrgkuDCP1VO0dU08SgVnd3lp+RKZsOh4u+MlnXkloLNU 8pNlFEgWX1IrtnBYenGGeKf52NtVHYkYZ632nBUlbSFKIw705mPzwAcWD5K0UWUy9tZs dUoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724774829; x=1725379629; 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=L4GVbGUDr/SIkIAzGDiiG/B9lqyJg1hN0ces4Q/fxqk=; b=XBFsojYT41k/LIREqOcAsue2KDU/837CbOfPMy4Cuba118XN6otJT0a0gwmcZ6TYJ9 52xEqOcJ1r74F4NnwgQlBL8zZCnUf/JZ+x1LZoDT4AY57Cz2VTI3IBLfYouVk5XSfd02 6uf4uz1pZIcHHJiojlGSmBfHQxl90nL/stO5PxlymVMyHE78qC0fQ+n1l8AY6Rz4NQKt xzL6TFgzpVv0lboBvcOKxJU9sPM7gFvUM158Rsjncgd8oLnFBolDyUpfKJ8uuCx5vAQW f3gnxrv9F4DWgrLZSupdaQ+TEe+R7wou6jphUWCYpx59lebGGw5w2d3co9WyjLMI3LFZ wfGg== X-Gm-Message-State: AOJu0YxEsshTnXdnkmDkb/lt4lfNQwDkh2PzisoXlBB3d5s+LP+LqKQQ Cx6w54eZ1COPxvDP6eVDT3dnp1gHVmvXwzy9IcNyWS3SuJuZX4L4mZeYDw== X-Google-Smtp-Source: AGHT+IFYBiGogy3OftT1Ob40M4zbzXjZ2h+YtpSQJF6/Q/3ffkqMuJa3K0LP4gQs5fTjB4Pk0xjL2A== X-Received: by 2002:a05:6a21:1191:b0:1c0:f5fa:cbe6 with SMTP id adf61e73a8af0-1cc89d7dca6mr15296150637.22.1724774828441; Tue, 27 Aug 2024 09:07:08 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7143422ecd6sm8768819b3a.31.2024.08.27.09.07.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Aug 2024 09:07:08 -0700 (PDT) Date: Wed, 28 Aug 2024 00:07:58 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v2 2/4] ref: add regular ref content check for files backend Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We implicitly rely on "git-fsck(1)" to check the consistency of regular refs. However, when parsing the regular refs for files backend by using "files-backend.c::parse_loose_ref_contents", we allow the ref content to be end with no newline or contain some garbages. It may seem that we should report an error or warn fsck message to the user about above situations. However, there may be some third-party tools customizing the content of refs. We should not report an error fsck message. And we cannot either report a warn fsck message to the user. This is because if the caller set the "strict" field in "fsck_options" to to upgrade the fsck warnings to errors. We should not allow the user to upgrade the fsck warnings to errors. It might cause compatibility issue which will break the legacy repository. So we add the following two fsck infos to represent the situation where the ref content ends without newline or has garbages: 1. "refMissingNewline(INFO)": A valid ref does not end with newline. 2. "trailingRefContent(INFO)": A ref has trailing contents. In "fsck.c::fsck_vreport", we will convert "FSCK_INFO" to "FSCK_WARN", and we can still warn the user about these situations when using "git-refs verify" without introducing compatibility issue. In current "git-fsck(1)", it will report an error when the ref content is bad, so we should following this to report an error to the user when "parse_loose_ref_contents" fails. And we add a new fsck error message called "badRefContent(ERROR)" to represent that a ref has a bad content. In order to tell whether the ref has trailing content, add a new parameter "trailing" to "parse_loose_ref_contents". Then introduce a new function "files_fsck_refs_content" to check the regular refs to enhance the "git-refs verify". Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 9 ++++ fsck.h | 3 ++ refs.c | 2 +- refs/files-backend.c | 68 ++++++++++++++++++++++++++- refs/refs-internal.h | 2 +- t/t0602-reffiles-fsck.sh | 87 +++++++++++++++++++++++++++++++++++ 6 files changed, 167 insertions(+), 4 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 68a2801f15..fc074fc571 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -19,6 +19,9 @@ `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. +`badRefContent`:: + (ERROR) A ref has a bad content. + `badRefFiletype`:: (ERROR) A ref has a bad file type. @@ -170,6 +173,12 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`refMissingNewline`:: + (INFO) A valid ref does not end with newline. + +`trailingRefContent`:: + (INFO) A ref has trailing contents. + `treeNotSorted`:: (ERROR) A tree is not properly sorted. diff --git a/fsck.h b/fsck.h index 500b4c04d2..b85072df57 100644 --- a/fsck.h +++ b/fsck.h @@ -31,6 +31,7 @@ enum fsck_msg_type { FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ + FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ @@ -84,6 +85,8 @@ enum fsck_msg_type { FUNC(MAILMAP_SYMLINK, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ + FUNC(REF_MISSING_NEWLINE, INFO) \ + FUNC(TRAILING_REF_CONTENT, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) diff --git a/refs.c b/refs.c index 74de3d3009..5e74881945 100644 --- a/refs.c +++ b/refs.c @@ -1758,7 +1758,7 @@ static int refs_read_special_head(struct ref_store *ref_store, } result = parse_loose_ref_contents(ref_store->repo->hash_algo, content.buf, - oid, referent, type, failure_errno); + oid, referent, type, NULL, failure_errno); done: strbuf_release(&full_path); diff --git a/refs/files-backend.c b/refs/files-backend.c index d6fc3bd67c..69c00073eb 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -560,7 +560,7 @@ static int read_ref_internal(struct ref_store *ref_store, const char *refname, buf = sb_contents.buf; ret = parse_loose_ref_contents(ref_store->repo->hash_algo, buf, - oid, referent, type, &myerr); + oid, referent, type, NULL, &myerr); out: if (ret && !myerr) @@ -597,7 +597,7 @@ static int files_read_symbolic_ref(struct ref_store *ref_store, const char *refn int parse_loose_ref_contents(const struct git_hash_algo *algop, const char *buf, struct object_id *oid, struct strbuf *referent, unsigned int *type, - int *failure_errno) + const char **trailing, int *failure_errno) { const char *p; if (skip_prefix(buf, "ref:", &buf)) { @@ -619,6 +619,10 @@ int parse_loose_ref_contents(const struct git_hash_algo *algop, *failure_errno = EINVAL; return -1; } + + if (trailing) + *trailing = p; + return 0; } @@ -3430,6 +3434,65 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refs_check_dir, struct dir_iterator *iter); +static int files_fsck_refs_content(struct ref_store *ref_store, + struct fsck_options *o, + const char *refs_check_dir, + struct dir_iterator *iter) +{ + struct strbuf ref_content = STRBUF_INIT; + struct strbuf referent = STRBUF_INIT; + struct strbuf refname = STRBUF_INIT; + struct fsck_ref_report report = {0}; + const char *trailing = NULL; + unsigned int type = 0; + int failure_errno = 0; + struct object_id oid; + int ret = 0; + + strbuf_addf(&refname, "%s/%s", refs_check_dir, iter->relative_path); + report.path = refname.buf; + + if (S_ISREG(iter->st.st_mode)) { + if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { + ret = error_errno(_("%s/%s: unable to read the ref"), + refs_check_dir, iter->relative_path); + goto cleanup; + } + + if (parse_loose_ref_contents(ref_store->repo->hash_algo, + ref_content.buf, &oid, &referent, + &type, &trailing, &failure_errno)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "invalid ref content"); + goto cleanup; + } + + if (!(type & REF_ISSYMREF)) { + if (*trailing == '\0') { + ret = fsck_report_ref(o, &report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + goto cleanup; + } + + if (*trailing != '\n' || (*(trailing + 1) != '\0')) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing garbage in ref"); + goto cleanup; + } + } + goto cleanup; + } + +cleanup: + strbuf_release(&refname); + strbuf_release(&ref_content); + strbuf_release(&referent); + return ret; +} + static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, struct fsck_options *o, const char *refs_check_dir, @@ -3512,6 +3575,7 @@ static int files_fsck_refs(struct ref_store *ref_store, { files_fsck_refs_fn fsck_refs_fn[]= { files_fsck_refs_name, + files_fsck_refs_content, NULL, }; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2313c830d8..73b05f971b 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -715,7 +715,7 @@ struct ref_store { int parse_loose_ref_contents(const struct git_hash_algo *algop, const char *buf, struct object_id *oid, struct strbuf *referent, unsigned int *type, - int *failure_errno); + const char **trailing, int *failure_errno); /* * Fill in the generic part of refs and add it to our collection of diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 71a4d1a5ae..7c1910d784 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -89,4 +89,91 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' test_must_be_empty err ' +test_expect_success 'regular ref content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + git tag tag-1 && + git commit --allow-empty -m second && + git checkout -b branch-2 && + git tag tag-2 && + git checkout -b a/b/tag-2 && + + printf "%s" "$(git rev-parse branch-1)" > $branch_dir_prefix/branch-1-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-1-no-newline: refMissingNewline: missing newline + EOF + rm $branch_dir_prefix/branch-1-no-newline && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse branch-1)" > $branch_dir_prefix/branch-1-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/branch-1-garbage && + test_cmp expect err && + + printf "%s\n\n\n" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-1-garbage && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-1-garbage && + test_cmp expect err && + + printf "%s garbage\n\na" "$(git rev-parse tag-2)" > $tag_dir_prefix/tag-2-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-2-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-2-garbage && + test_cmp expect err && + + printf "%s garbage" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-garbage && + test_must_fail git -c fsck.trailingRefContent=error refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-1-garbage: trailingRefContent: trailing garbage in ref + EOF + rm $tag_dir_prefix/tag-1-garbage && + test_cmp expect err && + + printf "%sx" "$(git rev-parse tag-1)" > $tag_dir_prefix/tag-1-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-1-bad: badRefContent: invalid ref content + EOF + rm $tag_dir_prefix/tag-1-bad && + test_cmp expect err && + + printf "xfsazqfxcadas" > $tag_dir_prefix/tag-2-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/tag-2-bad: badRefContent: invalid ref content + EOF + rm $tag_dir_prefix/tag-2-bad && + test_cmp expect err && + + printf "xfsazqfxcadas" > $branch_dir_prefix/a/b/branch-2-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-2-bad: badRefContent: invalid ref content + EOF + rm $branch_dir_prefix/a/b/branch-2-bad && + test_cmp expect err +' + test_done From patchwork Tue Aug 27 16:08:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13779776 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 751471BFE00 for ; Tue, 27 Aug 2024 16:07:19 +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=1724774841; cv=none; b=alSl0LihpDOZnWnpGjQ30DUjfigCm2AfaOHsus0S/uDQbgPBiuCxZ+TEQtVwEi1tDwtml65hRW7qeYqW/mIxol+z13dEeuOFZBlX52IetLipEG2GffIAuowHxSZOuxZfQa8F2G/czBRm9GQdJ0t2XPb+E/GTo95LHl451lw2Z14= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724774841; c=relaxed/simple; bh=InAbJorLUaGwnIzVrSSSWTe6If+FcIgJ+95pLkQGLiM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DbowqaB8ogIQhARtVsE8xDo4Oszq5fyOB3hPewKjt7gyUsRLLQOZBG6SATr0a/IVPUD7OinHLu67eKBHnc6FrWhVOMG1/TunXfXBp958cI1Ti8LO0OYbBsrI5h6N+7c9jEkNvUdmqj7itn0X0FPz0Khmn8EkILzggFpGmpjdJf0= 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=PyJOniTf; 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="PyJOniTf" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-2020ac89cabso51671465ad.1 for ; Tue, 27 Aug 2024 09:07:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724774838; x=1725379638; 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=GigmFyM2jVUZNw11LS15DDLVzycUW3R7qYop1260sw4=; b=PyJOniTf0W4msjxVAN5Id0dgb3ueVCotDFReSTQe085gQuV1uEgHukIUuYaQLOVpHm Kd+Lj8W+mmr7xsWRgalqEPprtqNDKNXn081HdRIQxZPpoUF/owkTP7QK5NLvSnNUYnV+ aYuVEZBK7fRwbpPFJtkX5Xktk4eQTdLIiLWBkTRGhV+75lNceJDBS8ryXp8aJqvq1Dzo OdGckpWZ6yMRv1SJyHMgZsc2dM9S0IuV/DrKThskKHFpif7a0qImNFxZPFgOQVsHRTnR A41p/iJXgCZpLkuf4F18AfgbfgGmnPH+Os1oEaILVHVDAv/iOOUhxWMgnO2xk03ZrFYt uY5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724774838; x=1725379638; 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=GigmFyM2jVUZNw11LS15DDLVzycUW3R7qYop1260sw4=; b=e60oIz1bxLHfPLe09LuyjSGZPEXesm5gUk6AFhBq3Pt+XR+v2Xtf5/gcBzMiGqZpFE 61jxOk7YP+blMpibBCOo1cubnYC+s0+WbDHc8T/4LIxEf32tp+Um7ccosqEPNUYvxEpx DqZHJ4PbWYYC9aOO89aU08uPKrAuIDMV8T43bYY5gb/Oj4oAiNh8HlFMblkz66rC+PUO gkjlTzVr3HyOpAv/u9Bb3xZXDgIyBVrkkKfR6RgCPZKzQXOBHgIQGejn0u8HRbqv0kkG AQrEhTRhvcyDwVa44Ou34gaYrs88NQEgTaGtuZ3H3spFfOONvHNGdZ9y+HQrZ/ffQgrB RAAQ== X-Gm-Message-State: AOJu0YxCEIUUlMkHSwUQaW7PUSCUPxlN0ouOFw+SNjqy9JhIwjCHxhKA uPSam/A+uw9B/c2cSMotjicDGP1yFjQDK9FzLrDh9oMevqLwW1Z16syvjg== X-Google-Smtp-Source: AGHT+IHkTn+lJ+DzMddBXHHYP5JikpHnv9vgYfkPHCU16ysHoZa0ujzO6kaKspwJREoVui2kehuzog== X-Received: by 2002:a17:902:a705:b0:1fc:2e38:d3de with SMTP id d9443c01a7336-2039e443915mr98254315ad.7.1724774837775; Tue, 27 Aug 2024 09:07:17 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-20385fc6c26sm85011205ad.297.2024.08.27.09.07.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Aug 2024 09:07:17 -0700 (PDT) Date: Wed, 28 Aug 2024 00:08:07 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v2 3/4] ref: add symbolic ref content check for files backend Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We have already introduced the checks for regular refs. There is no need to check the consistency of the target which the symbolic ref points to. Instead, we just check the content of the symbolic ref itself. In order to check the content of the symbolic ref, create a function "files_fsck_symref_target". It will first check whether the "pointee" is under the "refs/" directory and then we will check the "pointee" itself. There is no specification about the content of the symbolic ref. Although we do write "ref: %s\n" to create a symbolic ref by using "git-symbolic-ref(1)" command. However, this is not mandatory. We still accept symbolic refs with null trailing garbage. Put it more specific, the following are correct: 1. "ref: refs/heads/master " 2. "ref: refs/heads/master \n \n" 3. "ref: refs/heads/master\n\n" But we do not allow any non-null trailing garbage. The following are bad symbolic contents which will be reported as fsck error by "git-fsck(1)". 1. "ref: refs/heads/master garbage\n" 2. "ref: refs/heads/master \n\n\n garbage " In order to provide above checks, we will use "strrchr" to check whether we have newline in the ref content. Then we will check the name of the "pointee" is correct by using "check_refname_format". If the function fails, we need to trim the "pointee" to see whether the null-garbage causes the function fails. If so, we need to report that there is null-garbage in the symref content. Otherwise, we should report the user the "pointee" is bad. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 ++ fsck.h | 1 + refs/files-backend.c | 77 +++++++++++++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 54 ++++++++++++++++++++++++ 4 files changed, 135 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index fc074fc571..85fd058c81 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -28,6 +28,9 @@ `badRefName`:: (ERROR) A ref has an invalid format. +`badSymrefPointee`:: + (ERROR) The pointee of a symref is bad. + `badTagName`:: (INFO) A tag has an invalid format. diff --git a/fsck.h b/fsck.h index b85072df57..cbe837f84c 100644 --- a/fsck.h +++ b/fsck.h @@ -34,6 +34,7 @@ enum fsck_msg_type { FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ + FUNC(BAD_SYMREF_POINTEE, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ FUNC(BAD_TREE, ERROR) \ FUNC(BAD_TREE_SHA1, ERROR) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index 69c00073eb..382c73fcf7 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3434,11 +3434,81 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refs_check_dir, struct dir_iterator *iter); +/* + * Check the symref "pointee_name" and "pointee_path". The caller should + * make sure that "pointee_path" is absolute. For symbolic ref, "pointee_name" + * would be the content after "refs:". + */ +static int files_fsck_symref_target(struct fsck_options *o, + struct fsck_ref_report *report, + const char *refname, + struct strbuf *pointee_name, + struct strbuf *pointee_path) +{ + const char *newline_pos = NULL; + const char *p = NULL; + struct stat st; + int ret = 0; + + if (!skip_prefix(pointee_name->buf, "refs/", &p)) { + + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "points to ref outside the refs directory"); + goto out; + } + + newline_pos = strrchr(p, '\n'); + if (!newline_pos || *(newline_pos + 1)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + } + + if (check_refname_format(pointee_name->buf, 0)) { + /* + * When containing null-garbage, "check_refname_format" will + * fail, we should trim the "pointee" to check again. + */ + strbuf_rtrim(pointee_name); + if (!check_refname_format(pointee_name->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing null-garbage"); + goto out; + } + + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "points to refname with invalid format"); + } + + /* + * Missing target should not be treated as any error worthy event and + * not even warn. It is a common case that a symbolic ref points to a + * ref that does not exist yet. If the target ref does not exist, just + * skip the check for the file type. + */ + if (lstat(pointee_path->buf, &st) < 0) + goto out; + + if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "points to an invalid file type"); + goto out; + } + +out: + return ret; +} + static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_options *o, const char *refs_check_dir, struct dir_iterator *iter) { + struct strbuf pointee_path = STRBUF_INIT; struct strbuf ref_content = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct strbuf refname = STRBUF_INIT; @@ -3482,6 +3552,12 @@ static int files_fsck_refs_content(struct ref_store *ref_store, "trailing garbage in ref"); goto cleanup; } + } else { + strbuf_addf(&pointee_path, "%s/%s", + ref_store->gitdir, referent.buf); + ret = files_fsck_symref_target(o, &report, refname.buf, + &referent, + &pointee_path); } goto cleanup; } @@ -3490,6 +3566,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, strbuf_release(&refname); strbuf_release(&ref_content); strbuf_release(&referent); + strbuf_release(&pointee_path); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 7c1910d784..69280795ca 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -176,4 +176,58 @@ test_expect_success 'regular ref content should be checked' ' test_cmp expect err ' +test_expect_success 'symbolic ref content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + git tag tag-1 && + git checkout -b a/b/branch-2 && + + printf "ref: refs/heads/branch" > $branch_dir_prefix/branch-1-no-newline && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-1-no-newline: refMissingNewline: missing newline + EOF + rm $branch_dir_prefix/branch-1-no-newline && + test_cmp expect err && + + printf "ref: refs/heads/branch " > $branch_dir_prefix/a/b/branch-trailing && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-trailing: trailingRefContent: trailing null-garbage + EOF + rm $branch_dir_prefix/a/b/branch-trailing && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" > $branch_dir_prefix/a/b/branch-trailing && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing: trailingRefContent: trailing null-garbage + EOF + rm $branch_dir_prefix/a/b/branch-trailing && + test_cmp expect err && + + printf "ref: refs/heads/branch \n\n " > $branch_dir_prefix/a/b/branch-trailing && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-trailing: trailingRefContent: trailing null-garbage + EOF + rm $branch_dir_prefix/a/b/branch-trailing && + test_cmp expect err && + + printf "ref: refs/heads/.branch\n" > $branch_dir_prefix/branch-2-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-2-bad: badSymrefPointee: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-2-bad && + test_cmp expect err +' + test_done From patchwork Tue Aug 27 16:08:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13779777 Received: from mail-pf1-f175.google.com (mail-pf1-f175.google.com [209.85.210.175]) (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 54FE71BFE00 for ; Tue, 27 Aug 2024 16:07:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724774850; cv=none; b=FZ+JyzcLnU/EZGJzvxqNRr+C1Ktqj7r/UehtYyyVuaYqagmgI4wF0rLYAj5B4O81p85XYbl/fyNAT0GZ2gYXO4FiBYh/V5pCLumgDzQWOOKyMw6tKKdR/9MOaRs/K7XEJURIPzupkwLcM+HRFk16HOO69oAvMldh0deJ/5T8gU8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724774850; c=relaxed/simple; bh=SUfPLe2cyTk7OHh9/pNm0aZuQddVHDUpsrnRJd2sN/E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Kf19d9bnIm+SNCsDns0/Pulp/GYYr99c03oP7soXrAKlGJFLxVDAJoC4gkpU1eaypgpCRi64fx7j8fmCftcqt6OOb6veLg/q5wVp/mtB8IQjtUSvPNrFIqvu7uiI2iyeIIiskBh7lJIfe+J1aoAVzCc94r1sXNzoO0t6hgQG2wM= 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=Xuv3U1Uk; arc=none smtp.client-ip=209.85.210.175 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="Xuv3U1Uk" Received: by mail-pf1-f175.google.com with SMTP id d2e1a72fcca58-71431524f33so4804557b3a.1 for ; Tue, 27 Aug 2024 09:07:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724774848; x=1725379648; 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=jsIZ1l/wDwukaEwsniotiPY8pApzP7Lrrjlz3XeAU+s=; b=Xuv3U1UkEOZ9G59JGat4f0xw9RGEZmsK1XmjFToFmKibZX4LUhDShU4zn/e22M6gwB EsJb+v5mO+WwbK8jFaEpgjdHbIfQZWYRSM3ScmkJiv3W35ugezV0xuwd7s2Q6+NqESNF WMfSl7Ps2OkgrSXakVAfeUehKq3HIEJ/4RMfCNU6wtc++1B1MEvk5/e7cOMpQQMTEGUb VaPWXt6nR3WKXvzohe2BEXCLxWJDo4IyGqZsSbUalQFbdutjyEXroY5lUsl+CSAy7Z7b DxaZEfTEdWDhI56IZZ1OZGWnAI+OAdFptRIQjvC7yBDtVV2dnXwtp7bxEdXUqniETw8f qNPA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724774848; x=1725379648; 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=jsIZ1l/wDwukaEwsniotiPY8pApzP7Lrrjlz3XeAU+s=; b=RHDMFfmNb/t9oaaQSHjA18lQG+JRGGYueJNC7AkeSUy3tB4bgKHkn6tgP2NtV4Y0ka 5dmFOUSryYrTKWTc+gcHSLuZQu5KjRmdEcl+mdtHhxmnSVs16eeFGrfW50QQrc+OM0L9 jzjXhWwmmze0bnHjt4Z+NxBAf2qeAROVH3nwoEQ07JJt5GIvem80B0brdedxLTosPXzM 31CcC4cnmmPQfwGpn7aDZoIeqYtc9r3lCBGgM7CgvPYcn60EMudKVWsEIyktl+kwmc6B gkO3LPwcBaJDgFe9tDcVpKXLqug9fSDzs+5n1ZgXREEynQyr/MBlhHwXnNRx6M5DBl2T QWxQ== X-Gm-Message-State: AOJu0YzUtlUofbwNUqP28qHQ6yoXyioMYVGXT9sh4Rz6J/T28y0Czt7+ UWVfB3FMyONbP3v+SDr2XDdjlEe3D6UhhbjweOt2laHlehtDYFCxo6ETZg== X-Google-Smtp-Source: AGHT+IGkKtoH2d/Hx3CVf2W0S6DBfbnpk/AhmhHmZWbDDhNKIUqQPOg2Q3nWWvVeNsypalBv2vmAFA== X-Received: by 2002:a05:6a00:2d9a:b0:70d:3420:9309 with SMTP id d2e1a72fcca58-715c0003324mr4185208b3a.17.1724774847754; Tue, 27 Aug 2024 09:07:27 -0700 (PDT) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-715a2c8ecffsm3922937b3a.151.2024.08.27.09.07.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Aug 2024 09:07:27 -0700 (PDT) Date: Wed, 28 Aug 2024 00:08:17 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v2 4/4] ref: add symlink ref check for files backend Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We have already introduced "files_fsck_symref_target". We should reuse this function to handle the symrefs which are legacy symbolic links. We should not check the trailing garbage for symbolic links. Add a new parameter "symbolic_link" to disable some checks which should only be used for symbolic ref. We firstly use the "strbuf_add_real_path" to resolve the symlinks and get the absolute path "pointee_path" which the symlink ref points to. Then we can get the absolute path "abs_gitdir" of the "gitdir". By combining "pointee_path" and "abs_gitdir", we can extract the "referent". Thus, we can reuse "files_fsck_symref_target" function to seamlessly check the symlink refs. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 68 +++++++++++++++++++++++++++++----------- t/t0602-reffiles-fsck.sh | 44 ++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 18 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 382c73fcf7..8641e3ba65 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1,4 +1,5 @@ #include "../git-compat-util.h" +#include "../abspath.h" #include "../copy.h" #include "../environment.h" #include "../gettext.h" @@ -3437,13 +3438,15 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, /* * Check the symref "pointee_name" and "pointee_path". The caller should * make sure that "pointee_path" is absolute. For symbolic ref, "pointee_name" - * would be the content after "refs:". + * would be the content after "refs:". For symblic link, "pointee_name" would + * be the relative path agaignst "gitdir". */ static int files_fsck_symref_target(struct fsck_options *o, struct fsck_ref_report *report, const char *refname, struct strbuf *pointee_name, - struct strbuf *pointee_path) + struct strbuf *pointee_path, + unsigned int symbolic_link) { const char *newline_pos = NULL; const char *p = NULL; @@ -3458,24 +3461,28 @@ static int files_fsck_symref_target(struct fsck_options *o, goto out; } - newline_pos = strrchr(p, '\n'); - if (!newline_pos || *(newline_pos + 1)) { - ret = fsck_report_ref(o, report, - FSCK_MSG_REF_MISSING_NEWLINE, - "missing newline"); + if (!symbolic_link) { + newline_pos = strrchr(p, '\n'); + if (!newline_pos || *(newline_pos + 1)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + } } if (check_refname_format(pointee_name->buf, 0)) { - /* - * When containing null-garbage, "check_refname_format" will - * fail, we should trim the "pointee" to check again. - */ - strbuf_rtrim(pointee_name); - if (!check_refname_format(pointee_name->buf, 0)) { - ret = fsck_report_ref(o, report, - FSCK_MSG_TRAILING_REF_CONTENT, - "trailing null-garbage"); - goto out; + if (!symbolic_link) { + /* + * When containing null-garbage, "check_refname_format" will + * fail, we should trim the "pointee" to check again. + */ + strbuf_rtrim(pointee_name); + if (!check_refname_format(pointee_name->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing null-garbage"); + goto out; + } } ret = fsck_report_ref(o, report, @@ -3510,9 +3517,12 @@ static int files_fsck_refs_content(struct ref_store *ref_store, { struct strbuf pointee_path = STRBUF_INIT; struct strbuf ref_content = STRBUF_INIT; + struct strbuf abs_gitdir = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct strbuf refname = STRBUF_INIT; struct fsck_ref_report report = {0}; + const char *pointee_name = NULL; + unsigned int symbolic_link = 0; const char *trailing = NULL; unsigned int type = 0; int failure_errno = 0; @@ -3557,16 +3567,38 @@ static int files_fsck_refs_content(struct ref_store *ref_store, ref_store->gitdir, referent.buf); ret = files_fsck_symref_target(o, &report, refname.buf, &referent, - &pointee_path); + &pointee_path, + symbolic_link); } goto cleanup; } + symbolic_link = 1; + + strbuf_add_real_path(&pointee_path, iter->path.buf); + strbuf_add_absolute_path(&abs_gitdir, ref_store->gitdir); + strbuf_normalize_path(&abs_gitdir); + if (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1])) + strbuf_addch(&abs_gitdir, '/'); + + if (!skip_prefix(pointee_path.buf, abs_gitdir.buf, &pointee_name)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "point to target outside gitdir"); + goto cleanup; + } + + strbuf_addstr(&referent, pointee_name); + ret = files_fsck_symref_target(o, &report, refname.buf, + &referent, &pointee_path, + symbolic_link); + cleanup: strbuf_release(&refname); strbuf_release(&ref_content); strbuf_release(&referent); strbuf_release(&pointee_path); + strbuf_release(&abs_gitdir); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 69280795ca..36992fbc7f 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -230,4 +230,48 @@ test_expect_success 'symbolic ref content should be checked' ' test_cmp expect err ' +test_expect_success SYMLINKS 'symbolic ref (symbolic link) content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + git tag tag-1 && + git checkout -b a/b/branch-2 && + + ln -sf ../../../../branch $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: point to target outside gitdir + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ../../logs/branch-bad $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: points to ref outside the refs directory + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./"branch space" $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./".branch" $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err +' + test_done From patchwork Thu Aug 29 15:00:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13783360 Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ED8981B1513 for ; Thu, 29 Aug 2024 15:00:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.70 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724943631; cv=none; b=ppZfjh6TCW/gEuG0Occ2OSgzf8lGuX5M1peJzK/RcshEHKJC7Z/zuw62aKSp3DP+73/bEYt+p2fy62oLA67JxLigAaNu36duzS3j42jYcfgVrPhkFDwEKfX7uKNUl8qAbWl/Yd1AaaaKroxr35jciuBMVmgNruCHgtvB4K3Obzs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724943631; c=relaxed/simple; bh=PtAztH7OeAfCQpNB/t/14fZVBQUMaGq+RAG4Rv3sc5Y=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=rzIl6WDIyvpEtUzNiy0HYham0/UN0IbOfKAEvOq1pu4iaRWQ+gCwkS/Zyzf85IjQBnh0XCloh4cp+FhlV33/CpmMcMuEc0Ie809B3jcMyWKQbNyjEGkLfJ6M1BbcSo+cyKouoPvI7o/pgat2grHURu866q/exSou17xvqZ1/+ek= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=Kvmo1FxR; arc=none smtp.client-ip=64.147.108.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="Kvmo1FxR" Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 19E38220DF; Thu, 29 Aug 2024 11:00:23 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:in-reply-to:references:date:message-id:mime-version :content-type; s=sasl; bh=PtAztH7OeAfCQpNB/t/14fZVBQUMaGq+RAG4Rv 3sc5Y=; b=Kvmo1FxROjswI0onUImM+Jz5VFT1nxZwLwxNKY2qKeDMv1MdoNyHk6 QzJtZ4ZwfsaqNzPi703R2kZTaDnzxgkW5NSFvYJaU6f9Lp2osOBulsIBsm4ryUO0 AOqgPnzHAFlov5eXGFGU91xJnpoKiqvtq8TW1YtknV+xjDu5Gk5TA= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id C774B220DE; Thu, 29 Aug 2024 11:00:22 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.94.240]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id AABE2220DD; Thu, 29 Aug 2024 11:00:21 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: Jeff King Cc: git@vger.kernel.org Subject: [PATCH 8/6] CodingGuidelines: also mention MAYBE_UNUSED In-Reply-To: <20240829040215.GA4054823@coredump.intra.peff.net> (Jeff King's message of "Thu, 29 Aug 2024 00:02:15 -0400") References: <20240829040215.GA4054823@coredump.intra.peff.net> Date: Thu, 29 Aug 2024 08:00:19 -0700 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: 69CEF6F8-6617-11EF-8DBC-2BAEEB2EC81B-77302942!pb-smtp1.pobox.com Jeff King writes: > On Wed, Aug 28, 2024 at 02:28:47PM -0700, Junio C Hamano wrote: > >> By the way, Peff, do we have MAYBE_UNUSED that can be used in a case >> like this one? Platforms without symbolic links supported may well >> define NO_SYMLINK_HEAD, which makes the incoming parameters unused. > > Yes, it would be fine to use MAYBE_UNUSED in a case like this. It turns out that I was, without realizing it myself, making an oblique reference to your patch 7/6 ;-) Perhaps something along this line? ---- >8 ---- Subject: CodingGuidelines: also mention MAYBE_UNUSED A function that uses a parameter in one build may lose all uses of the parameter in another build, depending on the configuration. A workaround for such a case, MAYBE_UNUSED, should also be mentioned when we recommend the use of UNUSED to our developers. Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 5 +++-- git-compat-util.h | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines index d0fc7cfe60..3263245b03 100644 --- c/Documentation/CodingGuidelines +++ w/Documentation/CodingGuidelines @@ -262,8 +262,9 @@ For C programs: like "error: unused parameter 'foo' [-Werror=unused-parameter]", which indicates that a function ignores its argument. If the unused parameter can't be removed (e.g., because the function is used as a - callback and has to match a certain interface), you can annotate the - individual parameters with the UNUSED keyword, like "int foo UNUSED". + callback and has to match a certain interface), you can annotate + the individual parameters with the UNUSED (or MAYBE_UNUSED) + keyword, like "int foo UNUSED". - We try to support a wide range of C compilers to compile Git with, including old ones. As of Git v2.35.0 Git requires C99 (we check diff --git c/git-compat-util.h w/git-compat-util.h index 71b4d23f03..23307ce780 100644 --- c/git-compat-util.h +++ w/git-compat-util.h @@ -195,6 +195,17 @@ struct strbuf; #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +/* + * UNUSED marks a function parameter that is always unused. + * + * A callback interface may dictate that a function accepts a + * parameter at that position, but the implementation of the function + * may not need to use the parameter. In such a case, mark the parameter + * with UNUSED. + * + * When a parameter may be used or unused, depending on conditional + * compilation, consider using MAYBE_UNUSED instead. + */ #if GIT_GNUC_PREREQ(4, 5) #define UNUSED __attribute__((unused)) \ __attribute__((deprecated ("parameter declared as UNUSED"))) @@ -649,6 +660,16 @@ static inline int git_has_dir_sep(const char *path) #define RESULT_MUST_BE_USED #endif +/* + * MAYBE_UNUSED marks a function parameter that may be unused, but + * whose use is not an error. + * + * Depending on a configuration, all uses of a function parameter may + * become #ifdef'ed away. Marking such a parameter with UNUSED would + * give a warning in a compilation where the parameter is indeed used, + * and not marking such a parameter would give a warning in a + * compilation where the parameter is unused. + */ #define MAYBE_UNUSED __attribute__((__unused__)) #include "compat/bswap.h"