{"thread":{"id":"11438","subject":"git config problem with strange config files","startedAt":"2007-12-31T17:47:41Z","lastAt":"2008-01-01T19:57:42Z","messageCount":5,"participants":["Peter Oberndorfer","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"64269","messageId":"200712311847.41499.kumbayo84@arcor.de","threadId":"11438","inReplyTo":null,"subject":"git config problem with strange config files","fromName":"Peter Oberndorfer","fromEmail":"kumbayo84@arcor.de","sentAt":"2007-12-31T17:47:41Z","receivedAt":"2007-12-31T17:47:41Z","isPatch":false,"sender":{"key":"kumbayo84@arcor.de","avatar":"https://avatars.githubusercontent.com/u/1041267?v=4"},"body":"Hi,\n\nIn case of a strange .git/config file \"git config key value\" can create a \nbogus config entry, where one old value is wrong and the new value resides in \nthe wrong section until the config file is fixed up by hand\n\nThe attached test config has a trailing tab in the last line\nand no newline after that.\nWhen git config is run the new section name is placed directly after the \ntrailing tab, which corrupts the old value.\n\nTo reproduce  the error do the following:\n\nmkdir git_test_config && cd git_test_config\ngit init\n<copy the attached config file to .git/config>\ngit config section2.key bar\ngit config --get section2.key\n-> returns nothing\ngit config --get section.key\n-> foo [section2]\n-> error: More than one value for the key section.key: bar\n\ni tried to make a test out of this but i failed to create a config file \nwithout a trailing newline using here documents :-(\n\nTested using v1.5.4.rc2 and v1.5.4-rc2-6-gab11903\n\nGreetings Peter\n\n\n[section]\n\tkey = foo\t\n\n[section]\n\tkey = foo\t[section2]\n\tkey = bar\n"},{"id":"64287","messageId":"20080101061734.GA15453@coredump.intra.peff.net","threadId":"11438","inReplyTo":"200712311847.41499.kumbayo84@arcor.de","subject":"Re: git config problem with strange config files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-01-01T06:17:34Z","receivedAt":"2008-01-01T06:17:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 31, 2007 at 06:47:41PM +0100, Peter Oberndorfer wrote:\n\n> In case of a strange .git/config file \"git config key value\" can\n> create a bogus config entry, where one old value is wrong and the new\n> value resides in the wrong section until the config file is fixed up\n> by hand\n\nThis patch fixes the test case you gave.\n\nJunio, even though such a config file should be rare, I think this is\nv1.5.4 material. But there is one tricky thing which I will point out in\na followup mail.\n\n-- >8 --\nconfig: handle lack of newline at end of file better\n\nThe config parsing routines use the static global\n'config_file' to store the FILE* pointing to the current\nconfig file being parsed. The function get_next_char()\nautomatically converts an EOF on this file to a newline for\nthe convenience of its callers, and it sets config_file to\nNULL to indicate that EOF was reached.\n\nThis throws away useful information, though, since some\nroutines want to call ftell on 'config_file' to find out\nexactly _where_ the routine ended. In the case of a key\nending at EOF boundary, we ended up segfaulting in some\ncases (changing that key or adding another key in its\nsection), or failing to provide the necessary newline\n(adding a new section).\n\nThis patch adds a new flag to indicate EOF and uses that\ninstead of setting config_file to NULL. It also makes sure\nto add newlines where necessary for truncated input. All\nthree included tests fail without the patch.\n---\n config.c                |   14 +++++++++-----\n t/t1303-wacky-config.sh |   37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 46 insertions(+), 5 deletions(-)\n create mode 100755 t/t1303-wacky-config.sh\n\ndiff --git a/config.c b/config.c\nindex fa56c70..7ef8b75 100644\n--- a/config.c\n+++ b/config.c\n@@ -13,6 +13,7 @@\n static FILE *config_file;\n static const char *config_file_name;\n static int config_linenr;\n+static int config_file_eof;\n static int zlib_compression_seen;\n \n static int get_next_char(void)\n@@ -34,7 +35,7 @@ static int get_next_char(void)\n \t\tif (c == '\\n')\n \t\t\tconfig_linenr++;\n \t\tif (c == EOF) {\n-\t\t\tconfig_file = NULL;\n+\t\t\tconfig_file_eof = 1;\n \t\t\tc = '\\n';\n \t\t}\n \t}\n@@ -118,7 +119,7 @@ static int get_value(config_fn_t fn, char *name, unsigned int len)\n \t/* Get the full name */\n \tfor (;;) {\n \t\tc = get_next_char();\n-\t\tif (c == EOF)\n+\t\tif (config_file_eof)\n \t\t\tbreak;\n \t\tif (!iskeychar(c))\n \t\t\tbreak;\n@@ -182,7 +183,7 @@ static int get_base_var(char *name)\n \n \tfor (;;) {\n \t\tint c = get_next_char();\n-\t\tif (c == EOF)\n+\t\tif (config_file_eof)\n \t\t\treturn -1;\n \t\tif (c == ']')\n \t\t\treturn baselen;\n@@ -205,8 +206,7 @@ static int git_parse_file(config_fn_t fn)\n \tfor (;;) {\n \t\tint c = get_next_char();\n \t\tif (c == '\\n') {\n-\t\t\t/* EOF? */\n-\t\t\tif (!config_file)\n+\t\t\tif (config_file_eof)\n \t\t\t\treturn 0;\n \t\t\tcomment = 0;\n \t\t\tcontinue;\n@@ -469,6 +469,7 @@ int git_config_from_file(config_fn_t fn, const char *filename)\n \t\tconfig_file = f;\n \t\tconfig_file_name = filename;\n \t\tconfig_linenr = 1;\n+\t\tconfig_file_eof = 0;\n \t\tret = git_parse_file(fn);\n \t\tfclose(f);\n \t\tconfig_file_name = NULL;\n@@ -918,6 +919,9 @@ int git_config_set_multivar(const char* key, const char* value,\n \t\t\t\t\tcontents, contents_sz,\n \t\t\t\t\tstore.offset[i]-2, &new_line);\n \n+\t\t\tif (copy_end > 0 && contents[copy_end-1] != '\\n')\n+\t\t\t\tnew_line = 1;\n+\n \t\t\t/* write the first part of the config */\n \t\t\tif (copy_end > copy_begin) {\n \t\t\t\tif (write_in_full(fd, contents + copy_begin,\ndiff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh\nnew file mode 100755\nindex 0000000..99985dc\n--- /dev/null\n+++ b/t/t1303-wacky-config.sh\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+\n+test_description='Test wacky input to git config'\n+. ./test-lib.sh\n+\n+setup() {\n+\t(printf \"[section]\\n\" &&\n+\tprintf \"  key = foo\") >.git/config\n+}\n+\n+check() {\n+\techo \"$2\" >expected\n+\tgit config --get \"$1\" >actual\n+\tgit diff actual expected\n+}\n+\n+test_expect_success 'modify same key' '\n+\tsetup &&\n+\tgit config section.key bar &&\n+\tcheck section.key bar\n+'\n+\n+test_expect_success 'add key in same section' '\n+\tsetup &&\n+\tgit config section.other bar &&\n+\tcheck section.key foo &&\n+\tcheck section.other bar\n+'\n+\n+test_expect_success 'add key in different section' '\n+\tsetup &&\n+\tgit config section2.key bar &&\n+\tcheck section.key foo &&\n+\tcheck section2.key bar\n+'\n+\n+test_done\n-- \n1.5.4.rc2.1103.g1042-dirty\n"},{"id":"64288","messageId":"20080101062314.GA21555@coredump.intra.peff.net","threadId":"11438","inReplyTo":"20080101061734.GA15453@coredump.intra.peff.net","subject":"Re: git config problem with strange config files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-01-01T06:23:14Z","receivedAt":"2008-01-01T06:23:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 01, 2008 at 01:17:34AM -0500, Jeff King wrote:\n\n> @@ -118,7 +119,7 @@ static int get_value(config_fn_t fn, char *name, unsigned int len)\n>  \t/* Get the full name */\n>  \tfor (;;) {\n>  \t\tc = get_next_char();\n> -\t\tif (c == EOF)\n> +\t\tif (config_file_eof)\n>  \t\t\tbreak;\n>  \t\tif (!iskeychar(c))\n>  \t\t\tbreak;\n> @@ -182,7 +183,7 @@ static int get_base_var(char *name)\n>  \n>  \tfor (;;) {\n>  \t\tint c = get_next_char();\n> -\t\tif (c == EOF)\n> +\t\tif (config_file_eof)\n>  \t\t\treturn -1;\n>  \t\tif (c == ']')\n>  \t\t\treturn baselen;\n\nIn these two hunks, we now check the eof flag correctly, which follows\nthrough on the intent of the original code. But if you look carefully at\nget_next_char(), it _never_ returns EOF in the first place; it replaces\nit with '\\n' and sets an external flag. So this 'if' branch was never\nreached, but happened to work because the returned newline triggers a\nsimilar effect.\n\nBy actually looking at the flag, I am implementing the original intent,\nbut it is a potential behavior change (I don't _think_ it should matter,\nand it passes all of the config tests, but I haven't looked carefully at\nall the ramifications). So it might be simpler yet to just eliminate\nthat conditional return.\n\n-Peff\n"},{"id":"64297","messageId":"200801011525.48148.kumbayo84@arcor.de","threadId":"11438","inReplyTo":"20080101061734.GA15453@coredump.intra.peff.net","subject":"Re: git config problem with strange config files","fromName":"Peter Oberndorfer","fromEmail":"kumbayo84@arcor.de","sentAt":"2008-01-01T14:25:48Z","receivedAt":"2008-01-01T14:25:48Z","isPatch":false,"sender":{"key":"kumbayo84@arcor.de","avatar":"https://avatars.githubusercontent.com/u/1041267?v=4"},"body":"On Dienstag 01 Januar 2008, Jeff King wrote:\n> On Mon, Dec 31, 2007 at 06:47:41PM +0100, Peter Oberndorfer wrote:\n> \n> > In case of a strange .git/config file \"git config key value\" can\n> > create a bogus config entry, where one old value is wrong and the new\n> > value resides in the wrong section until the config file is fixed up\n> > by hand\n> \n> This patch fixes the test case you gave.\n> \n> Junio, even though such a config file should be rare, I think this is\n> v1.5.4 material. But there is one tricky thing which I will point out in\n> a followup mail.\n> \nThank you for the patch\n\nI would also guess this is relative rare, but IIRC somebody posted a patch to \nhandle a bogus config with a better message,\nand the breakage example in the mail looked similar to mine.\n\nHow i came up with this bogus config is relative simple.\nI wanted to add a new remote so i copied the already existing remote and \nbranch sections down to the last line and somehow also added a trailing tab.\n\nThen i did a stg init which uses git repo-config to add its\nstackformatversion = 2 setting\n\nGreetings Peter\n"},{"id":"64303","messageId":"7vir2d5nex.fsf@gitster.siamese.dyndns.org","threadId":"11438","inReplyTo":"20080101062314.GA21555@coredump.intra.peff.net","subject":"Re: git config problem with strange config files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-01T19:57:42Z","receivedAt":"2008-01-01T19:57:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I agree with your analysis, and thanks for the patch.\n"}]}