* [PATCH 0/2] Allow writing invalid OIDs into refs for testing purposes
@ 2021-11-25 15:55 Han-Wen Nienhuys via GitGitGadget
2021-11-25 15:56 ` [PATCH 1/2] refs: update comment Han-Wen Nienhuys via GitGitGadget
` (2 more replies)
0 siblings, 3 replies; 42+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-25 15:55 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
this covers a few cases of direct filesystem access to the ref database in
tests.
Han-Wen Nienhuys (2):
refs: update comment.
refs: allow skipping OID verification
refs.h | 8 ++++++-
refs/files-backend.c | 53 +++++++++++++++++++++++++-------------------
t/t1006-cat-file.sh | 5 ++---
t/t3800-mktag.sh | 6 +++--
4 files changed, 43 insertions(+), 29 deletions(-)
base-commit: 5f439a0ecfb4657100ec1e56ef9c6eca963b5a94
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v1
Pull-Request: https://github.com/git/git/pull/1147
--
gitgitgadget
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 1/2] refs: update comment. 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 ` 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-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget 2 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-25 15:56 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the `ref_update` flags"). Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 151b0056fe5..5cfdec1e820 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -16,8 +16,7 @@ * This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, - * REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in - * `ref_update::flags`. + * or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /* -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/2] refs: allow skipping OID verification 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 ` 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 2 siblings, 1 reply; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-25 15:56 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This introduces the REF_SKIP_OID_VERIFICATION flag, which lets the ref-store test helper write non-existent or unparsable objects into the ref storage. Use this to make t1006 and t3800 independent of the files storage backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++++------------------- t/t1006-cat-file.sh | 5 ++--- t/t3800-mktag.sh | 6 ++++-- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/refs.h b/refs.h index d5099d4984e..46e68fd4c2a 100644 --- a/refs.h +++ b/refs.h @@ -611,12 +611,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); */ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* + * Blindly write an object_id. This is useful for testing data corruption + * scenarios. + */ +#define REF_SKIP_OID_VERIFICATION (1 << 10) + /* * 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_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION) /* * Add a reference update to transaction. `new_oid` is the value that diff --git a/refs/files-backend.c b/refs/files-backend.c index 5cfdec1e820..28a349b24ad 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1351,7 +1351,8 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname) } static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err); + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err); static int commit_ref_update(struct files_ref_store *refs, struct ref_lock *lock, const struct object_id *oid, const char *logmsg, @@ -1468,7 +1469,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, } oidcpy(&lock->old_oid, &orig_oid); - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, logmsg, &err)) { error("unable to write current sha1 into %s: %s", newrefname, err.buf); strbuf_release(&err); @@ -1488,7 +1489,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, flag = log_all_ref_updates; log_all_ref_updates = LOG_REFS_NONE; - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, NULL, &err)) { error("unable to write current sha1 into %s: %s", oldrefname, err.buf); strbuf_release(&err); @@ -1724,26 +1725,31 @@ static int files_log_ref_write(struct files_ref_store *refs, * errors, rollback the lockfile, fill in *err and return -1. */ static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err) + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err) { static char term = '\n'; struct object *o; int fd; - o = parse_object(the_repository, oid); - if (!o) { - strbuf_addf(err, - "trying to write ref '%s' with nonexistent object %s", - lock->ref_name, oid_to_hex(oid)); - unlock_ref(lock); - return -1; - } - if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { - strbuf_addf(err, - "trying to write non-commit object %s to branch '%s'", - oid_to_hex(oid), lock->ref_name); - unlock_ref(lock); - return -1; + if (!skip_oid_verification) { + o = parse_object(the_repository, oid); + if (!o) { + strbuf_addf( + err, + "trying to write ref '%s' with nonexistent object %s", + lock->ref_name, oid_to_hex(oid)); + unlock_ref(lock); + return -1; + } + if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { + strbuf_addf( + err, + "trying to write non-commit object %s to branch '%s'", + oid_to_hex(oid), lock->ref_name); + unlock_ref(lock); + return -1; + } } fd = get_lock_file_fd(&lock->lk); if (write_in_full(fd, oid_to_hex(oid), the_hash_algo->hexsz) < 0 || @@ -2150,7 +2156,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) } static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) + struct object_id *peeled) { BUG("ref_iterator_peel() called for reflog_iterator"); } @@ -2534,8 +2540,10 @@ static int lock_ref_for_update(struct files_ref_store *refs, * The reference already has the desired * value, so we don't need to write it. */ - } else if (write_ref_to_lockfile(lock, &update->new_oid, - err)) { + } else if (write_ref_to_lockfile( + lock, &update->new_oid, + update->flags & REF_SKIP_OID_VERIFICATION, + err)) { char *write_err = strbuf_detach(err, NULL); /* diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 658628375c8..f5333196b07 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 && cat >expect <<-EOF && commit diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh index 0544d58a6ea..128d374f740 100755 --- a/t/t3800-mktag.sh +++ b/t/t3800-mktag.sh @@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && # Unlike fsck-ing unreachable content above, this # will always fail. @@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 2/2] refs: allow skipping OID verification 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 0 siblings, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-11-26 7:32 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > This introduces the REF_SKIP_OID_VERIFICATION flag, which lets the ref-store > test helper write non-existent or unparsable objects into the ref storage. Yay. > diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh > index 658628375c8..f5333196b07 100755 > --- a/t/t1006-cat-file.sh > +++ b/t/t1006-cat-file.sh > @@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme > # Create it manually, as "git replace" will die on bogus > # types. > head=$(git rev-parse --verify HEAD) && > - test_when_finished "rm -rf .git/refs/replace" && > - mkdir -p .git/refs/replace && > - echo $head >.git/refs/replace/$bogus_short_sha1 && > + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && > + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 && Double yay. Tests that are independent of backend is indeed much nicer. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes 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-29 18:49 ` 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 ` (7 more replies) 2 siblings, 8 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys this covers a few cases of direct filesystem access to the ref database in tests. v2 folds in more fixes for making t1430 reftable-proof. * add REF_SKIP_REFNAME_VERIFICATION as well * upgrade some more bits of t1430 Han-Wen Nienhuys (6): test-ref-store: plug memory leak in cmd_delete_refs refs: update comment. refs: allow skipping OID verification refs: add REF_SKIP_REFNAME_VERIFICATION flag t1430: remove refs using test-tool t1430: create valid symrefs using test-helper refs.c | 7 +-- refs.h | 16 ++++++- refs/files-backend.c | 53 ++++++++++++---------- t/helper/test-ref-store.c | 6 ++- t/t1006-cat-file.sh | 5 +-- t/t1430-bad-ref-name.sh | 95 ++++++++++++++++++++------------------- t/t3800-mktag.sh | 6 ++- 7 files changed, 108 insertions(+), 80 deletions(-) base-commit: 35151cf0720460a897cde9b8039af364743240e7 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v2 Pull-Request: https://github.com/git/git/pull/1147 Range-diff vs v1: -: ----------- > 1: 7fa5c247c8b test-ref-store: plug memory leak in cmd_delete_refs 1: b83bfda2443 = 2: 82971ddbfcf refs: update comment. 2: 900cea2ade9 = 3: 93fe3f03fb2 refs: allow skipping OID verification -: ----------- > 4: 0a297f0c3e8 refs: add REF_SKIP_REFNAME_VERIFICATION flag -: ----------- > 5: 06c8ff7df15 t1430: remove refs using test-tool -: ----------- > 6: 3c100702fda t1430: create valid symrefs using test-helper -- gitgitgadget ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 1/6] test-ref-store: plug memory leak in cmd_delete_refs 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 ` 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 ` (6 subsequent siblings) 7 siblings, 1 reply; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/helper/test-ref-store.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index b314b81a45b..ccc2bb01bf3 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -86,11 +86,13 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv) unsigned int flags = arg_flags(*argv++, "flags"); const char *msg = *argv++; struct string_list refnames = STRING_LIST_INIT_NODUP; - + int result; while (*argv) string_list_append(&refnames, *argv++); - return refs_delete_refs(refs, msg, &refnames, flags); + result = refs_delete_refs(refs, msg, &refnames, flags); + string_list_clear(&refnames, 0); + return result; } static int cmd_rename_ref(struct ref_store *refs, const char **argv) -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 1/6] test-ref-store: plug memory leak in cmd_delete_refs 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 0 siblings, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-11-29 23:15 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > t/helper/test-ref-store.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c > index b314b81a45b..ccc2bb01bf3 100644 > --- a/t/helper/test-ref-store.c > +++ b/t/helper/test-ref-store.c > @@ -86,11 +86,13 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv) > unsigned int flags = arg_flags(*argv++, "flags"); > const char *msg = *argv++; > struct string_list refnames = STRING_LIST_INIT_NODUP; > - > + int result; Thanks, but let's not lose the blank line between the declarations and the first statement, which serves for readability. > while (*argv) > string_list_append(&refnames, *argv++); > > - return refs_delete_refs(refs, msg, &refnames, flags); > + result = refs_delete_refs(refs, msg, &refnames, flags); > + string_list_clear(&refnames, 0); > + return result; > } > > static int cmd_rename_ref(struct ref_store *refs, const char **argv) ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 2/6] refs: update comment. 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 18:49 ` 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 ` (5 subsequent siblings) 7 siblings, 1 reply; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the `ref_update` flags"). Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 151b0056fe5..5cfdec1e820 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -16,8 +16,7 @@ * This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, - * REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in - * `ref_update::flags`. + * or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /* -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 2/6] refs: update comment. 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 0 siblings, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-11-29 23:17 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > REF_IS_PRUNING is right below this comment, so it clearly does not belong in > this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 > "refs: tidy up and adjust visibility of the `ref_update` flags"). Thanks for carefully reading and correcting. > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > refs/files-backend.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/refs/files-backend.c b/refs/files-backend.c > index 151b0056fe5..5cfdec1e820 100644 > --- a/refs/files-backend.c > +++ b/refs/files-backend.c > @@ -16,8 +16,7 @@ > * This backend uses the following flags in `ref_update::flags` for > * internal bookkeeping purposes. Their numerical values must not > * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, > - * REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in > - * `ref_update::flags`. > + * or REF_HAVE_OLD, which are also stored in `ref_update::flags`. > */ > > /* ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 3/6] refs: allow skipping OID verification 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 18:49 ` [PATCH v2 2/6] refs: update comment Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 ` 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 ` (4 subsequent siblings) 7 siblings, 1 reply; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This introduces the REF_SKIP_OID_VERIFICATION flag, which lets the ref-store test helper write non-existent or unparsable objects into the ref storage. Use this to make t1006 and t3800 independent of the files storage backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++++------------------- t/t1006-cat-file.sh | 5 ++--- t/t3800-mktag.sh | 6 ++++-- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/refs.h b/refs.h index d5099d4984e..46e68fd4c2a 100644 --- a/refs.h +++ b/refs.h @@ -611,12 +611,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); */ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* + * Blindly write an object_id. This is useful for testing data corruption + * scenarios. + */ +#define REF_SKIP_OID_VERIFICATION (1 << 10) + /* * 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_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION) /* * Add a reference update to transaction. `new_oid` is the value that diff --git a/refs/files-backend.c b/refs/files-backend.c index 5cfdec1e820..28a349b24ad 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1351,7 +1351,8 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname) } static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err); + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err); static int commit_ref_update(struct files_ref_store *refs, struct ref_lock *lock, const struct object_id *oid, const char *logmsg, @@ -1468,7 +1469,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, } oidcpy(&lock->old_oid, &orig_oid); - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, logmsg, &err)) { error("unable to write current sha1 into %s: %s", newrefname, err.buf); strbuf_release(&err); @@ -1488,7 +1489,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, flag = log_all_ref_updates; log_all_ref_updates = LOG_REFS_NONE; - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, NULL, &err)) { error("unable to write current sha1 into %s: %s", oldrefname, err.buf); strbuf_release(&err); @@ -1724,26 +1725,31 @@ static int files_log_ref_write(struct files_ref_store *refs, * errors, rollback the lockfile, fill in *err and return -1. */ static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err) + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err) { static char term = '\n'; struct object *o; int fd; - o = parse_object(the_repository, oid); - if (!o) { - strbuf_addf(err, - "trying to write ref '%s' with nonexistent object %s", - lock->ref_name, oid_to_hex(oid)); - unlock_ref(lock); - return -1; - } - if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { - strbuf_addf(err, - "trying to write non-commit object %s to branch '%s'", - oid_to_hex(oid), lock->ref_name); - unlock_ref(lock); - return -1; + if (!skip_oid_verification) { + o = parse_object(the_repository, oid); + if (!o) { + strbuf_addf( + err, + "trying to write ref '%s' with nonexistent object %s", + lock->ref_name, oid_to_hex(oid)); + unlock_ref(lock); + return -1; + } + if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { + strbuf_addf( + err, + "trying to write non-commit object %s to branch '%s'", + oid_to_hex(oid), lock->ref_name); + unlock_ref(lock); + return -1; + } } fd = get_lock_file_fd(&lock->lk); if (write_in_full(fd, oid_to_hex(oid), the_hash_algo->hexsz) < 0 || @@ -2150,7 +2156,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) } static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) + struct object_id *peeled) { BUG("ref_iterator_peel() called for reflog_iterator"); } @@ -2534,8 +2540,10 @@ static int lock_ref_for_update(struct files_ref_store *refs, * The reference already has the desired * value, so we don't need to write it. */ - } else if (write_ref_to_lockfile(lock, &update->new_oid, - err)) { + } else if (write_ref_to_lockfile( + lock, &update->new_oid, + update->flags & REF_SKIP_OID_VERIFICATION, + err)) { char *write_err = strbuf_detach(err, NULL); /* diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 658628375c8..f5333196b07 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 && cat >expect <<-EOF && commit diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh index 0544d58a6ea..128d374f740 100755 --- a/t/t3800-mktag.sh +++ b/t/t3800-mktag.sh @@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && # Unlike fsck-ing unreachable content above, this # will always fail. @@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 3/6] refs: allow skipping OID verification 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 0 siblings, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-11-29 23:28 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > +/* > + * Blindly write an object_id. This is useful for testing data corruption > + * scenarios. > + */ > +#define REF_SKIP_OID_VERIFICATION (1 << 10) That makes sense. Readers would expect that this flag is passed only from test-helper but not in the regular code, at least without an option like "git update-ref --skip-oid-verification ..."? After applying these 6 patches, I do not seem to find anybody who actually flips this bit on. Perhaps I missed a step or so? ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 2021-11-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (2 preceding siblings ...) 2021-11-29 18:49 ` [PATCH v2 3/6] refs: allow skipping OID verification Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 ` Han-Wen Nienhuys via GitGitGadget 2021-11-29 23:22 ` Junio C Hamano 2021-11-29 23:31 ` Junio C Hamano 2021-11-29 18:49 ` [PATCH v2 5/6] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget ` (3 subsequent siblings) 7 siblings, 2 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys 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/t1430-bad-ref-name.sh | 59 ++++++++++++++++++++++------------------- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/refs.c b/refs.c index d7cc0a23a3b..086b1341d2a 100644 --- a/refs.c +++ b/refs.c @@ -1078,9 +1078,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 46e68fd4c2a..684f8b33d17 100644 --- a/refs.h +++ b/refs.h @@ -617,12 +617,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/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 4c77cf89a6c..56d0726944e 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 2048 && + test_when_finished "test-tool ref-store main delete-refs 1 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 2048 && + test_when_finished "test-tool ref-store main delete-refs 1 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 2048 && + test_when_finished "test-tool ref-store main delete-refs 1 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 1 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 1 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 2048 && + + test_when_finished "test-tool ref-store main delete-refs 1 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 1 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 2048 ) && 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 1 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 2048 && 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 2048 && + test_when_finished "test-tool ref-store main delete-refs 1 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 2048 && + test_when_finished "test-tool ref-store main delete-refs 1 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 2048 && + test_when_finished "test-tool ref-store main delete-refs 1 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 2048 && + + test_when_finished "test-tool ref-store main delete-refs 1 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 2048 && + + test_when_finished "test-tool ref-store main delete-refs 1 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 2048 && + + test_when_finished "test-tool ref-store main delete-refs 1 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 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 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 1 sibling, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-11-29 23:22 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > Use this flag with the test-helper in t1430, to avoid direct writes to the ref > database. OK, this is similar to 3/6 where invalid object names are allowed. This is about allowing invalid refnames. Makes sense. > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > refs.c | 7 ++--- > refs.h | 10 +++++-- > t/t1430-bad-ref-name.sh | 59 ++++++++++++++++++++++------------------- > 3 files changed, 44 insertions(+), 32 deletions(-) > > diff --git a/refs.c b/refs.c > index d7cc0a23a3b..086b1341d2a 100644 > --- a/refs.c > +++ b/refs.c > @@ -1078,9 +1078,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))) { OK. Initially my eyes went "Huh?", but it was just re-indentation that fooled them. > @@ -617,12 +617,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) OK. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 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 1 sibling, 1 reply; 42+ messages in thread From: Junio C Hamano @ 2021-11-29 23:31 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > + if (!(flags & REF_SKIP_REFNAME_VERIFICATION) && > + ((new_oid && !is_null_oid(new_oid)) ? > + check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) : > + !refname_is_safe(refname))) { So, if somebody passes REF_SKIP_REFNAME_VERIFICATION in flags, we will not do the check. Again, like 3/6, this new bit is flipped on by test-helper somewhere? Again I do not see anybody doing so in these 6 patches, but I should double check. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 2021-11-29 23:31 ` Junio C Hamano @ 2021-11-30 10:31 ` Han-Wen Nienhuys 2021-12-01 19:00 ` Junio C Hamano 0 siblings, 1 reply; 42+ messages in thread From: Han-Wen Nienhuys @ 2021-11-30 10:31 UTC (permalink / raw) To: Junio C Hamano; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys On Tue, Nov 30, 2021 at 12:31 AM Junio C Hamano <gitster@pobox.com> wrote: > > + if (!(flags & REF_SKIP_REFNAME_VERIFICATION) && > > + ((new_oid && !is_null_oid(new_oid)) ? > > + check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) : > > + !refname_is_safe(refname))) { > > So, if somebody passes REF_SKIP_REFNAME_VERIFICATION in flags, we > will not do the check. > > Again, like 3/6, this new bit is flipped on by test-helper > somewhere? Again I do not see anybody doing so in these 6 patches, > but I should double check. The test helper takes the flag as an argument, in decimal. If you look for 2048, you should find it. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 2021-11-30 10:31 ` Han-Wen Nienhuys @ 2021-12-01 19:00 ` Junio C Hamano 2021-12-01 19:26 ` Jeff King 0 siblings, 1 reply; 42+ messages in thread From: Junio C Hamano @ 2021-12-01 19:00 UTC (permalink / raw) To: Han-Wen Nienhuys; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys Han-Wen Nienhuys <hanwen@google.com> writes: > On Tue, Nov 30, 2021 at 12:31 AM Junio C Hamano <gitster@pobox.com> wrote: >> > + if (!(flags & REF_SKIP_REFNAME_VERIFICATION) && >> > + ((new_oid && !is_null_oid(new_oid)) ? >> > + check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) : >> > + !refname_is_safe(refname))) { >> >> So, if somebody passes REF_SKIP_REFNAME_VERIFICATION in flags, we >> will not do the check. >> >> Again, like 3/6, this new bit is flipped on by test-helper >> somewhere? Again I do not see anybody doing so in these 6 patches, >> but I should double check. > > The test helper takes the flag as an argument, in decimal. If you look > for 2048, you should find it. Awful---when the symbolic constants change in the code, the test will silently break? It has been this way since 80f2a609 (t/helper: add test-ref-store to test ref-store functions, 2017-03-26), so it is nothing new, but at some point, we should do a better job. Even if it is used only as a tool for testing, we shouldn't have to force developers to write in assembly ;-) Perhaps when the dust settles after this series graduates to be a part of released version. It may be a good bite-sized microproject material for aspiring new developers. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 2021-12-01 19:00 ` Junio C Hamano @ 2021-12-01 19:26 ` Jeff King 2021-12-02 16:40 ` Han-Wen Nienhuys 0 siblings, 1 reply; 42+ messages in thread From: Jeff King @ 2021-12-01 19:26 UTC (permalink / raw) To: Junio C Hamano Cc: Han-Wen Nienhuys, Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys On Wed, Dec 01, 2021 at 11:00:04AM -0800, Junio C Hamano wrote: > > The test helper takes the flag as an argument, in decimal. If you look > > for 2048, you should find it. > > Awful---when the symbolic constants change in the code, the test > will silently break? Agreed, this is quite nasty. > It has been this way since 80f2a609 (t/helper: add test-ref-store to > test ref-store functions, 2017-03-26), so it is nothing new, but at > some point, we should do a better job. Even if it is used only as a > tool for testing, we shouldn't have to force developers to write in > assembly ;-) Sort of. The code to pass the flags was added then, but nobody was using it until now. At least for ref updates. Symref creation allowed "1" for force, but that is a true boolean. It looks like some pack-refs calls pass "3" for PACK_REFS_PRUNE|PACK_REFS_ALL. So it may be considered a "new" issue in that sense. > Perhaps when the dust settles after this series graduates to be a > part of released version. It may be a good bite-sized microproject > material for aspiring new developers. It is annoying to have to plumb through the names of the flags, but in practice I don't think we need all (or even most) of them. And it's equally annoying not to be able to grep for the flags. I already had a hard enough time just grepping for the callers, and resorted to: diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 3986665037..1b7a124a86 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -271,6 +271,8 @@ int cmd__ref_store(int argc, const char **argv) const char *func; struct command *cmd; + trace_argv_printf(argv, "trace: test-tool:"); + setup_git_directory(); argv = get_store(argv + 1, &refs); to find them. -Peff ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 2021-12-01 19:26 ` Jeff King @ 2021-12-02 16:40 ` Han-Wen Nienhuys 2021-12-02 19:05 ` Junio C Hamano 0 siblings, 1 reply; 42+ messages in thread From: Han-Wen Nienhuys @ 2021-12-02 16:40 UTC (permalink / raw) To: Jeff King Cc: Junio C Hamano, Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys On Wed, Dec 1, 2021 at 8:26 PM Jeff King <peff@peff.net> wrote: > > On Wed, Dec 01, 2021 at 11:00:04AM -0800, Junio C Hamano wrote: > > > > The test helper takes the flag as an argument, in decimal. If you look > > > for 2048, you should find it. > > > > Awful---when the symbolic constants change in the code, the test > > will silently break? > > Agreed, this is quite nasty. I've added parsing symbolic constants in v3. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag 2021-12-02 16:40 ` Han-Wen Nienhuys @ 2021-12-02 19:05 ` Junio C Hamano 0 siblings, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-12-02 19:05 UTC (permalink / raw) To: Han-Wen Nienhuys Cc: Jeff King, Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys Han-Wen Nienhuys <hanwen@google.com> writes: > On Wed, Dec 1, 2021 at 8:26 PM Jeff King <peff@peff.net> wrote: >> >> On Wed, Dec 01, 2021 at 11:00:04AM -0800, Junio C Hamano wrote: >> >> > > The test helper takes the flag as an argument, in decimal. If you look >> > > for 2048, you should find it. >> > >> > Awful---when the symbolic constants change in the code, the test >> > will silently break? >> >> Agreed, this is quite nasty. > > I've added parsing symbolic constants in v3. Thanks for going an extra mile. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 5/6] t1430: remove refs using test-tool 2021-11-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (3 preceding siblings ...) 2021-11-29 18:49 ` [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 ` 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 ` (2 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t1430-bad-ref-name.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 56d0726944e..5c600980b5f 100755 --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -154,7 +154,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && - test_when_finished "rm -f .git/refs/tags/shadow" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && test_cmp expect actual && @@ -165,9 +165,9 @@ test_expect_success 'for-each-ref emits warnings for broken names' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 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" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git for-each-ref >output 2>error && ! grep -e "broken\.\.\.ref" output && ! grep -e "badname" output && @@ -204,7 +204,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test_when_finished "test-tool ref-store main delete-refs 1 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" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ -216,7 +216,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_when_finished "test-tool ref-store main delete-refs 1 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" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ -225,7 +225,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ -234,7 +234,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok test_expect_success 'branch -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ -246,7 +246,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_when_finished "test-tool ref-store main delete-refs 1 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" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && test_must_be_empty output && @@ -255,7 +255,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_expect_success 'update-ref --no-deref -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ -264,7 +264,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name test_expect_success 'branch -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output && @@ -273,7 +273,7 @@ test_expect_success 'branch -d can delete symref with broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ -282,7 +282,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br test_expect_success 'branch -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] t1430: create valid symrefs using test-helper 2021-11-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (4 preceding siblings ...) 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 ` 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 7 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-11-29 18:49 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This still leaves some other direct filesystem access. Currently, the files backend does not allow invalidly named symrefs. Fixes for this are currently in the 'seen' branch Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t1430-bad-ref-name.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 5c600980b5f..05a4ee712ca 100755 --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -153,7 +153,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && - printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && + test-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && @@ -203,7 +203,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -215,7 +215,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -224,7 +224,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -233,7 +233,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok ' test_expect_success 'branch -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -245,7 +245,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes 2021-11-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (5 preceding siblings ...) 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 ` Junio C Hamano 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget 7 siblings, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-11-29 23:12 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > this covers a few cases of direct filesystem access to the ref database in > tests. > > v2 folds in more fixes for making t1430 reftable-proof. > > * add REF_SKIP_REFNAME_VERIFICATION as well > * upgrade some more bits of t1430 Thanks, will replace. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 0/8] Allow writing invalid OIDs into refs for testing purposes 2021-11-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (6 preceding siblings ...) 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 ` 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 ` (8 more replies) 7 siblings, 9 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys this covers a few cases of direct filesystem access to the ref database in tests. v3: * parse symbolic constants in ref-store test-helper. Han-Wen Nienhuys (8): test-ref-store: remove force-create argument for create-reflog test-ref-store: parse symbolic flag constants test-ref-store: plug memory leak in cmd_delete_refs refs: update comment. refs: introduce REF_SKIP_OID_VERIFICATION flag refs: introduce REF_SKIP_REFNAME_VERIFICATION flag t1430: remove refs using test-tool t1430: create valid symrefs using test-helper refs.c | 7 +-- refs.h | 16 +++++- refs/files-backend.c | 53 ++++++++++--------- t/helper/test-ref-store.c | 78 ++++++++++++++++++++++++---- t/t1006-cat-file.sh | 5 +- t/t1405-main-ref-store.sh | 8 ++- t/t1406-submodule-ref-store.sh | 2 +- t/t1430-bad-ref-name.sh | 93 ++++++++++++++++++---------------- t/t3800-mktag.sh | 6 ++- 9 files changed, 175 insertions(+), 93 deletions(-) base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v3 Pull-Request: https://github.com/git/git/pull/1147 Range-diff vs v2: -: ----------- > 1: 3649ef6d0fa test-ref-store: remove force-create argument for create-reflog -: ----------- > 2: 3cdebd2dbca test-ref-store: parse symbolic flag constants 1: 7fa5c247c8b ! 3: 536005d65a2 test-ref-store: plug memory leak in cmd_delete_refs @@ Commit message ## t/helper/test-ref-store.c ## @@ t/helper/test-ref-store.c: static int cmd_delete_refs(struct ref_store *refs, const char **argv) - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); const char *msg = *argv++; struct string_list refnames = STRING_LIST_INIT_NODUP; -- + int result; + while (*argv) string_list_append(&refnames, *argv++); 2: 82971ddbfcf = 4: 466b4451015 refs: update comment. 3: 93fe3f03fb2 ! 5: 89d692a34b8 refs: allow skipping OID verification @@ Metadata Author: Han-Wen Nienhuys <hanwen@google.com> ## Commit message ## - refs: allow skipping OID verification + refs: introduce REF_SKIP_OID_VERIFICATION flag - This introduces the REF_SKIP_OID_VERIFICATION flag, which lets the ref-store - test helper write non-existent or unparsable objects into the ref storage. + This lets the ref-store test helper write non-existent or unparsable objects + into the ref storage. Use this to make t1006 and t3800 independent of the files storage backend. @@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *ref /* + ## t/helper/test-ref-store.c ## +@@ t/helper/test-ref-store.c: static int cmd_create_symref(struct ref_store *refs, const char **argv) + static struct flag_definition transaction_flags[] = { + FLAG_DEF(REF_NO_DEREF), + FLAG_DEF(REF_FORCE_CREATE_REFLOG), ++ FLAG_DEF(REF_SKIP_OID_VERIFICATION), + { NULL, 0 }, + }; + + ## t/t1006-cat-file.sh ## @@ t/t1006-cat-file.sh: test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus @@ t/t1006-cat-file.sh: test_expect_success 'the --allow-unknown-type option does n - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && -+ test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 && ++ test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID REF_SKIP_OID_VERIFICATION && cat >expect <<-EOF && commit @@ t/t3800-mktag.sh: check_verify_failure () { # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && -+ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && ++ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && # Unlike fsck-ing unreachable content above, this # will always fail. @@ t/t3800-mktag.sh: check_verify_failure () { - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && -+ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && ++ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual && 4: 0a297f0c3e8 ! 6: af9e94935bf refs: add REF_SKIP_REFNAME_VERIFICATION flag @@ Metadata Author: Han-Wen Nienhuys <hanwen@google.com> ## Commit message ## - refs: add REF_SKIP_REFNAME_VERIFICATION flag + refs: introduce REF_SKIP_REFNAME_VERIFICATION flag Use this flag with the test-helper in t1430, to avoid direct writes to the ref database. @@ refs.h: struct ref_transaction *ref_transaction_begin(struct strbuf *err); /* * Add a reference update to transaction. `new_oid` is the value that + ## t/helper/test-ref-store.c ## +@@ t/helper/test-ref-store.c: 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 }, + }; + + ## t/t1430-bad-ref-name.sh ## @@ t/t1430-bad-ref-name.sh: TEST_PASSES_SANITIZE_LEAK=true @@ t/t1430-bad-ref-name.sh: test_expect_success 'fast-import: fail on invalid branc 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 2048 && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg 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 @@ t/t1430-bad-ref-name.sh: test_expect_success 'fast-import: fail on invalid branc 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 2048 && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete badly named r 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 2048 && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -D cannot delete absolute p 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 1 msg 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'git branch cannot create a badly n 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 1 msg 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -m cannot rename to a bad r 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 2048 && ++ 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 1 msg refs/heads/broken...ref" && ++ 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 && @@ t/t1430-bad-ref-name.sh: test_expect_failure 'branch -m can rename from a bad re 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 1 msg 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 && @@ t/t1430-bad-ref-name.sh: test_expect_failure 'push --mirror can delete badly nam 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 2048 ++ 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 && @@ t/t1430-bad-ref-name.sh: test_expect_failure 'push --mirror can delete badly nam 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 1 msg 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 2048 && ++ 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'rev-parse skips symref pointing to 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 2048 && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'for-each-ref emits warnings for br 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 2048 && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken na 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 2048 && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg 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 && @@ t/t1430-bad-ref-name.sh: 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 2048 && ++ 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 1 msg refs/heads/broken...ref" && ++ 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet 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 2048 && ++ 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 1 msg refs/heads/broken...ref" && ++ 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 && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete dangling symr 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 2048 && ++ 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 1 msg refs/heads/broken...ref" && ++ 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 && 5: 06c8ff7df15 ! 7: d4f1d209468 t1430: remove refs using test-tool @@ Commit message ## t/t1430-bad-ref-name.sh ## @@ t/t1430-bad-ref-name.sh: test_expect_success 'rev-parse skips symref pointing to broken name' ' git branch shadow one && - test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && + 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" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && test_cmp expect actual && @@ t/t1430-bad-ref-name.sh: test_expect_success 'for-each-ref emits warnings for broken names' ' - test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && - test_when_finished "test-tool ref-store main delete-refs 1 msg 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" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git for-each-ref >output 2>error && ! grep -e "broken\.\.\.ref" output && ! grep -e "badname" output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref to broken name' - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && + 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" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to broken name' ' - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && + 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" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to bro test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet test_expect_success 'branch -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken name through symref' ' - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && + 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" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && test_must_be_empty output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken na test_expect_success 'update-ref --no-deref -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet test_expect_success 'branch -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref with b test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet test_expect_success 'branch -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && -+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && ++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output && 6: 3c100702fda ! 8: ca7f8aea3d9 t1430: create valid symrefs using test-helper @@ Commit message ## t/t1430-bad-ref-name.sh ## @@ t/t1430-bad-ref-name.sh: test_expect_success 'rev-parse skips symref pointing to broken name' ' - test_when_finished "test-tool ref-store main delete-refs 1 msg 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 && - test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && + 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-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg && - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref to broken name' - test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && + 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 1 msg refs/heads/broken...ref" && + 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && -@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to broken name' ' - test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && +@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref to broken name' - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && + test_expect_success 'branch -d can delete symref to broken name' ' + 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to broken name' ' @@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to bro test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok @@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet test_expect_success 'branch -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && -@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken name through symref' ' - test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && +@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete dangling symref to broken name' ' - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && + test_expect_success 'update-ref -d can delete broken name through symref' ' + 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && - test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && -- gitgitgadget ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 1/8] test-ref-store: remove force-create argument for create-reflog 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 ` 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 ` (7 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Nobody uses force_create=0, so this flag is unnecessary. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/helper/test-ref-store.c | 2 +- t/t1405-main-ref-store.sh | 5 ++--- t/t1406-submodule-ref-store.sh | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 3986665037a..b795a56eedf 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -182,9 +182,9 @@ static int cmd_reflog_exists(struct ref_store *refs, const char **argv) static int cmd_create_reflog(struct ref_store *refs, const char **argv) { const char *refname = notnull(*argv++, "refname"); - int force_create = arg_flags(*argv++, "force-create"); struct strbuf err = STRBUF_INIT; int ret; + int force_create = 1; ret = refs_create_reflog(refs, refname, force_create, &err); if (err.len) diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh index 49718b7ea7f..5e0f7073286 100755 --- a/t/t1405-main-ref-store.sh +++ b/t/t1405-main-ref-store.sh @@ -35,8 +35,7 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' ' git rev-parse FOO -- && git rev-parse refs/tags/new-tag -- && m=$(git rev-parse main) && - REF_NO_DEREF=1 && - $RUN delete-refs $REF_NO_DEREF nothing FOO refs/tags/new-tag && + $RUN delete-refs REF_NO_DEREF nothing FOO refs/tags/new-tag && test_must_fail git rev-parse --symbolic-full-name FOO && test_must_fail git rev-parse FOO -- && test_must_fail git rev-parse refs/tags/new-tag -- @@ -108,7 +107,7 @@ test_expect_success 'delete_reflog(HEAD)' ' ' test_expect_success 'create-reflog(HEAD)' ' - $RUN create-reflog HEAD 1 && + $RUN create-reflog HEAD && git reflog exists HEAD ' diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh index 0a87058971e..f1e57a9c051 100755 --- a/t/t1406-submodule-ref-store.sh +++ b/t/t1406-submodule-ref-store.sh @@ -92,7 +92,7 @@ test_expect_success 'delete_reflog() not allowed' ' ' test_expect_success 'create-reflog() not allowed' ' - test_must_fail $RUN create-reflog HEAD 1 + test_must_fail $RUN create-reflog HEAD ' test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/8] test-ref-store: parse symbolic flag constants 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 ` 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 ` (6 subsequent siblings) 8 siblings, 1 reply; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This lets tests use REF_XXXX constants instead of hardcoded integers. The flag names should be separated by a ','. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/helper/test-ref-store.c | 69 +++++++++++++++++++++++++++++++++++---- t/t1405-main-ref-store.sh | 3 +- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index b795a56eedf..cbf1b5f506d 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -5,6 +5,50 @@ #include "object-store.h" #include "repository.h" +struct flag_definition { + const char *name; + uint64_t mask; +}; + +#define FLAG_DEF(x) \ + { \ +#x, (x) \ + } + +static unsigned int parse_flags(const char *str, struct flag_definition *defs) +{ + struct string_list masks = STRING_LIST_INIT_DUP; + int i = 0; + unsigned int result = 0; + + if (!strcmp(str, "0")) + return 0; + + string_list_split(&masks, str, ',', 64); + for (; i < masks.nr; i++) { + const char *name = masks.items[i].string; + struct flag_definition *def = defs; + int found = 0; + while (def->name) { + if (!strcmp(def->name, name)) { + result |= def->mask; + found = 1; + break; + } + def++; + } + if (!found) + die("unknown flag \"%s\"", name); + } + + string_list_clear(&masks, 0); + return result; +} + +static struct flag_definition empty_flags[] = { + { NULL, 0 }, +}; + static const char *notnull(const char *arg, const char *name) { if (!arg) @@ -12,9 +56,10 @@ static const char *notnull(const char *arg, const char *name) return arg; } -static unsigned int arg_flags(const char *arg, const char *name) +static unsigned int arg_flags(const char *arg, const char *name, + struct flag_definition *defs) { - return atoi(notnull(arg, name)); + return parse_flags(notnull(arg, name), defs); } static const char **get_store(const char **argv, struct ref_store **refs) @@ -64,10 +109,14 @@ static const char **get_store(const char **argv, struct ref_store **refs) return argv + 1; } +static struct flag_definition pack_flags[] = { + FLAG_DEF(PACK_REFS_PRUNE), + FLAG_DEF(PACK_REFS_ALL), +}; static int cmd_pack_refs(struct ref_store *refs, const char **argv) { - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", pack_flags); return refs_pack_refs(refs, flags); } @@ -81,9 +130,15 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv) return refs_create_symref(refs, refname, target, logmsg); } +static struct flag_definition transaction_flags[] = { + FLAG_DEF(REF_NO_DEREF), + FLAG_DEF(REF_FORCE_CREATE_REFLOG), + { NULL, 0 }, +}; + static int cmd_delete_refs(struct ref_store *refs, const char **argv) { - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); const char *msg = *argv++; struct string_list refnames = STRING_LIST_INIT_NODUP; @@ -120,7 +175,7 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv) { struct object_id oid = *null_oid(); const char *refname = notnull(*argv++, "refname"); - int resolve_flags = arg_flags(*argv++, "resolve-flags"); + int resolve_flags = arg_flags(*argv++, "resolve-flags", empty_flags); int flags; const char *ref; int ignore_errno; @@ -209,7 +264,7 @@ static int cmd_delete_ref(struct ref_store *refs, const char **argv) const char *msg = notnull(*argv++, "msg"); const char *refname = notnull(*argv++, "refname"); const char *sha1_buf = notnull(*argv++, "old-sha1"); - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); struct object_id old_oid; if (get_oid_hex(sha1_buf, &old_oid)) @@ -224,7 +279,7 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv) const char *refname = notnull(*argv++, "refname"); const char *new_sha1_buf = notnull(*argv++, "new-sha1"); const char *old_sha1_buf = notnull(*argv++, "old-sha1"); - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); struct object_id old_oid; struct object_id new_oid; diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh index 5e0f7073286..63e0ae82bdf 100755 --- a/t/t1405-main-ref-store.sh +++ b/t/t1405-main-ref-store.sh @@ -17,8 +17,7 @@ test_expect_success 'setup' ' test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' ' N=`find .git/refs -type f | wc -l` && test "$N" != 0 && - ALL_OR_PRUNE_FLAG=3 && - $RUN pack-refs ${ALL_OR_PRUNE_FLAG} && + $RUN pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL && N=`find .git/refs -type f` && test -z "$N" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/8] test-ref-store: parse symbolic flag constants 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 0 siblings, 0 replies; 42+ messages in thread From: Jeff King @ 2021-12-03 6:22 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys On Thu, Dec 02, 2021 at 06:39:56PM +0000, Han-Wen Nienhuys via GitGitGadget wrote: > From: Han-Wen Nienhuys <hanwen@google.com> > > This lets tests use REF_XXXX constants instead of hardcoded integers. The flag > names should be separated by a ','. So much nicer. Thank you for cleaning this up. One small bug: > +static unsigned int parse_flags(const char *str, struct flag_definition *defs) > +{ > + struct string_list masks = STRING_LIST_INIT_DUP; > + int i = 0; > + unsigned int result = 0; > + > + if (!strcmp(str, "0")) > + return 0; > + > + string_list_split(&masks, str, ',', 64); > + for (; i < masks.nr; i++) { > + const char *name = masks.items[i].string; > + struct flag_definition *def = defs; > + int found = 0; > + while (def->name) { > + if (!strcmp(def->name, name)) { > + result |= def->mask; > + found = 1; > + break; > + } > + def++; > + } We assume defs ends with a NULL entry here... > +static struct flag_definition empty_flags[] = { > + { NULL, 0 }, > +}; ...and this one does so... > +static struct flag_definition pack_flags[] = { > + FLAG_DEF(PACK_REFS_PRUNE), > + FLAG_DEF(PACK_REFS_ALL), > +}; ...but this one does not. So passing an unknown flag will result in us walking off the end of the list. > +static struct flag_definition transaction_flags[] = { > + FLAG_DEF(REF_NO_DEREF), > + FLAG_DEF(REF_FORCE_CREATE_REFLOG), > + { NULL, 0 }, > +}; This one is good. We might want to drop the trailing comma on the NULL entries, to make it more clear that they have to come last. The other option would be using ARRAY_SIZE() instead of a NULL terminator, but of course that has to happen in the caller. Which means either extra work there, or yet another macro. :) -Peff ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 3/8] test-ref-store: plug memory leak in cmd_delete_refs 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-02 18:39 ` Han-Wen Nienhuys via GitGitGadget 2021-12-02 18:39 ` [PATCH v3 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget ` (5 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/helper/test-ref-store.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index cbf1b5f506d..c8ae36e2172 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -141,11 +141,14 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv) unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); const char *msg = *argv++; struct string_list refnames = STRING_LIST_INIT_NODUP; + int result; while (*argv) string_list_append(&refnames, *argv++); - return refs_delete_refs(refs, msg, &refnames, flags); + result = refs_delete_refs(refs, msg, &refnames, flags); + string_list_clear(&refnames, 0); + return result; } static int cmd_rename_ref(struct ref_store *refs, const char **argv) -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 4/8] refs: update comment. 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget ` (2 preceding siblings ...) 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 ` 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 ` (4 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the `ref_update` flags"). Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 4b14f30d48f..37329b98cca 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -16,8 +16,7 @@ * This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, - * REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in - * `ref_update::flags`. + * or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /* -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 5/8] refs: introduce REF_SKIP_OID_VERIFICATION flag 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget ` (3 preceding siblings ...) 2021-12-02 18:39 ` [PATCH v3 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 ` Han-Wen Nienhuys via GitGitGadget 2021-12-02 18:40 ` [PATCH v3 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget ` (3 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:39 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This lets the ref-store test helper write non-existent or unparsable objects into the ref storage. Use this to make t1006 and t3800 independent of the files storage backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++---------------- t/helper/test-ref-store.c | 1 + t/t1006-cat-file.sh | 5 ++-- t/t3800-mktag.sh | 6 +++-- 5 files changed, 43 insertions(+), 27 deletions(-) diff --git a/refs.h b/refs.h index 45c34e99e3a..76efc589cca 100644 --- a/refs.h +++ b/refs.h @@ -615,12 +615,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); */ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* + * Blindly write an object_id. This is useful for testing data corruption + * scenarios. + */ +#define REF_SKIP_OID_VERIFICATION (1 << 10) + /* * 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_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION) /* * Add a reference update to transaction. `new_oid` is the value that diff --git a/refs/files-backend.c b/refs/files-backend.c index 37329b98cca..d0019fcd8b7 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1353,7 +1353,8 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname) } static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err); + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err); static int commit_ref_update(struct files_ref_store *refs, struct ref_lock *lock, const struct object_id *oid, const char *logmsg, @@ -1500,7 +1501,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, } oidcpy(&lock->old_oid, &orig_oid); - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, logmsg, &err)) { error("unable to write current sha1 into %s: %s", newrefname, err.buf); strbuf_release(&err); @@ -1520,7 +1521,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, flag = log_all_ref_updates; log_all_ref_updates = LOG_REFS_NONE; - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, NULL, &err)) { error("unable to write current sha1 into %s: %s", oldrefname, err.buf); strbuf_release(&err); @@ -1756,26 +1757,31 @@ static int files_log_ref_write(struct files_ref_store *refs, * errors, rollback the lockfile, fill in *err and return -1. */ static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err) + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err) { static char term = '\n'; struct object *o; int fd; - o = parse_object(the_repository, oid); - if (!o) { - strbuf_addf(err, - "trying to write ref '%s' with nonexistent object %s", - lock->ref_name, oid_to_hex(oid)); - unlock_ref(lock); - return -1; - } - if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { - strbuf_addf(err, - "trying to write non-commit object %s to branch '%s'", - oid_to_hex(oid), lock->ref_name); - unlock_ref(lock); - return -1; + if (!skip_oid_verification) { + o = parse_object(the_repository, oid); + if (!o) { + strbuf_addf( + err, + "trying to write ref '%s' with nonexistent object %s", + lock->ref_name, oid_to_hex(oid)); + unlock_ref(lock); + return -1; + } + if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { + strbuf_addf( + err, + "trying to write non-commit object %s to branch '%s'", + oid_to_hex(oid), lock->ref_name); + unlock_ref(lock); + return -1; + } } fd = get_lock_file_fd(&lock->lk); if (write_in_full(fd, oid_to_hex(oid), the_hash_algo->hexsz) < 0 || @@ -2189,7 +2195,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) } static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) + struct object_id *peeled) { BUG("ref_iterator_peel() called for reflog_iterator"); } @@ -2575,8 +2581,10 @@ static int lock_ref_for_update(struct files_ref_store *refs, * The reference already has the desired * value, so we don't need to write it. */ - } else if (write_ref_to_lockfile(lock, &update->new_oid, - err)) { + } else if (write_ref_to_lockfile( + lock, &update->new_oid, + update->flags & REF_SKIP_OID_VERIFICATION, + err)) { char *write_err = strbuf_detach(err, NULL); /* diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index c8ae36e2172..ac8fa2fe730 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -133,6 +133,7 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv) static struct flag_definition transaction_flags[] = { FLAG_DEF(REF_NO_DEREF), FLAG_DEF(REF_FORCE_CREATE_REFLOG), + FLAG_DEF(REF_SKIP_OID_VERIFICATION), { NULL, 0 }, }; diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 658628375c8..0d4c55f74ec 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID REF_SKIP_OID_VERIFICATION && cat >expect <<-EOF && commit diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh index 0544d58a6ea..e3cf0ffbe59 100755 --- a/t/t3800-mktag.sh +++ b/t/t3800-mktag.sh @@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && # Unlike fsck-ing unreachable content above, this # will always fail. @@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget ` (4 preceding siblings ...) 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 2021-12-02 18:40 ` [PATCH v3 7/8] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget ` (2 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:40 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys 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 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 7/8] t1430: remove refs using test-tool 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget ` (5 preceding siblings ...) 2021-12-02 18:40 ` [PATCH v3 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:40 ` 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 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:40 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t1430-bad-ref-name.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 326e5bd5e80..84e912777c5 100755 --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -154,7 +154,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' git branch shadow one && 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && test_cmp expect actual && @@ -165,9 +165,9 @@ test_expect_success 'for-each-ref emits warnings for broken names' ' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git for-each-ref >output 2>error && ! grep -e "broken\.\.\.ref" output && ! grep -e "badname" output && @@ -204,7 +204,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ -216,7 +216,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ -225,7 +225,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ -234,7 +234,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok test_expect_success 'branch -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ -246,7 +246,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && test_must_be_empty output && @@ -255,7 +255,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_expect_success 'update-ref --no-deref -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ -264,7 +264,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name test_expect_success 'branch -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output && @@ -273,7 +273,7 @@ test_expect_success 'branch -d can delete symref with broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ -282,7 +282,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br test_expect_success 'branch -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 8/8] t1430: create valid symrefs using test-helper 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget ` (6 preceding siblings ...) 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 ` 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 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-02 18:40 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This still leaves some other direct filesystem access. Currently, the files backend does not allow invalidly named symrefs. Fixes for this are currently in the 'seen' branch Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t1430-bad-ref-name.sh | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 84e912777c5..ff1c967d550 100755 --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -153,7 +153,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && git branch shadow one && 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-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && @@ -203,7 +203,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -213,9 +213,8 @@ 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' ' 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -224,7 +223,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -233,7 +232,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok ' test_expect_success 'branch -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -243,9 +242,8 @@ test_expect_success 'branch -d can delete dangling symref to broken name' ' test_expect_success 'update-ref -d can delete broken name through symref' ' 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes 2021-12-02 18:39 ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget ` (7 preceding siblings ...) 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 ` 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 ` (8 more replies) 8 siblings, 9 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys this covers a few cases of direct filesystem access to the ref database in tests. v4: * fix stack OOB read due to missing array terminator. Han-Wen Nienhuys (8): test-ref-store: remove force-create argument for create-reflog test-ref-store: parse symbolic flag constants test-ref-store: plug memory leak in cmd_delete_refs refs: update comment. refs: introduce REF_SKIP_OID_VERIFICATION flag refs: introduce REF_SKIP_REFNAME_VERIFICATION flag t1430: remove refs using test-tool t1430: create valid symrefs using test-helper refs.c | 7 +-- refs.h | 16 +++++- refs/files-backend.c | 53 ++++++++++--------- t/helper/test-ref-store.c | 75 +++++++++++++++++++++++---- t/t1006-cat-file.sh | 5 +- t/t1405-main-ref-store.sh | 8 ++- t/t1406-submodule-ref-store.sh | 2 +- t/t1430-bad-ref-name.sh | 93 ++++++++++++++++++---------------- t/t3800-mktag.sh | 6 ++- 9 files changed, 172 insertions(+), 93 deletions(-) base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v4 Pull-Request: https://github.com/git/git/pull/1147 Range-diff vs v3: 1: 3649ef6d0fa = 1: 3649ef6d0fa test-ref-store: remove force-create argument for create-reflog 2: 3cdebd2dbca ! 2: 865458bc0cb test-ref-store: parse symbolic flag constants @@ t/helper/test-ref-store.c + return result; +} + -+static struct flag_definition empty_flags[] = { -+ { NULL, 0 }, -+}; ++static struct flag_definition empty_flags[] = { { NULL, 0 } }; + static const char *notnull(const char *arg, const char *name) { @@ t/helper/test-ref-store.c: static const char **get_store(const char **argv, stru return argv + 1; } -+static struct flag_definition pack_flags[] = { -+ FLAG_DEF(PACK_REFS_PRUNE), -+ FLAG_DEF(PACK_REFS_ALL), -+}; ++static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE), ++ FLAG_DEF(PACK_REFS_ALL), ++ { NULL, 0 } }; static int cmd_pack_refs(struct ref_store *refs, const char **argv) { @@ t/helper/test-ref-store.c: static int cmd_create_symref(struct ref_store *refs, +static struct flag_definition transaction_flags[] = { + FLAG_DEF(REF_NO_DEREF), + FLAG_DEF(REF_FORCE_CREATE_REFLOG), -+ { NULL, 0 }, ++ { NULL, 0 } +}; + static int cmd_delete_refs(struct ref_store *refs, const char **argv) 3: 536005d65a2 = 3: dcd4e9f5f26 test-ref-store: plug memory leak in cmd_delete_refs 4: 466b4451015 = 4: 67d22211ac4 refs: update comment. 5: 89d692a34b8 ! 5: a8713dd59ee refs: introduce REF_SKIP_OID_VERIFICATION flag @@ t/helper/test-ref-store.c: static int cmd_create_symref(struct ref_store *refs, FLAG_DEF(REF_NO_DEREF), FLAG_DEF(REF_FORCE_CREATE_REFLOG), + FLAG_DEF(REF_SKIP_OID_VERIFICATION), - { NULL, 0 }, + { NULL, 0 } }; 6: af9e94935bf ! 6: 577726dc9df refs: introduce REF_SKIP_REFNAME_VERIFICATION flag @@ t/helper/test-ref-store.c: static struct flag_definition transaction_flags[] = { FLAG_DEF(REF_FORCE_CREATE_REFLOG), FLAG_DEF(REF_SKIP_OID_VERIFICATION), + FLAG_DEF(REF_SKIP_REFNAME_VERIFICATION), - { NULL, 0 }, + { NULL, 0 } }; 7: d4f1d209468 = 7: d85cfa3fee1 t1430: remove refs using test-tool 8: ca7f8aea3d9 = 8: 7597620ec83 t1430: create valid symrefs using test-helper -- gitgitgadget ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 1/8] test-ref-store: remove force-create argument for create-reflog 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 ` 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 ` (7 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Nobody uses force_create=0, so this flag is unnecessary. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/helper/test-ref-store.c | 2 +- t/t1405-main-ref-store.sh | 5 ++--- t/t1406-submodule-ref-store.sh | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 3986665037a..b795a56eedf 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -182,9 +182,9 @@ static int cmd_reflog_exists(struct ref_store *refs, const char **argv) static int cmd_create_reflog(struct ref_store *refs, const char **argv) { const char *refname = notnull(*argv++, "refname"); - int force_create = arg_flags(*argv++, "force-create"); struct strbuf err = STRBUF_INIT; int ret; + int force_create = 1; ret = refs_create_reflog(refs, refname, force_create, &err); if (err.len) diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh index 49718b7ea7f..5e0f7073286 100755 --- a/t/t1405-main-ref-store.sh +++ b/t/t1405-main-ref-store.sh @@ -35,8 +35,7 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' ' git rev-parse FOO -- && git rev-parse refs/tags/new-tag -- && m=$(git rev-parse main) && - REF_NO_DEREF=1 && - $RUN delete-refs $REF_NO_DEREF nothing FOO refs/tags/new-tag && + $RUN delete-refs REF_NO_DEREF nothing FOO refs/tags/new-tag && test_must_fail git rev-parse --symbolic-full-name FOO && test_must_fail git rev-parse FOO -- && test_must_fail git rev-parse refs/tags/new-tag -- @@ -108,7 +107,7 @@ test_expect_success 'delete_reflog(HEAD)' ' ' test_expect_success 'create-reflog(HEAD)' ' - $RUN create-reflog HEAD 1 && + $RUN create-reflog HEAD && git reflog exists HEAD ' diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh index 0a87058971e..f1e57a9c051 100755 --- a/t/t1406-submodule-ref-store.sh +++ b/t/t1406-submodule-ref-store.sh @@ -92,7 +92,7 @@ test_expect_success 'delete_reflog() not allowed' ' ' test_expect_success 'create-reflog() not allowed' ' - test_must_fail $RUN create-reflog HEAD 1 + test_must_fail $RUN create-reflog HEAD ' test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 2/8] test-ref-store: parse symbolic flag constants 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 ` 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 ` (6 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This lets tests use REF_XXXX constants instead of hardcoded integers. The flag names should be separated by a ','. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/helper/test-ref-store.c | 66 ++++++++++++++++++++++++++++++++++----- t/t1405-main-ref-store.sh | 3 +- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index b795a56eedf..abb0ba101ae 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -5,6 +5,48 @@ #include "object-store.h" #include "repository.h" +struct flag_definition { + const char *name; + uint64_t mask; +}; + +#define FLAG_DEF(x) \ + { \ +#x, (x) \ + } + +static unsigned int parse_flags(const char *str, struct flag_definition *defs) +{ + struct string_list masks = STRING_LIST_INIT_DUP; + int i = 0; + unsigned int result = 0; + + if (!strcmp(str, "0")) + return 0; + + string_list_split(&masks, str, ',', 64); + for (; i < masks.nr; i++) { + const char *name = masks.items[i].string; + struct flag_definition *def = defs; + int found = 0; + while (def->name) { + if (!strcmp(def->name, name)) { + result |= def->mask; + found = 1; + break; + } + def++; + } + if (!found) + die("unknown flag \"%s\"", name); + } + + string_list_clear(&masks, 0); + return result; +} + +static struct flag_definition empty_flags[] = { { NULL, 0 } }; + static const char *notnull(const char *arg, const char *name) { if (!arg) @@ -12,9 +54,10 @@ static const char *notnull(const char *arg, const char *name) return arg; } -static unsigned int arg_flags(const char *arg, const char *name) +static unsigned int arg_flags(const char *arg, const char *name, + struct flag_definition *defs) { - return atoi(notnull(arg, name)); + return parse_flags(notnull(arg, name), defs); } static const char **get_store(const char **argv, struct ref_store **refs) @@ -64,10 +107,13 @@ static const char **get_store(const char **argv, struct ref_store **refs) return argv + 1; } +static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE), + FLAG_DEF(PACK_REFS_ALL), + { NULL, 0 } }; static int cmd_pack_refs(struct ref_store *refs, const char **argv) { - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", pack_flags); return refs_pack_refs(refs, flags); } @@ -81,9 +127,15 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv) return refs_create_symref(refs, refname, target, logmsg); } +static struct flag_definition transaction_flags[] = { + FLAG_DEF(REF_NO_DEREF), + FLAG_DEF(REF_FORCE_CREATE_REFLOG), + { NULL, 0 } +}; + static int cmd_delete_refs(struct ref_store *refs, const char **argv) { - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); const char *msg = *argv++; struct string_list refnames = STRING_LIST_INIT_NODUP; @@ -120,7 +172,7 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv) { struct object_id oid = *null_oid(); const char *refname = notnull(*argv++, "refname"); - int resolve_flags = arg_flags(*argv++, "resolve-flags"); + int resolve_flags = arg_flags(*argv++, "resolve-flags", empty_flags); int flags; const char *ref; int ignore_errno; @@ -209,7 +261,7 @@ static int cmd_delete_ref(struct ref_store *refs, const char **argv) const char *msg = notnull(*argv++, "msg"); const char *refname = notnull(*argv++, "refname"); const char *sha1_buf = notnull(*argv++, "old-sha1"); - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); struct object_id old_oid; if (get_oid_hex(sha1_buf, &old_oid)) @@ -224,7 +276,7 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv) const char *refname = notnull(*argv++, "refname"); const char *new_sha1_buf = notnull(*argv++, "new-sha1"); const char *old_sha1_buf = notnull(*argv++, "old-sha1"); - unsigned int flags = arg_flags(*argv++, "flags"); + unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); struct object_id old_oid; struct object_id new_oid; diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh index 5e0f7073286..63e0ae82bdf 100755 --- a/t/t1405-main-ref-store.sh +++ b/t/t1405-main-ref-store.sh @@ -17,8 +17,7 @@ test_expect_success 'setup' ' test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' ' N=`find .git/refs -type f | wc -l` && test "$N" != 0 && - ALL_OR_PRUNE_FLAG=3 && - $RUN pack-refs ${ALL_OR_PRUNE_FLAG} && + $RUN pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL && N=`find .git/refs -type f` && test -z "$N" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 3/8] test-ref-store: plug memory leak in cmd_delete_refs 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 ` Han-Wen Nienhuys via GitGitGadget 2021-12-07 13:38 ` [PATCH v4 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget ` (5 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/helper/test-ref-store.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index abb0ba101ae..8fee3cfce1a 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -138,11 +138,14 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv) unsigned int flags = arg_flags(*argv++, "flags", transaction_flags); const char *msg = *argv++; struct string_list refnames = STRING_LIST_INIT_NODUP; + int result; while (*argv) string_list_append(&refnames, *argv++); - return refs_delete_refs(refs, msg, &refnames, flags); + result = refs_delete_refs(refs, msg, &refnames, flags); + string_list_clear(&refnames, 0); + return result; } static int cmd_rename_ref(struct ref_store *refs, const char **argv) -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 4/8] refs: update comment. 2021-12-07 13:38 ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (2 preceding siblings ...) 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 ` 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 ` (4 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the `ref_update` flags"). Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 4b14f30d48f..37329b98cca 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -16,8 +16,7 @@ * This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, - * REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in - * `ref_update::flags`. + * or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /* -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 5/8] refs: introduce REF_SKIP_OID_VERIFICATION flag 2021-12-07 13:38 ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (3 preceding siblings ...) 2021-12-07 13:38 ` [PATCH v4 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 ` 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 ` (3 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This lets the ref-store test helper write non-existent or unparsable objects into the ref storage. Use this to make t1006 and t3800 independent of the files storage backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++---------------- t/helper/test-ref-store.c | 1 + t/t1006-cat-file.sh | 5 ++-- t/t3800-mktag.sh | 6 +++-- 5 files changed, 43 insertions(+), 27 deletions(-) diff --git a/refs.h b/refs.h index 45c34e99e3a..76efc589cca 100644 --- a/refs.h +++ b/refs.h @@ -615,12 +615,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); */ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* + * Blindly write an object_id. This is useful for testing data corruption + * scenarios. + */ +#define REF_SKIP_OID_VERIFICATION (1 << 10) + /* * 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_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION) /* * Add a reference update to transaction. `new_oid` is the value that diff --git a/refs/files-backend.c b/refs/files-backend.c index 37329b98cca..d0019fcd8b7 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1353,7 +1353,8 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname) } static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err); + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err); static int commit_ref_update(struct files_ref_store *refs, struct ref_lock *lock, const struct object_id *oid, const char *logmsg, @@ -1500,7 +1501,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, } oidcpy(&lock->old_oid, &orig_oid); - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, logmsg, &err)) { error("unable to write current sha1 into %s: %s", newrefname, err.buf); strbuf_release(&err); @@ -1520,7 +1521,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, flag = log_all_ref_updates; log_all_ref_updates = LOG_REFS_NONE; - if (write_ref_to_lockfile(lock, &orig_oid, &err) || + if (write_ref_to_lockfile(lock, &orig_oid, 0, &err) || commit_ref_update(refs, lock, &orig_oid, NULL, &err)) { error("unable to write current sha1 into %s: %s", oldrefname, err.buf); strbuf_release(&err); @@ -1756,26 +1757,31 @@ static int files_log_ref_write(struct files_ref_store *refs, * errors, rollback the lockfile, fill in *err and return -1. */ static int write_ref_to_lockfile(struct ref_lock *lock, - const struct object_id *oid, struct strbuf *err) + const struct object_id *oid, + int skip_oid_verification, struct strbuf *err) { static char term = '\n'; struct object *o; int fd; - o = parse_object(the_repository, oid); - if (!o) { - strbuf_addf(err, - "trying to write ref '%s' with nonexistent object %s", - lock->ref_name, oid_to_hex(oid)); - unlock_ref(lock); - return -1; - } - if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { - strbuf_addf(err, - "trying to write non-commit object %s to branch '%s'", - oid_to_hex(oid), lock->ref_name); - unlock_ref(lock); - return -1; + if (!skip_oid_verification) { + o = parse_object(the_repository, oid); + if (!o) { + strbuf_addf( + err, + "trying to write ref '%s' with nonexistent object %s", + lock->ref_name, oid_to_hex(oid)); + unlock_ref(lock); + return -1; + } + if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) { + strbuf_addf( + err, + "trying to write non-commit object %s to branch '%s'", + oid_to_hex(oid), lock->ref_name); + unlock_ref(lock); + return -1; + } } fd = get_lock_file_fd(&lock->lk); if (write_in_full(fd, oid_to_hex(oid), the_hash_algo->hexsz) < 0 || @@ -2189,7 +2195,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) } static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) + struct object_id *peeled) { BUG("ref_iterator_peel() called for reflog_iterator"); } @@ -2575,8 +2581,10 @@ static int lock_ref_for_update(struct files_ref_store *refs, * The reference already has the desired * value, so we don't need to write it. */ - } else if (write_ref_to_lockfile(lock, &update->new_oid, - err)) { + } else if (write_ref_to_lockfile( + lock, &update->new_oid, + update->flags & REF_SKIP_OID_VERIFICATION, + err)) { char *write_err = strbuf_detach(err, NULL); /* diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 8fee3cfce1a..0f3aa84c9f0 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -130,6 +130,7 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv) static struct flag_definition transaction_flags[] = { FLAG_DEF(REF_NO_DEREF), FLAG_DEF(REF_FORCE_CREATE_REFLOG), + FLAG_DEF(REF_SKIP_OID_VERIFICATION), { NULL, 0 } }; diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 658628375c8..0d4c55f74ec 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID REF_SKIP_OID_VERIFICATION && cat >expect <<-EOF && commit diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh index 0544d58a6ea..e3cf0ffbe59 100755 --- a/t/t3800-mktag.sh +++ b/t/t3800-mktag.sh @@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && # Unlike fsck-ing unreachable content above, this # will always fail. @@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag 2021-12-07 13:38 ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (4 preceding siblings ...) 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 ` 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 ` (2 subsequent siblings) 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys 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 0f3aa84c9f0..8bca4f2af1b 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -131,6 +131,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 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 7/8] t1430: remove refs using test-tool 2021-12-07 13:38 ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (5 preceding siblings ...) 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 ` 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 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t1430-bad-ref-name.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 326e5bd5e80..84e912777c5 100755 --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -154,7 +154,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' git branch shadow one && 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && test_cmp expect actual && @@ -165,9 +165,9 @@ test_expect_success 'for-each-ref emits warnings for broken names' ' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git for-each-ref >output 2>error && ! grep -e "broken\.\.\.ref" output && ! grep -e "badname" output && @@ -204,7 +204,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ -216,7 +216,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ -225,7 +225,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output && @@ -234,7 +234,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok test_expect_success 'branch -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output && @@ -246,7 +246,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' 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" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && test_must_be_empty output && @@ -255,7 +255,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_expect_success 'update-ref --no-deref -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ -264,7 +264,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name test_expect_success 'branch -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output && @@ -273,7 +273,7 @@ test_expect_success 'branch -d can delete symref with broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output && @@ -282,7 +282,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br test_expect_success 'branch -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v4 8/8] t1430: create valid symrefs using test-helper 2021-12-07 13:38 ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (6 preceding siblings ...) 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 ` 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 8 siblings, 0 replies; 42+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-07 13:38 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This still leaves some other direct filesystem access. Currently, the files backend does not allow invalidly named symrefs. Fixes for this are currently in the 'seen' branch Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t1430-bad-ref-name.sh | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh index 84e912777c5..ff1c967d550 100755 --- a/t/t1430-bad-ref-name.sh +++ b/t/t1430-bad-ref-name.sh @@ -153,7 +153,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && git branch shadow one && 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-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && @@ -203,7 +203,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -213,9 +213,8 @@ 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' ' 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -224,7 +223,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -233,7 +232,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok ' test_expect_success 'branch -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && @@ -243,9 +242,8 @@ test_expect_success 'branch -d can delete dangling symref to broken name' ' test_expect_success 'update-ref -d can delete broken name through symref' ' 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-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && -- gitgitgadget ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes 2021-12-07 13:38 ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget ` (7 preceding siblings ...) 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 ` Junio C Hamano 8 siblings, 0 replies; 42+ messages in thread From: Junio C Hamano @ 2021-12-07 21:18 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget Cc: git, Han-Wen Nienhuys, Jeff King, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > this covers a few cases of direct filesystem access to the ref database in > tests. > > v4: > > * fix stack OOB read due to missing array terminator. Looks good to me. I'll tentatively mark it for 'next' and will merge it by the end of this week unless problems are found in the series. Thanks for a pleasant read. ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2021-12-07 21:18 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 ` [PATCH v3 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget 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
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).