Skip to content

Commit 1c1c416

Browse files
10ne1gitster
authored andcommitted
submodule: error out if gitdir name is too long
Encoding submodule names increases their name size, so there is an increased risk to hit the max filename length in the gitdir path. (the likelihood is still rather small, so it's an acceptable risk) This gitdir file-name-too-long corner case can be be addressed in multiple ways, including sharding or trimming, however for now, just add the portable logic (suggested by Peff) to detect the corner case then error out to avoid committing to a specific policy (or policies). In the future, instead of throwing an error (which we do now anyway without submodule encoding), we could maybe let the user specify via configs how to address this case, e.g. pick trimming or sharding. At least now we print a nice error instead of the OS defaults which can be rather cryptic for users. Suggested-by: Jeff King <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 35dbc30 commit 1c1c416

File tree

7 files changed

+58
-0
lines changed

7 files changed

+58
-0
lines changed

Makefile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,6 +2210,11 @@ ifndef HAVE_PLATFORM_PROCINFO
22102210
COMPAT_OBJS += compat/stub/procinfo.o
22112211
endif
22122212

2213+
ifdef NO_PATHCONF
2214+
COMPAT_CFLAGS += -DNO_PATHCONF
2215+
COMPAT_OBJS += compat/pathconf.o
2216+
endif
2217+
22132218
ifdef RUNTIME_PREFIX
22142219

22152220
ifdef HAVE_BSD_KERN_PROC_SYSCTL

compat/pathconf.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#include "git-compat-util.h"
2+
3+
/*
4+
* Minimal stub for platforms without pathconf() (e.g. Windows),
5+
* to fall back to NAME_MAX from limits.h or compat/posix.h.
6+
*/
7+
long git_pathconf(const char *path UNUSED, int name UNUSED)
8+
{
9+
return -1;
10+
}

compat/posix.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,14 @@ char *gitdirname(char *);
250250
#define NAME_MAX 255
251251
#endif
252252

253+
#ifdef NO_PATHCONF
254+
#ifndef _PC_NAME_MAX
255+
#define _PC_NAME_MAX 1 /* dummy value, only used for git_pathconf */
256+
#endif
257+
#define pathconf(a,b) git_pathconf(a,b)
258+
long git_pathconf(const char *path, int name);
259+
#endif
260+
253261
typedef uintmax_t timestamp_t;
254262
#define PRItime PRIuMAX
255263
#define parse_timestamp strtoumax

config.mak.uname

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,7 @@ ifeq ($(uname_S),Windows)
473473
NEEDS_CRYPTO_WITH_SSL = YesPlease
474474
NO_LIBGEN_H = YesPlease
475475
NO_POLL = YesPlease
476+
NO_PATHCONF = YesPlease
476477
NO_SYMLINK_HEAD = YesPlease
477478
NO_IPV6 = YesPlease
478479
NO_SETENV = YesPlease
@@ -688,6 +689,7 @@ ifeq ($(uname_S),MINGW)
688689
NEEDS_CRYPTO_WITH_SSL = YesPlease
689690
NO_LIBGEN_H = YesPlease
690691
NO_POLL = YesPlease
692+
NO_PATHCONF = YesPlease
691693
NO_SYMLINK_HEAD = YesPlease
692694
NO_SETENV = YesPlease
693695
NO_STRCASESTR = YesPlease

meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,6 +1392,7 @@ checkfuncs = {
13921392
'initgroups' : [],
13931393
'strtoumax' : ['strtoumax.c', 'strtoimax.c'],
13941394
'pread' : ['pread.c'],
1395+
'pathconf' : ['pathconf.c'],
13951396
}
13961397

13971398
if host_machine.system() == 'windows'

submodule.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2625,13 +2625,29 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
26252625

26262626
if (the_repository->repository_format_submodule_encoding) {
26272627
struct strbuf tmp = STRBUF_INIT;
2628+
size_t base_len;
2629+
long name_max;
26282630

26292631
strbuf_reset(buf);
26302632
repo_git_path_append(r, buf, "modules/");
2633+
base_len = buf->len;
26312634

26322635
strbuf_addstr_urlencode(&tmp, submodule_name, is_rfc3986_unreserved);
26332636
strbuf_addstr_case_encode(buf, tmp.buf);
26342637

2638+
/* Ensure final path length is below NAME_MAX after encoding */
2639+
name_max = pathconf(buf->buf, _PC_NAME_MAX);
2640+
if (name_max == -1)
2641+
name_max = NAME_MAX;
2642+
2643+
if (buf->len - base_len > name_max)
2644+
/*
2645+
* TODO: make this smarter; instead of erroring out, maybe we could trim or
2646+
* shard the gitdir names to make them fit under NAME_MAX.
2647+
*/
2648+
die(_("submodule name %s is too long (%"PRIuMAX" bytes, limit %"PRIuMAX")"),
2649+
buf->buf, (uintmax_t)buf->len - base_len, (uintmax_t)name_max);
2650+
26352651
strbuf_release(&tmp);
26362652
}
26372653
}

t/t7425-submodule-encoding.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,20 @@ test_expect_success 'submodule git dir nesting detection must work with parallel
143143
verify_submodule_gitdir_path clone_parallel hippo/hooks modules/hippo%2fhooks
144144
'
145145

146+
test_expect_success 'submodule encoded name exceeds max name limit' '
147+
(
148+
cd main &&
149+
150+
# find the system NAME_MAX (fall back to 255 if unknown)
151+
name_max=$(getconf NAME_MAX . 2>/dev/null || echo 255) &&
152+
153+
# each "%" char encodes to "%25" (3 chars), ensure we exceed NAME_MAX
154+
count=$((name_max + 10)) &&
155+
longname=$(test_seq -f "%%%0.s" 1 $count | tr -d "\n") &&
156+
157+
test_must_fail git submodule add ../new-sub "$longname" 2>err &&
158+
test_grep "fatal: submodule name .* is too long" err
159+
)
160+
'
161+
146162
test_done

0 commit comments

Comments
 (0)