Skip to content

Commit f6adec4

Browse files
peffgitster
authored andcommitted
submodule-config: ban submodule urls that start with dash
The previous commit taught the submodule code to invoke our "git clone $url $path" with a "--" separator so that we aren't confused by urls or paths that start with dashes. However, that's just one code path. It's not clear if there are others, and it would be an easy mistake to add one in the future. Moreover, even with the fix in the previous commit, it's quite hard to actually do anything useful with such an entry. Any url starting with a dash must fall into one of three categories: - it's meant as a file url, like "-path". But then any clone is not going to have the matching path, since it's by definition relative inside the newly created clone. If you spell it as "./-path", the submodule code sees the "/" and translates this to an absolute path, so it at least works (assuming the receiver has the same filesystem layout as you). But that trick does not apply for a bare "-path". - it's meant as an ssh url, like "-host:path". But this already doesn't work, as we explicitly disallow ssh hostnames that begin with a dash (to avoid option injection against ssh). - it's a remote-helper scheme, like "-scheme::data". This _could_ work if the receiver bends over backwards and creates a funny-named helper like "git-remote--scheme". But normally there would not be any helper that matches. Since such a url does not work today and is not likely to do anything useful in the future, let's simply disallow them entirely. That protects the existing "git clone" path (in a belt-and-suspenders way), along with any others that might exist. Our tests cover two cases: 1. A file url with "./" continues to work, showing that there's an escape hatch for people with truly silly repo names. 2. A url starting with "-" is rejected. Note that we expect case (2) to fail, but it would have done so even without this commit, for the reasons given above. So instead of just expecting failure, let's also check for the magic word "ignoring" on stderr. That lets us know that we failed for the right reason. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 98afac7 commit f6adec4

File tree

2 files changed

+42
-0
lines changed

2 files changed

+42
-0
lines changed

submodule-config.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,12 @@ static void warn_multiple_config(const unsigned char *treeish_name,
367367
commit_string, name, option);
368368
}
369369

370+
static void warn_command_line_option(const char *var, const char *value)
371+
{
372+
warning(_("ignoring '%s' which may be interpreted as"
373+
" a command-line option: %s"), var, value);
374+
}
375+
370376
struct parse_config_parameter {
371377
struct submodule_cache *cache;
372378
const unsigned char *treeish_name;
@@ -432,6 +438,8 @@ static int parse_config(const char *var, const char *value, void *data)
432438
} else if (!strcmp(item.buf, "url")) {
433439
if (!value) {
434440
ret = config_error_nonbool(var);
441+
} else if (looks_like_command_line_option(value)) {
442+
warn_command_line_option(var, value);
435443
} else if (!me->overwrite && submodule->url) {
436444
warn_multiple_config(me->treeish_name, submodule->name,
437445
"url");

t/t7416-submodule-dash-url.sh

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
#!/bin/sh
2+
3+
test_description='check handling of .gitmodule url with dash'
4+
. ./test-lib.sh
5+
6+
test_expect_success 'create submodule with protected dash in url' '
7+
git init upstream &&
8+
git -C upstream commit --allow-empty -m base &&
9+
mv upstream ./-upstream &&
10+
git submodule add ./-upstream sub &&
11+
git add sub .gitmodules &&
12+
git commit -m submodule
13+
'
14+
15+
test_expect_success 'clone can recurse submodule' '
16+
test_when_finished "rm -rf dst" &&
17+
git clone --recurse-submodules . dst &&
18+
echo base >expect &&
19+
git -C dst/sub log -1 --format=%s >actual &&
20+
test_cmp expect actual
21+
'
22+
23+
test_expect_success 'remove ./ protection from .gitmodules url' '
24+
perl -i -pe "s{\./}{}" .gitmodules &&
25+
git commit -am "drop protection"
26+
'
27+
28+
test_expect_success 'clone rejects unprotected dash' '
29+
test_when_finished "rm -rf dst" &&
30+
test_must_fail git clone --recurse-submodules . dst 2>err &&
31+
test_i18ngrep ignoring err
32+
'
33+
34+
test_done

0 commit comments

Comments
 (0)