From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 13 Jun 2019 20:19:40 +0000 (-0700)
Subject: Merge branch 'nd/worktree-name-sanitization'
X-Git-Tag: v2.23.0-rc0~138
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/0d107b19890db191c9281bff67180389fb53df24?ds=inline;hp=-c

Merge branch 'nd/worktree-name-sanitization'

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
---

0d107b19890db191c9281bff67180389fb53df24
diff --combined builtin/worktree.c
index d2a7e2f3f1,b0266f2d44..a5bb02b207
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@@ -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);
  
@@@ -290,17 -291,19 +291,23 @@@
  		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);
  	}
@@@ -310,6 -313,8 +317,6 @@@
  	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),
@@@ -418,6 -422,7 +425,7 @@@
  	strbuf_release(&symref);
  	strbuf_release(&sb_repo);
  	strbuf_release(&sb_git);
+ 	strbuf_release(&sb_name);
  	return ret;
  }
  
diff --combined refs.c
index 92d1f6dbdd,94e8d461c7..b8a8430c96
--- a/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
@@@ -71,31 -71,63 +71,63 @@@
   * - 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
@@@ -109,26 -141,48 +141,48 @@@
  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++;
@@@ -138,13 -192,29 +192,29 @@@
  		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;
@@@ -555,22 -619,15 +625,22 @@@
  	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;
@@@ -585,9 -642,8 +655,9 @@@
  		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);
@@@ -603,11 -659,9 +673,11 @@@
  	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;
@@@ -619,15 -673,13 +689,15 @@@
  
  		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;
@@@ -643,11 -695,6 +713,11 @@@
  	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)
  {
@@@ -984,7 -1030,7 +1054,7 @@@
  	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)
@@@ -995,7 -1041,7 +1065,7 @@@
  	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;
@@@ -1267,7 -1312,7 +1337,7 @@@
  			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;
  		}
  
@@@ -1286,12 -1331,6 +1356,12 @@@
  	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 2727405b61,4d8c5465c3..730d05ad91
--- a/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 **/