]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Merge branch 'hn/allow-bogus-oid-in-ref-tests'
authorJunio C Hamano <gitster@pobox.com>
Wed, 15 Dec 2021 17:39:54 +0000 (09:39 -0800)
committerJunio C Hamano <gitster@pobox.com>
Wed, 15 Dec 2021 17:39:54 +0000 (09:39 -0800)
The test helper for refs subsystem learned to write bogus and/or
nonexistent object name to refs to simulate error situations we
want to test Git in.

* hn/allow-bogus-oid-in-ref-tests:
  t1430: create valid symrefs using test-helper
  t1430: remove refs using test-tool
  refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
  refs: introduce REF_SKIP_OID_VERIFICATION flag
  refs: update comment.
  test-ref-store: plug memory leak in cmd_delete_refs
  test-ref-store: parse symbolic flag constants
  test-ref-store: remove force-create argument for create-reflog

1  2 
refs.c
refs.h
refs/files-backend.c
t/helper/test-ref-store.c
t/t1405-main-ref-store.sh

diff --combined refs.c
index 4338875d86bb952af38067d91df86514dbaebd90,93bfe5b30cf79208b510ee5e245163e59a4089c0..4c317955813a67222be95c16b337cb55f1a07326
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -1083,9 -1083,10 +1083,10 @@@ int ref_transaction_update(struct ref_t
  {
        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;
        if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS)
                BUG("illegal flags 0x%x passed to ref_transaction_update()", flags);
  
 +      /*
 +       * Clear flags outside the allowed set; this should be a noop because
 +       * of the BUG() check above, but it works around a -Wnonnull warning
 +       * with some versions of "gcc -O3".
 +       */
 +      flags &= REF_TRANSACTION_UPDATE_ALLOWED_FLAGS;
 +
        flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0);
  
        ref_transaction_add_update(transaction, refname, flags,
@@@ -2373,15 -2367,16 +2374,15 @@@ int reflog_exists(const char *refname
  }
  
  int refs_create_reflog(struct ref_store *refs, const char *refname,
 -                     int force_create, struct strbuf *err)
 +                     struct strbuf *err)
  {
 -      return refs->be->create_reflog(refs, refname, force_create, err);
 +      return refs->be->create_reflog(refs, refname, err);
  }
  
 -int safe_create_reflog(const char *refname, int force_create,
 -                     struct strbuf *err)
 +int safe_create_reflog(const char *refname, struct strbuf *err)
  {
        return refs_create_reflog(get_main_ref_store(the_repository), refname,
 -                                force_create, err);
 +                                err);
  }
  
  int refs_delete_reflog(struct ref_store *refs, const char *refname)
