[v4,2/2] worktree add: sanitize worktree names

Commit Message

Duy Nguyen March 5, 2019, 12:08 p.m. UTC

Worktree names are based on $(basename $GIT_WORK_TREE). They aren't
significant until 3a3b9d8cde (refs: new ref types to make per-worktree
refs visible to all worktrees - 2018-10-21), where worktree name could
be part of a refname and must follow refname rules.

Update 'worktree add' code to remove special characters to follow these
rules. In the future the user will be able to specify the worktree name
by themselves if they're not happy with this dumb character
substitution.

Reported-by: Konstantin Kharlamov <[e-mail]>
Signed-off-by: Nguyễn Thái Ngọc Duy <[e-mail]>
---
 builtin/worktree.c      |  7 ++++++-
 refs.c                  | 36 ++++++++++++++++++++++++++++++++++++
 refs.h                  |  1 +
 t/t2025-worktree-add.sh |  5 +++++
 4 files changed, 48 insertions(+), 1 deletion(-)

Patch

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 3f9907fcc9..ca63dd3df6 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -275,6 +275,7 @@  static int add_worktree(const char *path, const char *refname,
 	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,7 +291,10 @@  static int add_worktree(const char *path, const char *refname,
 		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, name, path + len - name);
+	sanitize_worktree_refname(&sb_name);
+	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'"),
@@ -415,6 +419,7 @@  static int add_worktree(const char *path, const char *refname,
 	strbuf_release(&symref);
 	strbuf_release(&sb_repo);
 	strbuf_release(&sb_git);
+	strbuf_release(&sb_name);
 	return ret;
 }
 
diff --git a/refs.c b/refs.c
index 70c55ea1b6..a23a84e431 100644
--- a/refs.c
+++ b/refs.c
@@ -163,6 +163,42 @@  static int check_refname_component(const char *refname, int *flags)
 	return cp - refname;
 }
 
+void sanitize_worktree_refname(struct strbuf *name)
+{
+	int flags = 0, i, max_tries;
+	const char *cp;
+	enum refname_check_code ret;
+
+	/*
+	 * name->len should be enough because we should never need to
+	 * substitute any position more than once, but let's just add
+	 * a couple more to be on the safe side.
+	 */
+	max_tries = name->len + 10;
+	for (i = 0; i < max_tries; i++) {
+		ret = do_check_refname_component(name->buf, &flags, &cp);
+		switch (ret) {
+		case refname_ok:
+			strbuf_setlen(name, cp - name->buf);
+			return;
+
+		case refname_component_has_zero_length:
+			strbuf_addstr(name, "worktree");
+			return;
+
+		case refname_contains_dotdot:
+		case refname_contains_atopen:
+		case refname_has_badchar:
+		case refname_contains_wildcard:
+		case refname_ends_with_dotlock:
+		case refname_starts_with_dot:
+			*(char *)cp = '-';
+			break;
+		}
+	}
+	BUG("stuck in infinite loop! buf = %s", name->buf);
+}
+
 int check_refname_format(const char *refname, int flags)
 {
 	int component_len, component_count = 0;
diff --git a/refs.h b/refs.h
index 308fa1f03b..3b65b8d27a 100644
--- a/refs.h
+++ b/refs.h
@@ -459,6 +459,7 @@  int for_each_reflog(each_ref_fn fn, void *cb_data);
  * repeated slashes are accepted.
  */
 int check_refname_format(const char *refname, int flags);
+void sanitize_worktree_refname(struct strbuf *name);
 
 const char *prettify_refname(const char *refname);
 
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 286bba35d8..6e2b90c84f 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -570,4 +570,9 @@  test_expect_success '"add" an existing locked but missing worktree' '
 	git worktree add --force --force --detach gnoo
 '
 
+test_expect_success FUNNYNAMES 'sanitize generated worktree name' '
+	git worktree add --detach ".  weird*..?.lock.lock" &&
+	test -d .git/worktrees/---weird--.-.lock-lock
+'
+
 test_done