All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Han-Wen Nienhuys <hanwen@google.com>, Jeff King <peff@peff.net>,
	Han-Wen Nienhuys <hanwenn@gmail.com>,
	Han-Wen Nienhuys <hanwen@google.com>
Subject: [PATCH v3 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
Date: Thu, 02 Dec 2021 18:40:00 +0000	[thread overview]
Message-ID: <af9e94935bf6995c1d33cfe5fff1d897bb4119f5.1638470403.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1147.v3.git.git.1638470403.gitgitgadget@gmail.com>

From: Han-Wen Nienhuys <hanwen@google.com>

Use this flag with the test-helper in t1430, to avoid direct writes to the ref
database.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 refs.c                    |  7 +++--
 refs.h                    | 10 +++++--
 t/helper/test-ref-store.c |  1 +
 t/t1430-bad-ref-name.sh   | 59 +++++++++++++++++++++------------------
 4 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/refs.c b/refs.c
index 996ac271641..93bfe5b30cf 100644
--- a/refs.c
+++ b/refs.c
@@ -1083,9 +1083,10 @@ int ref_transaction_update(struct ref_transaction *transaction,
 {
 	assert(err);
 
-	if ((new_oid && !is_null_oid(new_oid)) ?
-	    check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) :
-	    !refname_is_safe(refname)) {
+	if (!(flags & REF_SKIP_REFNAME_VERIFICATION) &&
+	    ((new_oid && !is_null_oid(new_oid)) ?
+		     check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) :
+			   !refname_is_safe(refname))) {
 		strbuf_addf(err, _("refusing to update ref with bad name '%s'"),
 			    refname);
 		return -1;
diff --git a/refs.h b/refs.h
index 76efc589cca..65017ceaefc 100644
--- a/refs.h
+++ b/refs.h
@@ -621,12 +621,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);
  */
 #define REF_SKIP_OID_VERIFICATION (1 << 10)
 
+/*
+ * Skip verifying refname. This is useful for testing data corruption scenarios.
+ */
+#define REF_SKIP_REFNAME_VERIFICATION (1 << 11)
+
 /*
  * Bitmask of all of the flags that are allowed to be passed in to
  * ref_transaction_update() and friends:
  */
-#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \
-	(REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION)
+#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS                                  \
+	(REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION | \
+	 REF_SKIP_REFNAME_VERIFICATION)
 
 /*
  * Add a reference update to transaction. `new_oid` is the value that
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index ac8fa2fe730..78270b3c880 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -134,6 +134,7 @@ static struct flag_definition transaction_flags[] = {
 	FLAG_DEF(REF_NO_DEREF),
 	FLAG_DEF(REF_FORCE_CREATE_REFLOG),
 	FLAG_DEF(REF_SKIP_OID_VERIFICATION),
+	FLAG_DEF(REF_SKIP_REFNAME_VERIFICATION),
 	{ NULL, 0 },
 };
 
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index 4c77cf89a6c..326e5bd5e80 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -9,7 +9,8 @@ TEST_PASSES_SANITIZE_LEAK=true
 
 test_expect_success setup '
 	test_commit one &&
-	test_commit two
+	test_commit two &&
+	main_sha1=$(git rev-parse refs/heads/main)
 '
 
 test_expect_success 'fast-import: fail on invalid branch name ".badbranchname"' '
@@ -43,16 +44,16 @@ test_expect_success 'fast-import: fail on invalid branch name "bad[branch]name"'
 '
 
 test_expect_success 'git branch shows badly named ref as warning' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	git branch >output 2>error &&
 	test_i18ngrep -e "ignoring ref with broken name refs/heads/broken\.\.\.ref" error &&
 	! grep -e "broken\.\.\.ref" output
 '
 
 test_expect_success 'branch -d can delete badly named ref' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	git branch -d broken...ref &&
 	git branch >output 2>error &&
 	! grep -e "broken\.\.\.ref" error &&
@@ -60,8 +61,8 @@ test_expect_success 'branch -d can delete badly named ref' '
 '
 
 test_expect_success 'branch -D can delete badly named ref' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	git branch -D broken...ref &&
 	git branch >output 2>error &&
 	! grep -e "broken\.\.\.ref" error &&
@@ -90,7 +91,7 @@ test_expect_success 'branch -D cannot delete absolute path' '
 '
 
 test_expect_success 'git branch cannot create a badly named ref' '
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	test_must_fail git branch broken...ref &&
 	git branch >output 2>error &&
 	! grep -e "broken\.\.\.ref" error &&
@@ -98,7 +99,7 @@ test_expect_success 'git branch cannot create a badly named ref' '
 '
 
 test_expect_success 'branch -m cannot rename to a bad ref name' '
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	test_might_fail git branch -D goodref &&
 	git branch goodref &&
 	test_must_fail git branch -m goodref broken...ref &&
@@ -109,8 +110,9 @@ test_expect_success 'branch -m cannot rename to a bad ref name' '
 '
 
 test_expect_failure 'branch -m can rename from a bad ref name' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	git branch -m broken...ref renamed &&
 	test_cmp_rev main renamed &&
 	git branch >output 2>error &&
@@ -119,7 +121,7 @@ test_expect_failure 'branch -m can rename from a bad ref name' '
 '
 
 test_expect_success 'push cannot create a badly named ref' '
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	test_must_fail git push "file://$(pwd)" HEAD:refs/heads/broken...ref &&
 	git branch >output 2>error &&
 	! grep -e "broken\.\.\.ref" error &&
@@ -139,7 +141,7 @@ test_expect_failure 'push --mirror can delete badly named ref' '
 		cd dest &&
 		test_commit two &&
 		git checkout --detach &&
-		cp .git/refs/heads/main .git/refs/heads/broken...ref
+		test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION
 	) &&
 	git -C src push --mirror "file://$top/dest" &&
 	git -C dest branch >output 2>error &&
@@ -148,9 +150,9 @@ test_expect_failure 'push --mirror can delete badly named ref' '
 '
 
 test_expect_success 'rev-parse skips symref pointing to broken name' '
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	git branch shadow one &&
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
 	printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow &&
 	test_when_finished "rm -f .git/refs/tags/shadow" &&
 	git rev-parse --verify one >expect &&
@@ -160,8 +162,8 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
 '
 
 test_expect_success 'for-each-ref emits warnings for broken names' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
 	test_when_finished "rm -f .git/refs/heads/badname" &&
 	printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
@@ -176,8 +178,8 @@ test_expect_success 'for-each-ref emits warnings for broken names' '
 '
 
 test_expect_success 'update-ref -d can delete broken name' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	git update-ref -d refs/heads/broken...ref >output 2>error &&
 	test_must_be_empty output &&
 	test_must_be_empty error &&
@@ -187,8 +189,8 @@ test_expect_success 'update-ref -d can delete broken name' '
 '
 
 test_expect_success 'branch -d can delete broken name' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	git branch -d broken...ref >output 2>error &&
 	test_i18ngrep "Deleted branch broken...ref (was broken)" output &&
 	test_must_be_empty error &&
@@ -198,8 +200,9 @@ test_expect_success 'branch -d can delete broken name' '
 '
 
 test_expect_success 'update-ref --no-deref -d can delete symref to broken name' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
 	test_when_finished "rm -f .git/refs/heads/badname" &&
 	git update-ref --no-deref -d refs/heads/badname >output 2>error &&
@@ -209,8 +212,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
 '
 
 test_expect_success 'branch -d can delete symref to broken name' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
 	test_when_finished "rm -f .git/refs/heads/badname" &&
 	git branch -d badname >output 2>error &&
@@ -238,8 +242,9 @@ test_expect_success 'branch -d can delete dangling symref to broken name' '
 '
 
 test_expect_success 'update-ref -d can delete broken name through symref' '
-	cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
+	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
 	printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
 	test_when_finished "rm -f .git/refs/heads/badname" &&
 	git update-ref -d refs/heads/badname >output 2>error &&
-- 
gitgitgadget


  parent reply	other threads:[~2021-12-02 18:40 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-25 15:55 [PATCH 0/2] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget
2021-11-25 15:56 ` [PATCH 1/2] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-11-25 15:56 ` [PATCH 2/2] refs: allow skipping OID verification Han-Wen Nienhuys via GitGitGadget
2021-11-26  7:32   ` Junio C Hamano
2021-11-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget
2021-11-29 18:49   ` [PATCH v2 1/6] test-ref-store: plug memory leak in cmd_delete_refs Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:15     ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 2/6] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:17     ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 3/6] refs: allow skipping OID verification Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:28     ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:22     ` Junio C Hamano
2021-11-29 23:31     ` Junio C Hamano
2021-11-30 10:31       ` Han-Wen Nienhuys
2021-12-01 19:00         ` Junio C Hamano
2021-12-01 19:26           ` Jeff King
2021-12-02 16:40             ` Han-Wen Nienhuys
2021-12-02 19:05               ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 5/6] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget
2021-11-29 18:49   ` [PATCH v2 6/6] t1430: create valid symrefs using test-helper Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:12   ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Junio C Hamano
2021-12-02 18:39   ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 1/8] test-ref-store: remove force-create argument for create-reflog Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 2/8] test-ref-store: parse symbolic flag constants Han-Wen Nienhuys via GitGitGadget
2021-12-03  6:22       ` Jeff King
2021-12-02 18:39     ` [PATCH v3 3/8] test-ref-store: plug memory leak in cmd_delete_refs Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 5/8] refs: introduce REF_SKIP_OID_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:40     ` Han-Wen Nienhuys via GitGitGadget [this message]
2021-12-02 18:40     ` [PATCH v3 7/8] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:40     ` [PATCH v3 8/8] t1430: create valid symrefs using test-helper Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38     ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 1/8] test-ref-store: remove force-create argument for create-reflog Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 2/8] test-ref-store: parse symbolic flag constants Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 3/8] test-ref-store: plug memory leak in cmd_delete_refs Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 5/8] refs: introduce REF_SKIP_OID_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 7/8] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 8/8] t1430: create valid symrefs using test-helper Han-Wen Nienhuys via GitGitGadget
2021-12-07 21:18       ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=af9e94935bf6995c1d33cfe5fff1d897bb4119f5.1638470403.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.