Skip to content

Commit fbfeeaf

Browse files
gitstergitster
committed
Merge branch 'lp/config-vername-check'
* lp/config-vername-check: Disallow empty section and variable names Sanity-check config variable names
2 parents ecd75dd + 2169ddc commit fbfeeaf

4 files changed

Lines changed: 111 additions & 50 deletions

File tree

‎builtin/config.c‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ static int show_config(const char *key_, const char *value_, void *cb)
153153
static int get_value(const char *key_, const char *regex_)
154154
{
155155
int ret = -1;
156-
char *tl;
157156
char *global = NULL, *repo_config = NULL;
158157
const char *system_wide = NULL, *local;
159158

@@ -167,18 +166,32 @@ static int get_value(const char *key_, const char *regex_)
167166
system_wide = git_etc_gitconfig();
168167
}
169168

170-
key = xstrdup(key_);
171-
for (tl=key+strlen(key)-1; tl >= key && *tl != '.'; --tl)
172-
*tl = tolower(*tl);
173-
for (tl=key; *tl && *tl != '.'; ++tl)
174-
*tl = tolower(*tl);
175-
176169
if (use_key_regexp) {
170+
char *tl;
171+
172+
/*
173+
* NEEDSWORK: this naive pattern lowercasing obviously does not
174+
* work for more complex patterns like "^[^.]*Foo.*bar".
175+
* Perhaps we should deprecate this altogether someday.
176+
*/
177+
178+
key = xstrdup(key_);
179+
for (tl = key + strlen(key) - 1;
180+
tl >= key && *tl != '.';
181+
tl--)
182+
*tl = tolower(*tl);
183+
for (tl = key; *tl && *tl != '.'; tl++)
184+
*tl = tolower(*tl);
185+
177186
key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
178187
if (regcomp(key_regexp, key, REG_EXTENDED)) {
179188
fprintf(stderr, "Invalid key pattern: %s\n", key_);
189+
free(key);
180190
goto free_strings;
181191
}
192+
} else {
193+
if (git_config_parse_key(key_, &key, NULL))
194+
goto free_strings;
182195
}
183196

184197
if (regex_) {

‎cache.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,7 @@ extern int git_config_maybe_bool(const char *, const char *);
10141014
extern int git_config_string(const char **, const char *, const char *);
10151015
extern int git_config_pathname(const char **, const char *, const char *);
10161016
extern int git_config_set(const char *, const char *);
1017+
extern int git_config_parse_key(const char *, char **, int *);
10171018
extern int git_config_set_multivar(const char *, const char *, const char *, int);
10181019
extern int git_config_rename_section(const char *, const char *);
10191020
extern const char *git_etc_gitconfig(void);

‎config.c‎

Lines changed: 72 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,75 @@ int git_config_set(const char *key, const char *value)
10981098
return git_config_set_multivar(key, value, NULL, 0);
10991099
}
11001100

