Merge branch 'rs/nth-parent-parse'
authorJunio C Hamano <gitster@pobox.com>
Mon, 7 Oct 2019 02:32:57 +0000 (11:32 +0900)
committerJunio C Hamano <gitster@pobox.com>
Mon, 7 Oct 2019 02:32:57 +0000 (11:32 +0900)
The object name parser for "Nth parent" syntax has been made more
robust against integer overflows.

* rs/nth-parent-parse:
  sha1-name: check for overflow of N in "foo^N" and "foo~N"
  rev-parse: demonstrate overflow of N for "foo^N" and "foo~N"

1  2 
sha1-name.c
t/t1506-rev-parse-diagnosis.sh

diff --combined sha1-name.c
@@@ -403,9 -403,9 +403,9 @@@ static int repo_collect_ambiguous(struc
        return collect_ambiguous(oid, data);
  }
  
 -static struct repository *sort_ambiguous_repo;
 -static int sort_ambiguous(const void *a, const void *b)
 +static int sort_ambiguous(const void *a, const void *b, void *ctx)
  {
 +      struct repository *sort_ambiguous_repo = ctx;
        int a_type = oid_object_info(sort_ambiguous_repo, a, NULL);
        int b_type = oid_object_info(sort_ambiguous_repo, b, NULL);
        int a_type_sort;
  
  static void sort_ambiguous_oid_array(struct repository *r, struct oid_array *a)
  {
 -      /* mutex will be needed if this code is to be made thread safe */
 -      sort_ambiguous_repo = r;
 -      QSORT(a->oid, a->nr, sort_ambiguous);
 -      sort_ambiguous_repo = NULL;
 +      QSORT_S(a->oid, a->nr, sort_ambiguous, r);
  }
  
  static enum get_oid_result get_short_oid(struct repository *r,
         * or migrated from loose to packed.
         */
        if (status == MISSING_OBJECT) {
 -              reprepare_packed_git(the_repository);
 +              reprepare_packed_git(r);
                find_short_object_filename(&ds);
                find_short_packed_object(&ds);
                status = finish_object_disambiguation(&ds, oid);
@@@ -798,7 -801,7 +798,7 @@@ static int get_oid_basic(struct reposit
        "because it will be ignored when you just specify 40-hex. These refs\n"
        "may be created by mistake. For example,\n"
        "\n"
 -      "  git checkout -b $br $(git rev-parse ...)\n"
 +      "  git switch -c $br $(git rev-parse ...)\n"
        "\n"
        "where \"$br\" is somehow empty and a 40-hex ref is created. Please\n"
        "examine these refs and maybe delete them. Turn this message off by\n"
@@@ -1160,13 -1163,22 +1160,22 @@@ static enum get_oid_result get_oid_1(st
        }
  
        if (has_suffix) {
-               int num = 0;
+               unsigned int num = 0;
                int len1 = cp - name;
                cp++;
-               while (cp < name + len)
-                       num = num * 10 + *cp++ - '0';
+               while (cp < name + len) {
+                       unsigned int digit = *cp++ - '0';
+                       if (unsigned_mult_overflows(num, 10))
+                               return MISSING_OBJECT;
+                       num *= 10;
+                       if (unsigned_add_overflows(num, digit))
+                               return MISSING_OBJECT;
+                       num += digit;
+               }
                if (!num && len1 == len - 1)
                        num = 1;
+               else if (num > INT_MAX)
+                       return MISSING_OBJECT;
                if (has_suffix == '^')
                        return get_parent(r, name, len1, oid, num);
                /* else if (has_suffix == '~') -- goes without saying */
@@@ -1386,7 -1398,9 +1395,7 @@@ int repo_get_oid_mb(struct repository *
        two = lookup_commit_reference_gently(r, &oid_tmp, 0);
        if (!two)
                return -1;
 -      if (r != the_repository)
 -              BUG("sorry get_merge_bases() can't take struct repository yet");
 -      mbs = get_merge_bases(one, two);
 +      mbs = repo_get_merge_bases(r, one, two);
        if (!mbs || mbs->next)
                st = -1;
        else {
@@@ -1672,8 -1686,7 +1681,8 @@@ int repo_get_oid_blob(struct repositor
  }
  
  /* Must be called only when object_name:filename doesn't exist. */
 -static void diagnose_invalid_oid_path(const char *prefix,
 +static void diagnose_invalid_oid_path(struct repository *r,
 +                                    const char *prefix,
                                      const char *filename,
                                      const struct object_id *tree_oid,
                                      const char *object_name,
        if (is_missing_file_error(errno)) {
                char *fullname = xstrfmt("%s%s", prefix, filename);
  
 -              if (!get_tree_entry(tree_oid, fullname, &oid, &mode)) {
 +              if (!get_tree_entry(r, tree_oid, fullname, &oid, &mode)) {
                        die("Path '%s' exists, but not '%s'.\n"
                            "Did you mean '%.*s:%s' aka '%.*s:./%s'?",
                            fullname,
@@@ -1885,15 -1898,23 +1894,15 @@@ static enum get_oid_result get_oid_with
                        new_filename = resolve_relative_path(repo, filename);
                        if (new_filename)
                                filename = new_filename;
 -                      /*
 -                       * NEEDSWORK: Eventually get_tree_entry*() should
 -                       * learn to take struct repository directly and we
 -                       * would not need to inject submodule odb to the
 -                       * in-core odb.
 -                       */
 -                      if (repo != the_repository)
 -                              add_to_alternates_memory(repo->objects->odb->path);
                        if (flags & GET_OID_FOLLOW_SYMLINKS) {
 -                              ret = get_tree_entry_follow_symlinks(&tree_oid,
 +                              ret = get_tree_entry_follow_symlinks(repo, &tree_oid,
                                        filename, oid, &oc->symlink_path,
                                        &oc->mode);
                        } else {
 -                              ret = get_tree_entry(&tree_oid, filename, oid,
 +                              ret = get_tree_entry(repo, &tree_oid, filename, oid,
                                                     &oc->mode);
                                if (ret && only_to_die) {
 -                                      diagnose_invalid_oid_path(prefix,
 +                                      diagnose_invalid_oid_path(repo, prefix,
                                                                   filename,
                                                                   &tree_oid,
                                                                   name, len);
@@@ -8,9 -8,10 +8,9 @@@ exec </dev/nul
  
  test_did_you_mean ()
  {
 -      sq="'" &&
        cat >expected <<-EOF &&
 -      fatal: Path '$2$3' $4, but not ${5:-$sq$3$sq}.
 -      Did you mean '$1:$2$3'${2:+ aka $sq$1:./$3$sq}?
 +      fatal: Path '$2$3' $4, but not ${5:-$SQ$3$SQ}.
 +      Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?
        EOF
        test_cmp expected error
  }
@@@ -214,4 -215,12 +214,12 @@@ test_expect_success 'arg before dashdas
        test_cmp expect actual
  '
  
+ test_expect_success 'reject Nth parent if N is too high' '
+       test_must_fail git rev-parse HEAD^100000000000000000000000000000000
+ '
+ test_expect_success 'reject Nth ancestor if N is too high' '
+       test_must_fail git rev-parse HEAD~100000000000000000000000000000000
+ '
  test_done