diff --combined refs.h
index dc6b1ce4ea149082c8b5efe73ac32bea9c9d20d5,65017ceaefc9c629d9a2f1ff9dcb555c06b6c480..92360e55a2045970a2a75ed6ca5505f0a0beb8fa
--- 1/refs.h
--- 2/refs.h
+++ b/refs.h
@@@ -417,8 -417,8 +417,8 @@@ int refs_pack_refs(struct ref_store *re
   * Setup reflog before using. Fill in err and return -1 on failure.
   */
  int refs_create_reflog(struct ref_store *refs, const char *refname,
 -                     int force_create, struct strbuf *err);
 -int safe_create_reflog(const char *refname, int force_create, struct strbuf *err);
 +                     struct strbuf *err);
 +int safe_create_reflog(const char *refname, struct strbuf *err);
  
  /** Reads log for the value of ref during at_time. **/
  int read_ref_at(struct ref_store *refs,
@@@ -463,29 -463,7 +463,29 @@@ int delete_reflog(const char *refname)
  
  /*
   * Callback to process a reflog entry found by the iteration functions (see
 - * below)
 + * below).
 + *
 + * The committer parameter is a single string, in the form
 + * "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" (without double quotes).
 + *
 + * The timestamp parameter gives the time when entry was created as the number
 + * of seconds since the UNIX epoch.
 + *
 + * The tz parameter gives the timezone offset for the user who created
 + * the reflog entry, and its value gives a positive or negative offset
 + * from UTC.  Its absolute value is formed by multiplying the hour
 + * part by 100 and adding the minute part.  For example, 1 hour ahead
 + * of UTC, CET == "+0100", is represented as positive one hundred (not
 + * postiive sixty).
 + *
 + * The msg parameter is a single complete line; a reflog message given
 + * to refs_delete_ref, refs_update_ref, etc. is returned to the
 + * callback normalized---each run of whitespaces are squashed into a
 + * single whitespace, trailing whitespace, if exists, is trimmed, and
 + * then a single LF is added at the end.
 + *
 + * The cb_data is a caller-supplied pointer given to the iterator
 + * functions.
   */
  typedef int each_reflog_ent_fn(
                struct object_id *old_oid, struct object_id *new_oid,
@@@ -637,12 -615,24 +637,24 @@@ struct ref_transaction *ref_transaction
   */
  #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)
+ /*
+  * 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)
+ #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 --combined refs/files-backend.c
index 237a2afb5d7efb510c66bff2d23a21fd5effe487,d0019fcd8b7fd95c3e9075d5b088c1fb6dc5fa29..90b671025a742d743cfeac118348de516c0425f9
@@@ -16,8 -16,7 +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`.
   */
  
  /*
@@@ -1354,7 -1353,8 +1353,8 @@@ static int rename_tmp_log(struct files_
  }
  
  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,
@@@ -1501,7 -1501,7 +1501,7 @@@ static int files_copy_or_rename_ref(str
        }
        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);
  
        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);
@@@ -1671,14 -1671,15 +1671,14 @@@ error
        return -1;
  }
  
 -static int files_create_reflog(struct ref_store *ref_store,
 -                             const char *refname, int force_create,
 +static int files_create_reflog(struct ref_store *ref_store, const char *refname,
                               struct strbuf *err)
  {
        struct files_ref_store *refs =
                files_downcast(ref_store, REF_STORE_WRITE, "create_reflog");
        int fd;
  
 -      if (log_ref_setup(refs, refname, force_create, &fd, err))
 +      if (log_ref_setup(refs, refname, 1, &fd, err))
                return -1;
  
        if (fd >= 0)
@@@ -1756,26 -1757,31 +1756,31 @@@ static int files_log_ref_write(struct f
   * 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 +2194,7 @@@ static int files_reflog_iterator_advanc
  }
  
  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 +2580,10 @@@ static int lock_ref_for_update(struct f
                         * 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);
  
                        /*
index 971b755579be5b387a3adc134bee3cab0280c03b,8bca4f2af1bedfd62fcec57f15d1981dcc6bfa9f..24dd4bec08c4cd6afc2c33bddfd7c86710141428
@@@ -5,6 -5,48 +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)
        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)
        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,16 -127,27 +127,27 @@@ static int cmd_create_symref(struct ref
        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),
+       FLAG_DEF(REF_SKIP_OID_VERIFICATION),
+       FLAG_DEF(REF_SKIP_REFNAME_VERIFICATION),
+       { 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;
+       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)
@@@ -120,7 -177,7 +177,7 @@@ static int cmd_resolve_ref(struct ref_s
  {
        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;
@@@ -152,9 -209,9 +209,9 @@@ static int each_reflog(struct object_i
                       const char *committer, timestamp_t timestamp,
                       int tz, const char *msg, void *cb_data)
  {
 -      printf("%s %s %s %"PRItime" %d %s\n",
 -             oid_to_hex(old_oid), oid_to_hex(new_oid),
 -             committer, timestamp, tz, msg);
 +      printf("%s %s %s %" PRItime " %+05d%s%s", oid_to_hex(old_oid),
 +             oid_to_hex(new_oid), committer, timestamp, tz,
 +             *msg == '\n' ? "" : "\t", msg);
        return 0;
  }
  
@@@ -184,8 -241,9 +241,8 @@@ static int cmd_create_reflog(struct ref
        const char *refname = notnull(*argv++, "refname");
        struct strbuf err = STRBUF_INIT;
        int ret;
 -      int force_create = 1;
  
 -      ret = refs_create_reflog(refs, refname, force_create, &err);
 +      ret = refs_create_reflog(refs, refname, &err);
        if (err.len)
                puts(err.buf);
        return ret;
@@@ -208,7 -266,7 +265,7 @@@ static int cmd_delete_ref(struct ref_st
        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))
@@@ -223,7 -281,7 +280,7 @@@ static int cmd_update_ref(struct ref_st
        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;
  
index 3cb5e23d6db551ad54e3e781bc93ffaeae48918f,63e0ae82bdf133910384219a0b6b5ecfc3f29688..1a3ee8845d67e92190261e33d93c98f540972f7f
@@@ -17,8 -17,7 +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"
  '
@@@ -35,8 -34,7 +34,7 @@@ test_expect_success 'delete_refs(FOO, r
        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 --
@@@ -89,13 -87,13 +87,13 @@@ test_expect_success 'for_each_reflog()
  test_expect_success 'for_each_reflog_ent()' '
        $RUN for-each-reflog-ent HEAD >actual &&
        head -n1 actual | grep one &&
 -      tail -n2 actual | head -n1 | grep recreate-main
 +      tail -n1 actual | grep recreate-main
  '
  
  test_expect_success 'for_each_reflog_ent_reverse()' '
        $RUN for-each-reflog-ent-reverse HEAD >actual &&
        head -n1 actual | grep recreate-main &&
 -      tail -n2 actual | head -n1 | grep one
 +      tail -n1 actual | grep one
  '
  
  test_expect_success 'reflog_exists(HEAD)' '