1101+
/*
1102+
* Auxiliary function to sanity-check and split the key into the section
1103+
* identifier and variable name.
1104+
*
1105+
* Returns 0 on success, -1 when there is an invalid character in the key and
1106+
* -2 if there is no section name in the key.
1107+
*
1108+
* store_key - pointer to char* which will hold a copy of the key with
1109+
* lowercase section and variable name
1110+
* baselen - pointer to int which will hold the length of the
1111+
* section + subsection part, can be NULL
1112+
*/
1113+
int git_config_parse_key(const char *key, char **store_key, int *baselen_)
1114+
{
1115+
int i, dot, baselen;
1116+
const char *last_dot = strrchr(key, '.');
1117+
1118+
/*
1119+
* Since "key" actually contains the section name and the real
1120+
* key name separated by a dot, we have to know where the dot is.
1121+
*/
1122+
1123+
if (last_dot == NULL || last_dot == key) {
1124+
error("key does not contain a section: %s", key);
1125+
return -2;
1126+
}
1127+
1128+
if (!last_dot[1]) {
1129+
error("key does not contain variable name: %s", key);
1130+
return -2;
1131+
}
1132+
1133+
baselen = last_dot - key;
1134+
if (baselen_)
1135+
*baselen_ = baselen;
1136+
1137+
/*
1138+
* Validate the key and while at it, lower case it for matching.
1139+
*/
1140+
*store_key = xmalloc(strlen(key) + 1);
1141+
1142+
dot = 0;
1143+
for (i = 0; key[i]; i++) {
1144+
unsigned char c = key[i];
1145+
if (c == '.')
1146+
dot = 1;
1147+
/* Leave the extended basename untouched.. */
1148+
if (!dot || i > baselen) {
1149+
if (!iskeychar(c) ||
1150+
(i == baselen + 1 && !isalpha(c))) {
1151+
error("invalid key: %s", key);
1152+
goto out_free_ret_1;
1153+
}
1154+
c = tolower(c);
1155+
} else if (c == '\n') {
1156+
error("invalid key (newline): %s", key);
1157+
goto out_free_ret_1;
1158+
}
1159+
(*store_key)[i] = c;
1160+
}
1161+
(*store_key)[i] = 0;
1162+
1163+
return 0;
1164+
1165+
out_free_ret_1:
1166+
free(*store_key);
1167+
return -1;
1168+
}
1169+
11011170
/*
11021171
* If value==NULL, unset in (remove from) config,
11031172
* if value_regex!=NULL, disregard key/value pairs where value does not match.
@@ -1124,59 +1193,23 @@ int git_config_set(const char *key, const char *value)
11241193
int git_config_set_multivar(const char *key, const char *value,
11251194
const char *value_regex, int multi_replace)
11261195
{
1127-
int i, dot;
11281196
int fd = -1, in_fd;
11291197
int ret;
11301198
char *config_filename;
11311199
struct lock_file *lock = NULL;
1132-
const char *last_dot = strrchr(key, '.');
11331200

11341201
if (config_exclusive_filename)
11351202
config_filename = xstrdup(config_exclusive_filename);
11361203
else
11371204
config_filename = git_pathdup("config");
11381205

1139-
/*
1140-
* Since "key" actually contains the section name and the real
1141-
* key name separated by a dot, we have to know where the dot is.
1142-
*/
1143-
1144-
if (last_dot == NULL) {
1145-
error("key does not contain a section: %s", key);
1146-
ret = 2;
1206+
/* parse-key returns negative; flip the sign to feed exit(3) */
1207+
ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
1208+
if (ret)
11471209
goto out_free;
1148-
}
1149-
store.baselen = last_dot - key;
11501210

11511211
store.multi_replace = multi_replace;
11521212

1153-
/*
1154-
* Validate the key and while at it, lower case it for matching.
1155-
*/
1156-
store.key = xmalloc(strlen(key) + 1);
1157-
dot = 0;
1158-
for (i = 0; key[i]; i++) {
1159-
unsigned char c = key[i];
1160-
if (c == '.')
1161-
dot = 1;
1162-
/* Leave the extended basename untouched.. */
1163-
if (!dot || i > store.baselen) {
1164-
if (!iskeychar(c) || (i == store.baselen+1 && !isalpha(c))) {
1165-
error("invalid key: %s", key);
1166-
free(store.key);
1167-
ret = 1;
1168-
goto out_free;
1169-
}
1170-
c = tolower(c);
1171-
} else if (c == '\n') {
1172-
error("invalid key (newline): %s", key);
1173-
free(store.key);
1174-
ret = 1;
1175-
goto out_free;
1176-
}
1177-
store.key[i] = c;
1178-
}
1179-
store.key[i] = 0;
11801213

11811214
/*
11821215
* The lock serves a purpose in addition to locking: the new

‎t/t1300-repo-config.sh‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -876,11 +876,25 @@ test_expect_success 'check split_cmdline return' "
876876
"
877877

878878
test_expect_success 'git -c "key=value" support' '
879-
test "z$(git -c name=value config name)" = zvalue &&
880879
test "z$(git -c core.name=value config core.name)" = zvalue &&
881-
test "z$(git -c CamelCase=value config camelcase)" = zvalue &&
882-
test "z$(git -c flag config --bool flag)" = ztrue &&
883-
test_must_fail git -c core.name=value config name
880+
test "z$(git -c foo.CamelCase=value config foo.camelcase)" = zvalue &&
881+
test "z$(git -c foo.flag config --bool foo.flag)" = ztrue &&
882+
test_must_fail git -c name=value config core.name
883+
'
884+
885+
test_expect_success 'key sanity-checking' '
886+
test_must_fail git config foo=bar &&
887+
test_must_fail git config foo=.bar &&
888+
test_must_fail git config foo.ba=r &&
889+
test_must_fail git config foo.1bar &&
890+
test_must_fail git config foo."ba
891+
z".bar &&
892+
test_must_fail git config . false &&
893+
test_must_fail git config .foo false &&
894+
test_must_fail git config foo. false &&
895+
test_must_fail git config .foo. false &&
896+
git config foo.bar true &&
897+
git config foo."ba =z".bar false
884898
'
885899

886900
test_done

0 commit comments

Comments
 (0)