From patchwork Mon Apr 29 08:16:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13646452 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 0D2A79470 for ; Mon, 29 Apr 2024 08:16:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378615; cv=none; b=kPF9VX+Nn9BXsskrJiOc8Gsbw3clWpehdW+4rjrHx0VnKn/uBHtnyQRhSIu4Z6pU5w7wnG/qAd9NbCwPpuH0bGF1sE4/4hh748mAwLTx6BLxjlkzXyxY54pEXvTXnjcdbWUHVkWG2aWZRGwJH1EgESASzqwIv6D0/L58CV0hA/g= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378615; c=relaxed/simple; bh=mzJACVV6cSWrvOcOCYanSBJo9lrOwny8rLcSFQO3U0c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=aAy/6HLI6MfeGcmXH2W7GlCcc+Zc22w7MQpTeNRIg4DMnqmuBdT0sZcnu/x/G7Y+C1yIp54KekdsLxAnn21YZTaPCNsB24XjkvmEAwB1dMyulbbQQ07DhEbG5459NxCEEFCLw3I0aoIBxkVhej+mHwu6ejGKaqEZDeTMWkmPtSE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 20179 invoked by uid 109); 29 Apr 2024 08:16:53 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 29 Apr 2024 08:16:53 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 26788 invoked by uid 111); 29 Apr 2024 08:16:57 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 29 Apr 2024 04:16:57 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 29 Apr 2024 04:16:52 -0400 From: Jeff King To: git@vger.kernel.org Cc: Patrick Steinhardt Subject: [PATCH 1/8] t0600: don't create ref outside of refs/ Message-ID: <20240429081652.GA228803@coredump.intra.peff.net> References: <20240429081512.GA4130242@coredump.intra.peff.net> 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: <20240429081512.GA4130242@coredump.intra.peff.net> We have a test that tries to manipulate the branch refs/heads/referrent, mostly using either the fully qualified refname or git-branch (which implies refs/heads/). However, the call to update-ref uses the unqualified name, meaning we were quietly creating ".git/referrent", which was otherwise unused by the test. Fix this to specify refs/heads/referrent. I _think_ it actually doesn't affect the test outcome either way. The point of the test is that expiring reflogs for "the_symref" should not fail when we cannot lock "refs/heads/referrent" that it points to (because we have created a fake .lock file for it). And that is true even if the "referrent" file does not even exist. After all, the process holding the lock could be in the act of creating it. So I suspect this "update-ref" line could just be dropped entirely. Which you can verify by going back to its origin in 41d796ed5c (refs: on symref reflog expire, lock symref not referrent, 2016-04-07) and removing it. The test fails without the matching code change and passes with it. But I think it's worth keeping the update-ref call, as it creates a situation which is more likely to match what we'd see in the real world. Even if it does not matter now, it's possible it could in the future. Signed-off-by: Jeff King --- t/t0600-reffiles-backend.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh index 64214340e7..41db9e30d4 100755 --- a/t/t0600-reffiles-backend.sh +++ b/t/t0600-reffiles-backend.sh @@ -343,7 +343,7 @@ test_expect_success SHA1 'parsing reverse reflogs at BUFSIZ boundaries' ' test_expect_success 'reflog expire operates on symref not referrent' ' git branch --create-reflog the_symref && git branch --create-reflog referrent && - git update-ref referrent HEAD && + git update-ref refs/heads/referrent HEAD && git symbolic-ref refs/heads/the_symref refs/heads/referrent && test_when_finished "rm -f .git/refs/heads/referrent.lock" && touch .git/refs/heads/referrent.lock && From patchwork Mon Apr 29 08:21:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13646453 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 48E7015E8B for ; Mon, 29 Apr 2024 08:21:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378891; cv=none; b=nZ4SulmKlyCAMlwejz/C5+UZ+lTEqJYGqyZ/ij6+OVZvusKkFKwzEvJXJkcHYmEYEBvR0Pit7DJKLpbh+kL/Glg6ZBp9zsP3PzWOWhHZWvtWqX1I+MnaMbRWMj/HNX+LbmVyFQ7UnBM53FOy1e/wdOhNDCXTEEFq8qKXiOR84+w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378891; c=relaxed/simple; bh=MOcTaq+LHiJUklOPv1EQ5cLjiU36W66L5zvEsbw+IvY=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=EZOStiEr7GbgmDWNQXWggNHUHsGS5HNONr5MP9RA/cpLOJ0b2qTXSfxr3xBZ4B3IEOxsLyXW09xCZyhOntDKwkFLL2Q8H0Vknix0bm8xeeoiX2pfCgNFrR4NBkhcZP99/VgfOnKnYfI2cId60vTT3XbTq7UG9NFsbqCkuQl8lrw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 20190 invoked by uid 109); 29 Apr 2024 08:21:29 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 29 Apr 2024 08:21:29 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 26831 invoked by uid 111); 29 Apr 2024 08:21:33 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 29 Apr 2024 04:21:33 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 29 Apr 2024 04:21:28 -0400 From: Jeff King To: git@vger.kernel.org Cc: Patrick Steinhardt , Taylor Blau Subject: [PATCH 2/8] t5619: use fully qualified refname for branch Message-ID: <20240429082128.GA233423@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline We use test_commit_bulk to update the ref "main". But without qualification, that is really creating ".git/main", not an actual branch in "refs/heads/". In the end it does not really matter, because after creating "main" its only purpose is for us to point HEAD to its same commit id. And that works regardless of how we named it (the later call to update-ref just calls it "main", but that is OK; we are resolving it to an oid there, so our DWIM logic will find it in "refs/heads/"). Since it seems like going outside of "refs/" was accidental here, let's do the more normal thing and just use a branch. That makes the test less confusing and will future proof us against ref updates becoming more picky. Signed-off-by: Jeff King --- t/t5619-clone-local-ambiguous-transport.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5619-clone-local-ambiguous-transport.sh b/t/t5619-clone-local-ambiguous-transport.sh index cce62bf78d..70db437496 100755 --- a/t/t5619-clone-local-ambiguous-transport.sh +++ b/t/t5619-clone-local-ambiguous-transport.sh @@ -21,7 +21,7 @@ test_expect_success 'setup' ' echo "secret" >sensitive/secret && git init --bare "$REPO" && - test_commit_bulk -C "$REPO" --ref=main 1 && + test_commit_bulk -C "$REPO" --ref=refs/heads/main 1 && git -C "$REPO" update-ref HEAD main && git -C "$REPO" update-server-info && From patchwork Mon Apr 29 08:21:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13646454 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 B8D82171A2 for ; Mon, 29 Apr 2024 08:21:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378911; cv=none; b=mP3vzKHQGxfJ7zO2j/TsuEvfgGV5r7eaO1ZPkIKXZgmhcNUq6E0RojAqXXLWcYotz5xf7zJQFnCho0Ipqqs5fHUfqFKt+YdJXnGR4iWA69JNtsMRnjRXJYim3H2fi4kEZWafqsMg+4z31T9sCKDcgH43yMVQT4Em7E4VzwbW7y4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378911; c=relaxed/simple; bh=0CjM4DdunfZIHn57I+E1dQQWziZfXl/U4F0mRel6jxE=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=VL4botwSDOW6903ybREmGvtFrxADEFUMNqxoCBSU5H3+esqcryzsQCfp29gqNTHkX2IU32lJiQ9E6cv+CmoeIODU4Kgoi6kPcuYsDFOaUje0nFdbJAFt9trHvLBkhGsHJV6XO8ZoYb4O8f3kqMJiSLgmvbuzHD0gbHKSA01XhY4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 20198 invoked by uid 109); 29 Apr 2024 08:21:48 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 29 Apr 2024 08:21:48 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 26841 invoked by uid 111); 29 Apr 2024 08:21:53 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 29 Apr 2024 04:21:53 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 29 Apr 2024 04:21:48 -0400 From: Jeff King To: git@vger.kernel.org Cc: Patrick Steinhardt Subject: [PATCH 3/8] refs: move is_pseudoref_syntax() definition earlier Message-ID: <20240429082148.GB233423@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline We have a static local function is_pseudoref_syntax(), used by is_pseudoref(). The difference being that the former checks only syntax, whereas the latter actually looks at the on-disk refs. There are a few other places in the file where we care about the pseudoref syntax and could make use of this function. Let's move it earlier in the file, so it can be used more extensively without having to forward declare it. Signed-off-by: Jeff King --- refs.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/refs.c b/refs.c index 55d2e0b2cb..d3d21ecd43 100644 --- a/refs.c +++ b/refs.c @@ -164,6 +164,22 @@ void update_ref_namespace(enum ref_namespace namespace, char *ref) info->ref_updated = 1; } +static int is_pseudoref_syntax(const char *refname) +{ + const char *c; + + for (c = refname; *c; c++) { + if (!isupper(*c) && *c != '-' && *c != '_') + return 0; + } + + /* + * HEAD is not a pseudoref, but it certainly uses the + * pseudoref syntax. + */ + return 1; +} + /* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is @@ -844,22 +860,6 @@ int is_per_worktree_ref(const char *refname) starts_with(refname, "refs/rewritten/"); } -static int is_pseudoref_syntax(const char *refname) -{ - const char *c; - - for (c = refname; *c; c++) { - if (!isupper(*c) && *c != '-' && *c != '_') - return 0; - } - - /* - * HEAD is not a pseudoref, but it certainly uses the - * pseudoref syntax. - */ - return 1; -} - int is_pseudoref(struct ref_store *refs, const char *refname) { static const char *const irregular_pseudorefs[] = { From patchwork Mon Apr 29 08:23:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13646455 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 32A9A1863F for ; Mon, 29 Apr 2024 08:23:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378985; cv=none; b=ZHDfZ71/4Hd1tivLw6GPdhYbW7vzqgFvyfDj9/8t62pGv3b7ypoJsvhqEXKNpY/4TpOwBLmFDUxBRIpGIeLNOXwCRtAYSJqHcpiVo4P+OFfl94e3NsW7njPckKewGlVTAiGVlFy4RfQbwV7W/enY8FLyzJzHYvvrqWIfTQn3KWI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714378985; c=relaxed/simple; bh=rmx5PhbYobdDWAXNGp0JUMblTdX6Pzq3nWsOFFzpilQ=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=kKrZK4kP3ED8LPrhKaJs0dmztJr0N0gJcBYGsGZzsHuoTHKOlTpXAaJFODEErpIF4h+ZTEVOeh5avfFKr7nt2L4VreD2/sPfK+AlMzaijoMpKx78pXXJH3trbnqKxwKdO6gqlAeTD6U58lOiuzbiwTIZXmB84cPfmOgswytY2GE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 20204 invoked by uid 109); 29 Apr 2024 08:23:03 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 29 Apr 2024 08:23:03 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 26846 invoked by uid 111); 29 Apr 2024 08:23:07 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 29 Apr 2024 04:23:07 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 29 Apr 2024 04:23:02 -0400 From: Jeff King To: git@vger.kernel.org Cc: Patrick Steinhardt Subject: [PATCH 4/8] refs: disallow dash in pseudoref syntax Message-ID: <20240429082302.GC233423@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline Our is_pseudoref_syntax() function allows upper-case letters, underscore ("_") and dash ("-"). Our glossary definition is vague on the allowed punctuation, but I don't think we have ever used a pseudoref with anything but an underscore. And the existing open-coded syntax check in refname_is_safe() allows only underscores. The logic comes from 266b18273a (refs: add ref_type function, 2015-07-31), but I couldn't find any comment on the dash in the commit message or the list discussion. It's used mostly for is_pseudoref(), which further requires that the name either end in "_HEAD" or be one of a specific set of "irregular" pseudorefs. So I don't think we'd ever see a dash in the real world (you'd need to have "FOO-BAR_HEAD"). Let's tighten this up now so that the function is consistent with (and can be used in) other spots. Signed-off-by: Jeff King --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index d3d21ecd43..b202dca904 100644 --- a/refs.c +++ b/refs.c @@ -169,7 +169,7 @@ static int is_pseudoref_syntax(const char *refname) const char *c; for (c = refname; *c; c++) { - if (!isupper(*c) && *c != '-' && *c != '_') + if (!isupper(*c) && *c != '_') return 0; } From patchwork Mon Apr 29 08:23:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13646456 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 2F2C017BCB for ; Mon, 29 Apr 2024 08:23:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714379003; cv=none; b=Y7eCh/ajp3z/u6wdPH5LdeIQm1041EWUqxCSlB0t9qhHzBLbt6jHydHGpsT2y+RAGhKdcErpgPVUHwVschyIkM7dAu162PxcBJXpOtO4KfS+8Grdqw5tpmIYHPFjA2fLuvNluAISdzmGPcZSmx/bYvIcskDgjG0YxvCBzqiKRAY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714379003; c=relaxed/simple; bh=qA9HjnxJXHzZyL88P6boplP1eGbuPBor4pVOwiLN6aw=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=kg5UweY54lOBWR2Us4YjMZBV+1+8zfXYAhbYwIU7035H5B785FDFfnHPl8E4h0HlFW2R6PWq/KvPTg+YZaqg6vBTPGSaUWmXUHyHKEvHHRabcSYh8KMoOvu0MqG3sh944nev5tkV6AyLNsf8zYa62cYh9yp9DjBOhdCXSVUpKSs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 20211 invoked by uid 109); 29 Apr 2024 08:23:21 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 29 Apr 2024 08:23:21 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 26850 invoked by uid 111); 29 Apr 2024 08:23:25 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 29 Apr 2024 04:23:25 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 29 Apr 2024 04:23:20 -0400 From: Jeff King To: git@vger.kernel.org Cc: Patrick Steinhardt Subject: [PATCH 5/8] refs: use is_pseudoref_syntax() in refname_is_safe() Message-ID: <20240429082320.GD233423@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline The open-coded logic in refname_is_safe() predates the addition of is_pseudoref_syntax(). But now that we have it, we can make use of it. This shortens the code, but also makes sure we use a consistent definition of a pseudoref. Note that refname_is_safe() allows HEAD, which of course is not itself a pseudoref by the strict definition (since it can be a symref). But the syntax for the two is the same, and that is all that we care about here. Signed-off-by: Jeff King --- refs.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/refs.c b/refs.c index b202dca904..8cac7e7e59 100644 --- a/refs.c +++ b/refs.c @@ -362,12 +362,7 @@ int refname_is_safe(const char *refname) return result; } - do { - if (!isupper(*refname) && *refname != '_') - return 0; - refname++; - } while (*refname); - return 1; + return is_pseudoref_syntax(refname); } /* From patchwork Mon Apr 29 08:33:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13646459 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 04F6C14A98 for ; Mon, 29 Apr 2024 08:33:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714379608; cv=none; b=qDqep8oHkfehtJ+dGGyX1NXYr13v0ZIUAIjgZy4mlVWnmrtGGWppLTRGCTyC3FBrQGd+xk185bnG0JP51JsdBEgF6vrzlPa8LbduWPrscrUoFbtIFpD0jUpaq6HUCxwKSw/Tpt4jcmJnAledw4JXsKcBhL6E//bdBLTHI1Cb7Qg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714379608; c=relaxed/simple; bh=yIwv39/wllnm4b7M+R/AQD40sCBlEk4g3ide0F9tzOo=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=N5S6DpOcMcEy/u+mawvNdGNX8026KUWNbF1yN9SjD1LtXJbUKgOxpmRShZ0JLUCmHYU/Kz2d7pK92upH/0+jg50VkXdL7DcW/MoU6vmRbywmvnlIjxX0bHrdvmCnd9rYKB7UiC38NXWdcIMxbn08d9PqAayUA3fI0+JTz555YO0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 20241 invoked by uid 109); 29 Apr 2024 08:33:26 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 29 Apr 2024 08:33:26 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 26930 invoked by uid 111); 29 Apr 2024 08:33:30 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 29 Apr 2024 04:33:30 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 29 Apr 2024 04:33:25 -0400 From: Jeff King To: git@vger.kernel.org Cc: Patrick Steinhardt Subject: [PATCH 6/8] check_refname_format(): add FULLY_QUALIFIED flag Message-ID: <20240429083325.GE233423@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline Before operating on a refname we get from a user, we usually check that it's syntactically valid. As a general rule, refs should be in the "refs/" namespace, the exception being HEAD and pseudorefs like FETCH_HEAD, etc. Those pseudorefs should consist only of all-caps and dash. But the syntactic rules are not enforced by check_refname_format(). So for example we will happily operate on a ref "foo/bar" that will use the file ".git/foo/bar" under the hood (when using the files backend, of course). Making things even more complicated, refname_is_safe() does enforce these syntax restrictions! When that function was added in 2014, we would have refused to work with such refs entirely. But we stopped being so picky in 03afcbee9b (read_packed_refs: avoid double-checking sane refs, 2015-04-16). That rationale there is that check_refname_format() is supposed to contain a superset of the checks of refname_is_safe(). The idea being that we usually would rely on the more-strict check_refname_format(), but for certain operations (e.g., deleting a ref) we want to allow invalid names as long as they are not unsafe (e.g., not escaping the on-disk "refs/" hierarchy). But the pseudoref handling flips this logic; check_refname_format() is more lenient than refname_is_safe(). So you can create "foo/bar" and read it, but you cannot delete it: $ git update-ref foo/bar HEAD $ git rev-parse foo/bar 747a29934757b7e695781e13e2511c43b951da2 $ git update-ref -d foo/bar error: refusing to update ref with bad name 'foo/bar' So we probably want check_ref_format() to learn the same syntactic restrictions that refname_is_safe() uses (namely insisting that anything outside of "refs/" matches the pseudoref syntax). The most obvious way to do that is simply to call refname_is_safe(). But the point of 03afcbee9b is that doing so is expensive. Without the syntactic restrictions of check_refname_format(), refname_is_safe() has to actually normalize the pathname to make sure it does not escape "refs/". That's redundant for us in check_refname_format(); we just need to make sure it either starts with "refs/" or is a pseudoref. But wait, it gets more complicated! We also allow "worktrees/foo/$X" and "main-worktree/$X". In that case we should only be checking "$X" (which should be either a pseudoref or start with "refs/"). We can use parse_worktree_ref(), which fairly efficiently gives us the "bare" refname (even for a non-worktree ref, where it returns the original name). And now when should this new logic kick in? Unfortunately we can't just do it all the time, because many callers pass in partial ref components. E.g., if they are thinking about making "refs/heads/foo", they'll pass us "foo". This is usually accompanied by the ALLOW_ONELEVEL flag. But we likewise can't take the absence of ALLOW_ONELEVEL as a hint that the name is fully qualified, because that flag is also used to indicate that pseudorefs should be allowed! We need a new flag to tell these two situations apart. So let's add a FULLY_QUALIFIED flag that callers can use to ask us to enforce these syntactic rules. There are no callers yet, but we should be able to examine users of ALLOW_ONELEVEL, figure out which semantics they wanted, and convert as needed. Signed-off-by: Jeff King --- The whole ALLOW_ONELEVEL thing is a long-standing confusion, and unfortunately has made it into the hands of users via "git check-ref-format --allow-onelevel". So I think it is there to stay. Possibly we should expose this new feature as --fully-qualified or similar. refs.c | 14 +++++++++++++- refs.h | 1 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 8cac7e7e59..44b4419050 100644 --- a/refs.c +++ b/refs.c @@ -288,6 +288,15 @@ static int check_or_sanitize_refname(const char *refname, int flags, { int component_len, component_count = 0; + if ((flags & REFNAME_FULLY_QUALIFIED)) { + const char *bare_ref; + + parse_worktree_ref(refname, NULL, NULL, &bare_ref); + if (!starts_with(bare_ref, "refs/") && + !is_pseudoref_syntax(bare_ref)) + return -1; + } + if (!strcmp(refname, "@")) { /* Refname is a single character '@'. */ if (sanitized) @@ -322,8 +331,11 @@ static int check_or_sanitize_refname(const char *refname, int flags, else return -1; } - if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2) + + if (!(flags & (REFNAME_ALLOW_ONELEVEL | REFNAME_FULLY_QUALIFIED)) && + component_count < 2) return -1; /* Refname has only one component. */ + return 0; } diff --git a/refs.h b/refs.h index d278775e08..cdd859c8b7 100644 --- a/refs.h +++ b/refs.h @@ -571,6 +571,7 @@ int for_each_reflog(each_reflog_fn fn, void *cb_data); #define REFNAME_ALLOW_ONELEVEL 1 #define REFNAME_REFSPEC_PATTERN 2 +#define REFNAME_FULLY_QUALIFIED 4 /* * Return 0 iff refname has the correct format for a refname according From patchwork Mon Apr 29 08:34:01 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13646460 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 DED141118D for ; Mon, 29 Apr 2024 08:34:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714379644; cv=none; b=p/VwhwvGxS9IW+R97AGgx5k94TrieYgIeWLctzePE+pZZ/zKK/cCERUvTSjspgI9caXHUlb7G/xWh601co2tDikObx+iZnhyrHVQ/8qfyYwtvtkSNv1P1tAJTIBZrJx+NZkN4PWaU8necSHkembJXk/yl39sZbhDD47u0AJvf7M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714379644; c=relaxed/simple; bh=ShSlES6vGSR503zjkHWaPctLjbYBOClKpZyyV1ibo2g=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=fU8zADTv5l/PEjIIu3UQUKFM75g2Tdbn6+O7faq21MqtQvnm24wrEuw7EV6EF/ELWN/gUBZMzPfDtD1NBk0qs38kN8wxYVVQqGNrcNdwvcC7XCC8x93b/FwaTmPI/PCv+Y8hWEQt7HMwJby69/W75Qlaga/kCqboQjujk7kiSBc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 20249 invoked by uid 109); 29 Apr 2024 08:34:02 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 29 Apr 2024 08:34:02 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 26938 invoked by uid 111); 29 Apr 2024 08:34:06 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 29 Apr 2024 04:34:06 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 29 Apr 2024 04:34:01 -0400 From: Jeff King To: git@vger.kernel.org Cc: Patrick Steinhardt Subject: [PATCH 7/8] refs: check refnames as fully qualified when writing Message-ID: <20240429083401.GF233423@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline When a ref update is queued via ref_transaction_update(), we call check_refname_format() to make sure the name is acceptable. We pass REFNAME_ALLOW_ONELEVEL, which allows pseudorefs like MERGE_HEAD. But that's not enough to forbid names outside of refs/ like "foo/bar" or even scary stuff like "config" (though fortunately I think that should never work because we cannot resolve "config" to read the old value). Let's instead pass REFNAME_FULLY_QUALIFIED, which tells the checking function that we really do have a full refname, and it can enforce it as such. This means that "git update-ref foo/bar HEAD" will now be rejected. Note that _deleting_ such a ref is already forbidden (and there is a test in t1430 for that already), due to some confusing differences between check_refname_format() and refname_is_safe(). See the previous commit for more details. Signed-off-by: Jeff King --- refs.c | 2 +- t/t1430-bad-ref-name.sh | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 44b4419050..57663cfe9e 100644 --- a/refs.c +++ b/refs.c @@ -1267,7 +1267,7 @@ int ref_transaction_update(struct ref_transaction *transaction, if (!(flags & REF_SKIP_REFNAME_VERIFICATION) && ((new_oid && !is_null_oid(new_oid)) ? - check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) : + check_refname_format(refname, REFNAME_FULLY_QUALIFIED) : !refname_is_safe(refname))) { strbuf_addf(err, _("refusing to update ref with bad name '%s'"), refname); diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 0c00118c2b..120e1557d7 100755 --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -390,4 +390,14 @@ test_expect_success 'branch -m can rename refs/heads/-dash' ' git show-ref refs/heads/dash ' +test_expect_success 'update-ref refuses lowercase outside of refs/' ' + test_must_fail git update-ref lowercase HEAD 2>err && + test_grep "refusing to update ref with bad name" err +' + +test_expect_success 'update-ref refuses non-underscore outside of refs/' ' + test_must_fail git update-ref FOO/HEAD HEAD 2>err && + test_grep "refusing to update ref with bad name" err +' + test_done