Merge pull request #3063 from ethomson/config_validate_name

Validate configuration keys

3 files changed, 112 insertions, 14 deletions

diff --git a/src/config_file.c b/src/config_file.c
index 7c6cb81fe..010c494eb 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -1423,7 +1423,7 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
 	while (!reader->eof) {
 		c = reader_peek(reader, SKIP_WHITESPACE);
 
-		if (c == '\0') { /* We've arrived at the end of the file */
+		if (c == '\n') { /* We've arrived at the end of the file */
 			break;
 
 		} else if (c == '[') { /* section header, new section begins */
@@ -1723,9 +1723,47 @@ static int parse_multiline_variable(struct reader *reader, git_buf *value, int i
 	return 0;
 }
 
+GIT_INLINE(bool) is_namechar(char c)
+{
+	return isalnum(c) || c == '-';
+}
+
+static int parse_name(
+	char **name, const char **value, struct reader *reader, const char *line)
+{
+	const char *name_end = line, *value_start;
+
+	*name = NULL;
+	*value = NULL;
+
+	while (*name_end && is_namechar(*name_end))
+		name_end++;
+
+	if (line == name_end) {
+		set_parse_error(reader, 0, "Invalid configuration key");
+		return -1;
+	}
+
+	value_start = name_end;
+
+	while (*value_start && git__isspace(*value_start))
+		value_start++;
+
+	if (*value_start == '=') {
+		*value = value_start + 1;
+	} else if (*value_start) {
+		set_parse_error(reader, 0, "Invalid configuration key");
+		return -1;
+	}
+
+	if ((*name = git__strndup(line, name_end - line)) == NULL)
+		return -1;
+
+	return 0;
+}
+
 static int parse_variable(struct reader *reader, char **var_name, char **var_value)
 {
-	const char *var_end = NULL;
 	const char *value_start = NULL;
 	char *line;
 	int quote_count;
@@ -1737,18 +1775,8 @@ static int parse_variable(struct reader *reader, char **var_name, char **var_val
 
 	quote_count = strip_comments(line, 0);
 
-	var_end = strchr(line, '=');
-
-	if (var_end == NULL)
-		var_end = strchr(line, '\0');
-	else
-		value_start = var_end + 1;
-
-	do var_end--;
-	while (var_end>line && git__isspace(*var_end));
-
-	*var_name = git__strndup(line, var_end - line + 1);
-	GITERR_CHECK_ALLOC(*var_name);
+	if (parse_name(var_name, &value_start, reader, line) < 0)
+		return -1;
 
 	/* If there is no value, boolean true is assumed */
 	*var_value = NULL;
diff --git a/tests/config/read.c b/tests/config/read.c
index a19bc7dcb..f86b2d79e 100644
--- a/tests/config/read.c
+++ b/tests/config/read.c
@@ -611,6 +611,41 @@ void test_config_read__corrupt_header3(void)
 	git_config_free(cfg);
 }
 
+void test_config_read__invalid_key_chars(void)
+{
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "[foo]\n    has_underscore = git2\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	cl_git_rewritefile("./testconfig", "[foo]\n  has/slash = git2\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	cl_git_rewritefile("./testconfig", "[foo]\n  has+plus = git2\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	cl_git_rewritefile("./testconfig", "[no_key]\n  = git2\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	git_config_free(cfg);
+}
+
+void test_config_read__lone_variable_with_trailing_whitespace(void)
+{
+	git_config *cfg;
+	int b;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "[foo]\n    lonevariable   \n");
+	cl_git_pass(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	cl_git_pass(git_config_get_bool(&b, cfg, "foo.lonevariable"));
+	cl_assert_equal_b(true, b);
+
+	git_config_free(cfg);
+}
+
 void test_config_read__override_variable(void)
 {
 	git_config *cfg;
diff --git a/tests/config/write.c b/tests/config/write.c
index bcc87571c..1832466ae 100644
--- a/tests/config/write.c
+++ b/tests/config/write.c
@@ -1,5 +1,6 @@
 #include "clar_libgit2.h"
 #include "buffer.h"
+#include "fileops.h"
 
 void test_config_write__initialize(void)
 {
@@ -393,3 +394,37 @@ void test_config_write__outside_change(void)
 
 	git_config_free(cfg);
 }
+
+void test_config_write__to_empty_file(void)
+{
+	git_config *cfg;
+	const char *filename = "config-file";
+	git_buf result = GIT_BUF_INIT;
+
+	cl_git_mkfile(filename, "");
+	cl_git_pass(git_config_open_ondisk(&cfg, filename));
+	cl_git_pass(git_config_set_string(cfg, "section.name", "value"));
+	git_config_free(cfg);
+
+	cl_git_pass(git_futils_readbuffer(&result, "config-file"));
+	cl_assert_equal_s("[section]\n\tname = value\n", result.ptr);
+
+	git_buf_free(&result);
+}
+
+void test_config_write__to_file_with_only_comment(void)
+{
+	git_config *cfg;
+	const char *filename = "config-file";
+	git_buf result = GIT_BUF_INIT;
+
+	cl_git_mkfile(filename, "\n\n");
+	cl_git_pass(git_config_open_ondisk(&cfg, filename));
+	cl_git_pass(git_config_set_string(cfg, "section.name", "value"));
+	git_config_free(cfg);
+
+	cl_git_pass(git_futils_readbuffer(&result, "config-file"));
+	cl_assert_equal_s("\n\n[section]\n\tname = value\n", result.ptr);
+
+	git_buf_free(&result);
+}