config: handle lack of newline at end of file better

The config parsing routines use the static global
'config_file' to store the FILE* pointing to the current
config file being parsed. The function get_next_char()
automatically converts an EOF on this file to a newline for
the convenience of its callers, and it sets config_file to
NULL to indicate that EOF was reached.

This throws away useful information, though, since some
routines want to call ftell on 'config_file' to find out
exactly _where_ the routine ended. In the case of a key
ending at EOF boundary, we ended up segfaulting in some
cases (changing that key or adding another key in its
section), or failing to provide the necessary newline
(adding a new section).

This patch adds a new flag to indicate EOF and uses that
instead of setting config_file to NULL. It also makes sure
to add newlines where necessary for truncated input. All
three included tests fail without the patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/config.c b/config.c
index 80db92990aa9e51fe5eccac6e734b5570c51a12f..857deb6c85091057f663d2eb7143f3717bd82687 100644 (file)
--- a/config.c
+++ b/config.c
@@ -13,6 +13,7 @@
 static FILE *config_file;
 static const char *config_file_name;
 static int config_linenr;
 static FILE *config_file;
 static const char *config_file_name;
 static int config_linenr;

+static int config_file_eof;

 static int zlib_compression_seen;
 
 static int get_next_char(void)
 static int zlib_compression_seen;
 
 static int get_next_char(void)


@@ -34,7 +35,7 @@ static int get_next_char(void)
                if (c == '\n')
                        config_linenr++;
                if (c == EOF) {
                if (c == '\n')
                        config_linenr++;
                if (c == EOF) {

-                       config_file = NULL;
+                       config_file_eof = 1;

                        c = '\n';
                }
        }
                        c = '\n';
                }
        }


@@ -118,7 +119,7 @@ static int get_value(config_fn_t fn, char *name, unsigned int len)
        /* Get the full name */
        for (;;) {
                c = get_next_char();
        /* Get the full name */
        for (;;) {
                c = get_next_char();

-               if (c == EOF)
+               if (config_file_eof)

                        break;
                if (!iskeychar(c))
                        break;
                        break;
                if (!iskeychar(c))
                        break;


@@ -182,7 +183,7 @@ static int get_base_var(char *name)
 
        for (;;) {
                int c = get_next_char();
 
        for (;;) {
                int c = get_next_char();

-               if (c == EOF)
+               if (config_file_eof)

                        return -1;
                if (c == ']')
                        return baselen;
                        return -1;
                if (c == ']')
                        return baselen;


@@ -205,8 +206,7 @@ static int git_parse_file(config_fn_t fn)
        for (;;) {
                int c = get_next_char();
                if (c == '\n') {
        for (;;) {
                int c = get_next_char();
                if (c == '\n') {

-                       /* EOF? */
-                       if (!config_file)
+                       if (config_file_eof)

                                return 0;
                        comment = 0;
                        continue;
                                return 0;
                        comment = 0;
                        continue;


@@ -469,6 +469,7 @@ int git_config_from_file(config_fn_t fn, const char *filename)
                config_file = f;
                config_file_name = filename;
                config_linenr = 1;
                config_file = f;
                config_file_name = filename;
                config_linenr = 1;

+               config_file_eof = 0;

                ret = git_parse_file(fn);
                fclose(f);
                config_file_name = NULL;
                ret = git_parse_file(fn);
                fclose(f);
                config_file_name = NULL;


@@ -918,6 +919,9 @@ int git_config_set_multivar(const char* key, const char* value,
                                        contents, contents_sz,
                                        store.offset[i]-2, &new_line);
 
                                        contents, contents_sz,
                                        store.offset[i]-2, &new_line);
 

+                       if (copy_end > 0 && contents[copy_end-1] != '\n')
+                               new_line = 1;
+

                        /* write the first part of the config */
                        if (copy_end > copy_begin) {
                                if (write_in_full(fd, contents + copy_begin,
                        /* write the first part of the config */
                        if (copy_end > copy_begin) {
                                if (write_in_full(fd, contents + copy_begin,

diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
new file mode 100755 (executable)
index 0000000..99985dc
--- /dev/null
+++ b/t/t1303-wacky-config.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='Test wacky input to git config'
+. ./test-lib.sh
+
+setup() {
+       (printf "[section]\n" &&
+       printf "  key = foo") >.git/config
+}
+
+check() {
+       echo "$2" >expected
+       git config --get "$1" >actual
+       git diff actual expected
+}
+
+test_expect_success 'modify same key' '
+       setup &&
+       git config section.key bar &&
+       check section.key bar
+'
+
+test_expect_success 'add key in same section' '
+       setup &&
+       git config section.other bar &&
+       check section.key foo &&
+       check section.other bar
+'
+
+test_expect_success 'add key in different section' '
+       setup &&
+       git config section2.key bar &&
+       check section.key foo &&
+       check section2.key bar
+'
+
+test_done