worktree add: sanitize worktree names

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 <hi-angel@yandex.ru> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Mar 8, 2019 at 16:28 UTC 1de16aecf51daf5794aa074f6dd133e088a12690
4 files changed +110 -21
builtin/worktree.c
+9 -1
@@ -275,6 +275,7 @@ static int add_worktree(const char *path, const char *refname,
275 struct strbuf symref = STRBUF_INIT;
276 struct commit *commit = NULL;
277 int is_branch = 0;
278 + struct strbuf sb_name = STRBUF_INIT;
279
280 validate_worktree_add(path, opts);
281
@@ -290,7 +291,13 @@ static int add_worktree(const char *path, const char *refname,
291 die(_("invalid reference: %s"), refname);
292
293 name = worktree_basename(path, &len);
293 - git_path_buf(&sb_repo, "worktrees/%.*s", (int)(path + len - name), name);
294 + strbuf_add(&sb, name, path + len - name);
295 + sanitize_refname_component(sb.buf, &sb_name);
296 + if (!sb_name.len)
297 + BUG("How come '%s' becomes empty after sanitization?", sb.buf);
298 + strbuf_reset(&sb);
299 + name = sb_name.buf;
300 + git_path_buf(&sb_repo, "worktrees/%s", name);
301 len = sb_repo.len;
302 if (safe_create_leading_directories_const(sb_repo.buf))
303 die_errno(_("could not create leading directories of '%s'"),
@@ -415,6 +422,7 @@ done:
422 strbuf_release(&symref);
423 strbuf_release(&sb_repo);
424 strbuf_release(&sb_git);
425 + strbuf_release(&sb_name);
426 return ret;
427 }
428
refs.c
+90 -20
@@ -63,7 +63,7 @@ static unsigned char refname_disposition[256] = {
63 * not legal. It is legal if it is something reasonable to have under
64 * ".git/refs/"; We do not like it if:
65 *
66 - * - any path component of it begins with ".", or
66 + * - it begins with ".", or
67 * - it has double dots "..", or
68 * - it has ASCII control characters, or
69 * - it has ":", "?", "[", "\", "^", "~", SP, or TAB anywhere, or
@@ -71,31 +71,63 @@ static unsigned char refname_disposition[256] = {
71 * - it ends with a "/", or
72 * - it ends with ".lock", or
73 * - it contains a "@{" portion
74 + *
75 + * When sanitized is not NULL, instead of rejecting the input refname
76 + * as an error, try to come up with a usable replacement for the input
77 + * refname in it.
78 */
75 -static int check_refname_component(const char *refname, int *flags)
79 +static int check_refname_component(const char *refname, int *flags,
80 + struct strbuf *sanitized)
81 {
82 const char *cp;
83 char last = '\0';
84 + size_t component_start = 0; /* garbage - not a reasonable initial value */
85 +
86 + if (sanitized)
87 + component_start = sanitized->len;
88
89 for (cp = refname; ; cp++) {
90 int ch = *cp & 255;
91 unsigned char disp = refname_disposition[ch];
92 +
93 + if (sanitized && disp != 1)
94 + strbuf_addch(sanitized, ch);
95 +
96 switch (disp) {
97 case 1:
98 goto out;
99 case 2:
87 - if (last == '.')
88 - return -1; /* Refname contains "..". */
100 + if (last == '.') { /* Refname contains "..". */
101 + if (sanitized)
102 + /* collapse ".." to single "." */
103 + strbuf_setlen(sanitized, sanitized->len - 1);
104 + else
105 + return -1;
106 + }
107 break;
108 case 3:
91 - if (last == '@')
92 - return -1; /* Refname contains "@{". */
109 + if (last == '@') { /* Refname contains "@{". */
110 + if (sanitized)
111 + sanitized->buf[sanitized->len-1] = '-';
112 + else
113 + return -1;
114 + }
115 break;
116 case 4:
95 - return -1;
117 + /* forbidden char */
118 + if (sanitized)
119 + sanitized->buf[sanitized->len-1] = '-';
120 + else
121 + return -1;
122 + break;
123 case 5:
97 - if (!(*flags & REFNAME_REFSPEC_PATTERN))
98 - return -1; /* refspec can't be a pattern */
124 + if (!(*flags & REFNAME_REFSPEC_PATTERN)) {
125 + /* refspec can't be a pattern */
126 + if (sanitized)
127 + sanitized->buf[sanitized->len-1] = '-';
128 + else
129 + return -1;
130 + }
131
132 /*
133 * Unset the pattern flag so that we only accept
@@ -109,26 +141,48 @@ static int check_refname_component(const char *refname, int *flags)
141 out:
142 if (cp == refname)
143 return 0; /* Component has zero length. */
112 - if (refname[0] == '.')
113 - return -1; /* Component starts with '.'. */
144 +
145 + if (refname[0] == '.') { /* Component starts with '.'. */
146 + if (sanitized)
147 + sanitized->buf[component_start] = '-';
148 + else
149 + return -1;
150 + }
151 if (cp - refname >= LOCK_SUFFIX_LEN &&
115 - !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN))
116 - return -1; /* Refname ends with ".lock". */
152 + !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN)) {
153 + if (!sanitized)
154 + return -1;
155 + /* Refname ends with ".lock". */
156 + while (strbuf_strip_suffix(sanitized, LOCK_SUFFIX)) {
157 + /* try again in case we have .lock.lock */
158 + }
159 + }
160 return cp - refname;
161 }
162
120 -int check_refname_format(const char *refname, int flags)
163 +static int check_or_sanitize_refname(const char *refname, int flags,
164 + struct strbuf *sanitized)
165 {
166 int component_len, component_count = 0;
167
124 - if (!strcmp(refname, "@"))
168 + if (!strcmp(refname, "@")) {
169 /* Refname is a single character '@'. */
126 - return -1;
170 + if (sanitized)
171 + strbuf_addch(sanitized, '-');
172 + else
173 + return -1;
174 + }
175
176 while (1) {
177 + if (sanitized && sanitized->len)
178 + strbuf_complete(sanitized, '/');
179 +
180 /* We are at the start of a path component. */
130 - component_len = check_refname_component(refname, &flags);
131 - if (component_len <= 0)
181 + component_len = check_refname_component(refname, &flags,
182 + sanitized);
183 + if (sanitized && component_len == 0)
184 + ; /* OK, omit empty component */
185 + else if (component_len <= 0)
186 return -1;
187
188 component_count++;
@@ -138,13 +192,29 @@ int check_refname_format(const char *refname, int flags)
192 refname += component_len + 1;
193 }
194
141 - if (refname[component_len - 1] == '.')
142 - return -1; /* Refname ends with '.'. */
195 + if (refname[component_len - 1] == '.') {
196 + /* Refname ends with '.'. */
197 + if (sanitized)
198 + ; /* omit ending dot */
199 + else
200 + return -1;
201 + }
202 if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
203 return -1; /* Refname has only one component. */
204 return 0;
205 }
206
207 +int check_refname_format(const char *refname, int flags)
208 +{
209 + return check_or_sanitize_refname(refname, flags, NULL);
210 +}
211 +
212 +void sanitize_refname_component(const char *refname, struct strbuf *out)
213 +{
214 + if (check_or_sanitize_refname(refname, REFNAME_ALLOW_ONELEVEL, out))
215 + BUG("sanitizing refname '%s' check returned error", refname);
216 +}
217 +
218 int refname_is_safe(const char *refname)
219 {
220 const char *rest;
refs.h
+6
@@ -460,6 +460,12 @@ int for_each_reflog(each_ref_fn fn, void *cb_data);
460 */
461 int check_refname_format(const char *refname, int flags);
462
463 +/*
464 + * Apply the rules from check_refname_format, but mutate the result until it
465 + * is acceptable, and place the result in "out".
466 + */
467 +void sanitize_refname_component(const char *refname, struct strbuf *out);
468 +
469 const char *prettify_refname(const char *refname);
470
471 char *shorten_unambiguous_ref(const char *refname, int strict);
t/t2400-worktree-add.sh
+5
@@ -570,4 +570,9 @@ test_expect_success '"add" an existing locked but missing worktree' '
570 git worktree add --force --force --detach gnoo
571 '
572
573 +test_expect_success FUNNYNAMES 'sanitize generated worktree name' '
574 + git worktree add --detach ". weird*..?.lock.lock" &&
575 + test -d .git/worktrees/---weird-.-
576 +'
577 +
578 test_done