Merge branch 'nd/worktree-name-sanitization'
authorJunio C Hamano <gitster@pobox.com>
Thu, 13 Jun 2019 20:19:40 +0000 (13:19 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 13 Jun 2019 20:19:40 +0000 (13:19 -0700)
In recent versions of Git, per-worktree refs are exposed in
refs/worktrees/<wtname>/ hierarchy, which means that worktree names
must be a valid refname component. The code now sanitizes the names
given to worktrees, to make sure these refs are well-formed.

* nd/worktree-name-sanitization:
worktree add: sanitize worktree names

1  2 
builtin/worktree.c
refs.c
refs.h
diff --combined builtin/worktree.c
index d2a7e2f3f18ba411d065a52ac868db612de5beaa,b0266f2d441c8be7e9d8c528f4c6290d8306ecbc..a5bb02b2076a27a78947fda25c1e16dafd637622
@@@ -268,13 -268,14 +268,14 @@@ static int add_worktree(const char *pat
        struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
        struct strbuf sb = STRBUF_INIT;
        const char *name;
 -      struct stat st;
        struct child_process cp = CHILD_PROCESS_INIT;
        struct argv_array child_env = ARGV_ARRAY_INIT;
 -      int counter = 0, len, ret;
 +      unsigned int counter = 0;
 +      int len, ret;
        struct strbuf symref = STRBUF_INIT;
        struct commit *commit = NULL;
        int is_branch = 0;
+       struct strbuf sb_name = STRBUF_INIT;
  
        validate_worktree_add(path, opts);
  
                die(_("invalid reference: %s"), refname);
  
        name = worktree_basename(path, &len);
-       git_path_buf(&sb_repo, "worktrees/%.*s", (int)(path + len - name), name);
+       strbuf_add(&sb, name, path + len - name);
+       sanitize_refname_component(sb.buf, &sb_name);
+       if (!sb_name.len)
+               BUG("How come '%s' becomes empty after sanitization?", sb.buf);
+       strbuf_reset(&sb);
+       name = sb_name.buf;
+       git_path_buf(&sb_repo, "worktrees/%s", name);
        len = sb_repo.len;
        if (safe_create_leading_directories_const(sb_repo.buf))
                die_errno(_("could not create leading directories of '%s'"),
                          sb_repo.buf);
 -      while (!stat(sb_repo.buf, &st)) {
 +
 +      while (mkdir(sb_repo.buf, 0777)) {
                counter++;
 +              if ((errno != EEXIST) || !counter /* overflow */)
 +                      die_errno(_("could not create directory of '%s'"),
 +                                sb_repo.buf);
                strbuf_setlen(&sb_repo, len);
                strbuf_addf(&sb_repo, "%d", counter);
        }
        atexit(remove_junk);
        sigchain_push_common(remove_junk_on_signal);
  
 -      if (mkdir(sb_repo.buf, 0777))
 -              die_errno(_("could not create directory of '%s'"), sb_repo.buf);
        junk_git_dir = xstrdup(sb_repo.buf);
        is_junk = 1;
  
@@@ -404,7 -409,6 +411,7 @@@ done
                        cp.dir = path;
                        cp.env = env;
                        cp.argv = NULL;
 +                      cp.trace2_hook_name = "post-checkout";
                        argv_array_pushl(&cp.args, absolute_path(hook),
                                         oid_to_hex(&null_oid),
                                         oid_to_hex(&commit->object.oid),
        strbuf_release(&symref);
        strbuf_release(&sb_repo);
        strbuf_release(&sb_git);
+       strbuf_release(&sb_name);
        return ret;
  }
  
diff --combined refs.c
index 92d1f6dbdd0d313cdf109e02f31a51bb55cfc601,94e8d461c741b27469bde2b9a48c393237166ca7..b8a8430c963831d9f761fbd8dad099370286987f
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -63,7 -63,7 +63,7 @@@ static unsigned char refname_dispositio
   * not legal.  It is legal if it is something reasonable to have under
   * ".git/refs/"; We do not like it if:
   *
-  * - any path component of it begins with ".", or
+  * - it begins with ".", or
   * - it has double dots "..", or
   * - it has ASCII control characters, or
   * - it has ":", "?", "[", "\", "^", "~", SP, or TAB anywhere, or
   * - it ends with a "/", or
   * - it ends with ".lock", or
   * - it contains a "@{" portion
+  *
+  * When sanitized is not NULL, instead of rejecting the input refname
+  * as an error, try to come up with a usable replacement for the input
+  * refname in it.
   */
- static int check_refname_component(const char *refname, int *flags)
+ static int check_refname_component(const char *refname, int *flags,
+                                  struct strbuf *sanitized)
  {
        const char *cp;
        char last = '\0';
+       size_t component_start = 0; /* garbage - not a reasonable initial value */
+       if (sanitized)
+               component_start = sanitized->len;
  
        for (cp = refname; ; cp++) {
                int ch = *cp & 255;
                unsigned char disp = refname_disposition[ch];
+               if (sanitized && disp != 1)
+                       strbuf_addch(sanitized, ch);
                switch (disp) {
                case 1:
                        goto out;
                case 2:
-                       if (last == '.')
-                               return -1; /* Refname contains "..". */
+                       if (last == '.') { /* Refname contains "..". */
+                               if (sanitized)
+                                       /* collapse ".." to single "." */
+                                       strbuf_setlen(sanitized, sanitized->len - 1);
+                               else
+                                       return -1;
+                       }
                        break;
                case 3:
-                       if (last == '@')
-                               return -1; /* Refname contains "@{". */
+                       if (last == '@') { /* Refname contains "@{". */
+                               if (sanitized)
+                                       sanitized->buf[sanitized->len-1] = '-';
+                               else
+                                       return -1;
+                       }
                        break;
                case 4:
-                       return -1;
+                       /* forbidden char */
+                       if (sanitized)
+                               sanitized->buf[sanitized->len-1] = '-';
+                       else
+                               return -1;
+                       break;
                case 5:
-                       if (!(*flags & REFNAME_REFSPEC_PATTERN))
-                               return -1; /* refspec can't be a pattern */
+                       if (!(*flags & REFNAME_REFSPEC_PATTERN)) {
+                               /* refspec can't be a pattern */
+                               if (sanitized)
+                                       sanitized->buf[sanitized->len-1] = '-';
+                               else
+                                       return -1;
+                       }
  
                        /*
                         * Unset the pattern flag so that we only accept
  out:
        if (cp == refname)
                return 0; /* Component has zero length. */
-       if (refname[0] == '.')
-               return -1; /* Component starts with '.'. */
+       if (refname[0] == '.') { /* Component starts with '.'. */
+               if (sanitized)
+                       sanitized->buf[component_start] = '-';
+               else
+                       return -1;
+       }
        if (cp - refname >= LOCK_SUFFIX_LEN &&
-           !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN))
-               return -1; /* Refname ends with ".lock". */
+           !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN)) {
+               if (!sanitized)
+                       return -1;
+               /* Refname ends with ".lock". */
+               while (strbuf_strip_suffix(sanitized, LOCK_SUFFIX)) {
+                       /* try again in case we have .lock.lock */
+               }
+       }
        return cp - refname;
  }
  
- int check_refname_format(const char *refname, int flags)
+ static int check_or_sanitize_refname(const char *refname, int flags,
+                                    struct strbuf *sanitized)
  {
        int component_len, component_count = 0;
  
-       if (!strcmp(refname, "@"))
+       if (!strcmp(refname, "@")) {
                /* Refname is a single character '@'. */
-               return -1;
+               if (sanitized)
+                       strbuf_addch(sanitized, '-');
+               else
+                       return -1;
+       }
  
        while (1) {
+               if (sanitized && sanitized->len)
+                       strbuf_complete(sanitized, '/');
                /* We are at the start of a path component. */
-               component_len = check_refname_component(refname, &flags);
-               if (component_len <= 0)
+               component_len = check_refname_component(refname, &flags,
+                                                       sanitized);
+               if (sanitized && component_len == 0)
+                       ; /* OK, omit empty component */
+               else if (component_len <= 0)
                        return -1;
  
                component_count++;
                refname += component_len + 1;
        }
  
-       if (refname[component_len - 1] == '.')
-               return -1; /* Refname ends with '.'. */
+       if (refname[component_len - 1] == '.') {
+               /* Refname ends with '.'. */
+               if (sanitized)
+                       ; /* omit ending dot */
+               else
+                       return -1;
+       }
        if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
                return -1; /* Refname has only one component. */
        return 0;
  }
  
+ int check_refname_format(const char *refname, int flags)
+ {
+       return check_or_sanitize_refname(refname, flags, NULL);
+ }
+ void sanitize_refname_component(const char *refname, struct strbuf *out)
+ {
+       if (check_or_sanitize_refname(refname, REFNAME_ALLOW_ONELEVEL, out))
+               BUG("sanitizing refname '%s' check returned error", refname);
+ }
  int refname_is_safe(const char *refname)
  {
        const char *rest;
@@@ -241,14 -311,9 +311,14 @@@ int read_ref(const char *refname, struc
        return read_ref_full(refname, RESOLVE_REF_READING, oid, NULL);
  }
  
 +static int refs_ref_exists(struct ref_store *refs, const char *refname)
 +{
 +      return !!refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING, NULL, NULL);
 +}
 +
  int ref_exists(const char *refname)
  {
 -      return !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, NULL, NULL);
 +      return refs_ref_exists(get_main_ref_store(the_repository), refname);
  }
  
  static int match_ref_pattern(const char *refname,
@@@ -539,11 -604,10 +609,11 @@@ void expand_ref_prefix(struct argv_arra
   * later free()ing) if the string passed in is a magic short-hand form
   * to name a branch.
   */
 -static char *substitute_branch_name(const char **string, int *len)
 +static char *substitute_branch_name(struct repository *r,
 +                                  const char **string, int *len)
  {
        struct strbuf buf = STRBUF_INIT;
 -      int ret = interpret_branch_name(*string, *len, &buf, 0);
 +      int ret = repo_interpret_branch_name(r, *string, *len, &buf, 0);
  
        if (ret == *len) {
                size_t size;
        return NULL;
  }
  
 -int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
 +int repo_dwim_ref(struct repository *r, const char *str, int len,
 +                struct object_id *oid, char **ref)
  {
 -      char *last_branch = substitute_branch_name(&str, &len);
 -      int   refs_found  = expand_ref(str, len, oid, ref);
 +      char *last_branch = substitute_branch_name(r, &str, &len);
 +      int   refs_found  = expand_ref(r, str, len, oid, ref);
        free(last_branch);
        return refs_found;
  }
  
 -int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
 +int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
 +{
 +      return repo_dwim_ref(the_repository, str, len, oid, ref);
 +}
 +
 +int expand_ref(struct repository *repo, const char *str, int len,
 +             struct object_id *oid, char **ref)
  {
        const char **p, *r;
        int refs_found = 0;
                this_result = refs_found ? &oid_from_ref : oid;
                strbuf_reset(&fullref);
                strbuf_addf(&fullref, *p, len, str);
 -              r = resolve_ref_unsafe(fullref.buf, RESOLVE_REF_READING,
 -                                     this_result, &flag);
 +              r = refs_resolve_ref_unsafe(get_main_ref_store(repo),
 +                                          fullref.buf, RESOLVE_REF_READING,
 +                                          this_result, &flag);
                if (r) {
                        if (!refs_found++)
                                *ref = xstrdup(r);
        return refs_found;
  }
  
 -int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 +int repo_dwim_log(struct repository *r, const char *str, int len,
 +                struct object_id *oid, char **log)
  {
 -      char *last_branch = substitute_branch_name(&str, &len);
 +      struct ref_store *refs = get_main_ref_store(r);
 +      char *last_branch = substitute_branch_name(r, &str, &len);
        const char **p;
        int logs_found = 0;
        struct strbuf path = STRBUF_INIT;
  
                strbuf_reset(&path);
                strbuf_addf(&path, *p, len, str);
 -              ref = resolve_ref_unsafe(path.buf, RESOLVE_REF_READING,
 -                                       &hash, NULL);
 +              ref = refs_resolve_ref_unsafe(refs, path.buf,
 +                                            RESOLVE_REF_READING,
 +                                            &hash, NULL);
                if (!ref)
                        continue;
 -              if (reflog_exists(path.buf))
 +              if (refs_reflog_exists(refs, path.buf))
                        it = path.buf;
 -              else if (strcmp(ref, path.buf) && reflog_exists(ref))
 +              else if (strcmp(ref, path.buf) &&
 +                       refs_reflog_exists(refs, ref))
                        it = ref;
                else
                        continue;
        return logs_found;
  }
  
 +int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 +{
 +      return repo_dwim_log(the_repository, str, len, oid, log);
 +}
 +
  static int is_per_worktree_ref(const char *refname)
  {
        return !strcmp(refname, "HEAD") ||
@@@ -967,8 -1014,7 +1037,8 @@@ static int read_ref_at_ent_oldest(struc
        return 1;
  }
  
 -int read_ref_at(const char *refname, unsigned int flags, timestamp_t at_time, int cnt,
 +int read_ref_at(struct ref_store *refs, const char *refname,
 +              unsigned int flags, timestamp_t at_time, int cnt,
                struct object_id *oid, char **msg,
                timestamp_t *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
  {
        cb.cutoff_cnt = cutoff_cnt;
        cb.oid = oid;
  
 -      for_each_reflog_ent_reverse(refname, read_ref_at_ent, &cb);
 +      refs_for_each_reflog_ent_reverse(refs, refname, read_ref_at_ent, &cb);
  
        if (!cb.reccnt) {
                if (flags & GET_OID_QUIETLY)
        if (cb.found_it)
                return 0;
  
 -      for_each_reflog_ent(refname, read_ref_at_ent_oldest, &cb);
 +      refs_for_each_reflog_ent(refs, refname, read_ref_at_ent_oldest, &cb);
  
        return 1;
  }
@@@ -1188,8 -1234,7 +1258,8 @@@ int update_ref(const char *msg, const c
                               old_oid, flags, onerr);
  }
  
 -char *shorten_unambiguous_ref(const char *refname, int strict)
 +char *refs_shorten_unambiguous_ref(struct ref_store *refs,
 +                                 const char *refname, int strict)
  {
        int i;
        static char **scanf_fmts;
                        strbuf_reset(&resolved_buf);
                        strbuf_addf(&resolved_buf, rule,
                                    short_name_len, short_name);
 -                      if (ref_exists(resolved_buf.buf))
 +                      if (refs_ref_exists(refs, resolved_buf.buf))
                                break;
                }
  
        return xstrdup(refname);
  }
  
 +char *shorten_unambiguous_ref(const char *refname, int strict)
 +{
 +      return refs_shorten_unambiguous_ref(get_main_ref_store(the_repository),
 +                                          refname, strict);
 +}
 +
  static struct string_list *hide_refs;
  
  int parse_hide_refs_config(const char *var, const char *value, const char *section)
diff --combined refs.h
index 2727405b61c4e26538c3e76b528df37078337376,4d8c5465c33ba7a3c1326d2f5ee7ea1b9f6dab44..730d05ad91a6ac8961c58d599942a5721960d7d7
--- 1/refs.h
--- 2/refs.h
+++ b/refs.h
@@@ -111,7 -111,7 +111,7 @@@ int should_autocreate_reflog(const cha
  
  int is_branch(const char *refname);
  
 -extern int refs_init_db(struct strbuf *err);
 +int refs_init_db(struct strbuf *err);
  
  /*
   * If refname is a non-symbolic reference that refers to a tag object,
@@@ -148,9 -148,7 +148,9 @@@ int refname_match(const char *abbrev_na
  struct argv_array;
  void expand_ref_prefix(struct argv_array *prefixes, const char *prefix);
  
 -int expand_ref(const char *str, int len, struct object_id *oid, char **ref);
 +int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
 +int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
 +int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
  int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
  int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
  
@@@ -388,8 -386,7 +388,8 @@@ int refs_create_reflog(struct ref_stor
  int safe_create_reflog(const char *refname, int force_create, struct strbuf *err);
  
  /** Reads log for the value of ref during at_time. **/
 -int read_ref_at(const char *refname, unsigned int flags,
 +int read_ref_at(struct ref_store *refs,
 +              const char *refname, unsigned int flags,
                timestamp_t at_time, int cnt,
                struct object_id *oid, char **msg,
                timestamp_t *cutoff_time, int *cutoff_tz, int *cutoff_cnt);
@@@ -463,10 -460,14 +463,16 @@@ int for_each_reflog(each_ref_fn fn, voi
   */
  int check_refname_format(const char *refname, int flags);
  
+ /*
+  * Apply the rules from check_refname_format, but mutate the result until it
+  * is acceptable, and place the result in "out".
+  */
+ void sanitize_refname_component(const char *refname, struct strbuf *out);
  const char *prettify_refname(const char *refname);
  
 +char *refs_shorten_unambiguous_ref(struct ref_store *refs,
 +                                 const char *refname, int strict);
  char *shorten_unambiguous_ref(const char *refname, int strict);
  
  /** rename ref, return 0 on success **/