From 0d49b1e5a8c789dd3f1029f382bd76a464b72da0 Mon Sep 17 00:00:00 2001 From: Dragan Simic Date: Thu, 21 Mar 2024 07:06:05 +0100 Subject: [PATCH 1/4] config: minor addition of whitespace In general, binary operators should be enclosed in a pair of leading and trailing space (SP) characters. Thus, clean up one spotted expression that for some reason had a "bunched up" operator. Signed-off-by: Dragan Simic Signed-off-by: Junio C Hamano --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index 3846a37be9..e6741df5bf 100644 --- a/config.c +++ b/config.c @@ -871,7 +871,7 @@ static char *parse_value(struct config_source *cs) continue; } if (c == '"') { - quote = 1-quote; + quote = 1 - quote; continue; } strbuf_addch(&cs->value, c); From f0b894443040cb222fbd80d9661cfa7583a9164f Mon Sep 17 00:00:00 2001 From: Dragan Simic Date: Thu, 21 Mar 2024 07:06:06 +0100 Subject: [PATCH 2/4] config: really keep value-internal whitespace verbatim Fix a bug in function parse_value() that prevented whitespace characters (i.e. spaces and horizontal tabs) found inside configuration option values from being parsed and returned in their original form. The bug caused any number of consecutive whitespace characters to be wrongly "squashed" into the same number of space characters. This bug was introduced back in July 2009, in commit ebdaae372b46 ("config: Keep inner whitespace verbatim"). Further investigation showed that setting a configuration value, by invoking git-config(1), converts value-internal horizontal tabs into "\t" escape sequences, which the buggy value-parsing logic in function parse_value() didn't "squash" into spaces. That's why the test included in the ebdaae37 commit passed, which presumably made the bug remain undetected for this long. On the other hand, value-internal literal horizontal tab characters, found in a configuration file edited by hand, do get "squashed" by the value-parsing logic, so the right choice was to fix this bug by making the value-internal whitespace characters preserved verbatim. Signed-off-by: Dragan Simic Signed-off-by: Junio C Hamano --- config.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/config.c b/config.c index e6741df5bf..b1fc12b3a2 100644 --- a/config.c +++ b/config.c @@ -819,7 +819,8 @@ static int get_next_char(struct config_source *cs) static char *parse_value(struct config_source *cs) { - int quote = 0, comment = 0, space = 0; + int quote = 0, comment = 0; + size_t trim_len = 0; strbuf_reset(&cs->value); for (;;) { @@ -829,13 +830,17 @@ static char *parse_value(struct config_source *cs) cs->linenr--; return NULL; } + if (trim_len) + strbuf_setlen(&cs->value, trim_len); return cs->value.buf; } if (comment) continue; if (isspace(c) && !quote) { + if (!trim_len) + trim_len = cs->value.len; if (cs->value.len) - space++; + strbuf_addch(&cs->value, c); continue; } if (!quote) { @@ -844,8 +849,8 @@ static char *parse_value(struct config_source *cs) continue; } } - for (; space; space--) - strbuf_addch(&cs->value, ' '); + if (trim_len) + trim_len = 0; if (c == '\\') { c = get_next_char(cs); switch (c) { From d71bc1b4a3301b69b83f1363c85c3e07f114c426 Mon Sep 17 00:00:00 2001 From: Dragan Simic Date: Thu, 21 Mar 2024 07:06:07 +0100 Subject: [PATCH 3/4] t1300: add more tests for whitespace and inline comments Add a handful of additional tests, to improve the coverage of the handling of configuration file entries whose values contain internal whitespace, leading and/or trailing whitespace, which may or may not be enclosed within quotation marks, or which contain an additional inline comment. At the same time, rework one already existing whitespace-related test a bit, to ensure its consistency with the newly added tests. This change introduced no functional changes to the already existing test. Helped-by: Eric Sunshine Helped-by: Junio C Hamano Signed-off-by: Dragan Simic Signed-off-by: Junio C Hamano --- t/t1300-config.sh | 114 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 111 insertions(+), 3 deletions(-) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 387d336c91..624ed7545f 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -11,6 +11,98 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh +test_expect_success 'setup whitespace config' ' + sed -e "s/^|//" \ + -e "s/[$]$//" \ + -e "s/X/ /g" >.git/config <<-\EOF + [section] + | solid = rock + | sparse = big XX blue + | sparseAndTail = big XX blue $ + | sparseAndTailQuoted = "big XX blue " + | sparseAndBiggerTail = big XX blue X X + | sparseAndBiggerTailQuoted = "big XX blue X X" + | sparseAndBiggerTailQuotedPlus = "big XX blue X X"X $ + | headAndTail = Xbig blue $ + | headAndTailQuoted = "Xbig blue " + | headAndTailQuotedPlus = "Xbig blue " $ + | annotated = big blueX# to be discarded + | annotatedQuoted = "big blue"X# to be discarded + EOF +' + +test_expect_success 'no internal whitespace' ' + echo "rock" >expect && + git config --get section.solid >actual && + test_cmp expect actual +' + +test_expect_success 'internal whitespace' ' + echo "big QQ blue" | q_to_tab >expect && + git config --get section.sparse >actual && + test_cmp expect actual +' + +test_expect_success 'internal and trailing whitespace' ' + echo "big QQ blue" | q_to_tab >expect && + git config --get section.sparseAndTail >actual && + test_cmp expect actual +' + +test_expect_success 'internal and trailing whitespace, all quoted' ' + echo "big QQ blue " | q_to_tab >expect && + git config --get section.sparseAndTailQuoted >actual && + test_cmp expect actual +' + +test_expect_success 'internal and more trailing whitespace' ' + echo "big QQ blue" | q_to_tab >expect && + git config --get section.sparseAndBiggerTail >actual && + test_cmp expect actual +' + +test_expect_success 'internal and more trailing whitespace, all quoted' ' + echo "big QQ blue Q Q" | q_to_tab >expect && + git config --get section.sparseAndBiggerTailQuoted >actual && + test_cmp expect actual +' + +test_expect_success 'internal and more trailing whitespace, not all quoted' ' + echo "big QQ blue Q Q" | q_to_tab >expect && + git config --get section.sparseAndBiggerTailQuotedPlus >actual && + test_cmp expect actual +' + +test_expect_success 'leading and trailing whitespace' ' + echo "big blue" >expect && + git config --get section.headAndTail >actual && + test_cmp expect actual +' + +test_expect_success 'leading and trailing whitespace, all quoted' ' + echo "Qbig blue " | q_to_tab >expect && + git config --get section.headAndTailQuoted >actual && + test_cmp expect actual +' + +test_expect_success 'leading and trailing whitespace, not all quoted' ' + echo "Qbig blue " | q_to_tab >expect && + git config --get section.headAndTailQuotedPlus >actual && + test_cmp expect actual +' + +test_expect_success 'inline comment' ' + echo "big blue" >expect && + git config --get section.annotated >actual && + test_cmp expect actual +' + +test_expect_success 'inline comment, quoted' ' + echo "big blue" >expect && + git config --get section.annotatedQuoted >actual && + test_cmp expect actual +' + test_expect_success 'clear default config' ' rm -f .git/config ' @@ -1066,9 +1158,25 @@ test_expect_success '--null --get-regexp' ' test_cmp expect result ' -test_expect_success 'inner whitespace kept verbatim' ' - git config section.val "foo bar" && - test_cmp_config "foo bar" section.val +test_expect_success 'inner whitespace kept verbatim, spaces only' ' + echo "foo bar" >expect && + git config section.val "foo bar" && + git config --get section.val >actual && + test_cmp expect actual +' + +test_expect_success 'inner whitespace kept verbatim, horizontal tabs only' ' + echo "fooQQbar" | q_to_tab >expect && + git config section.val "$(cat expect)" && + git config --get section.val >actual && + test_cmp expect actual +' + +test_expect_success 'inner whitespace kept verbatim, horizontal tabs and spaces' ' + echo "foo Q bar" | q_to_tab >expect && + git config section.val "$(cat expect)" && + git config --get section.val >actual && + test_cmp expect actual ' test_expect_success SYMLINKS 'symlinked configuration' ' From e6895c3f971dc4d60f7a9fac6ef41a6593a37ba9 Mon Sep 17 00:00:00 2001 From: Dragan Simic Date: Thu, 21 Mar 2024 07:06:08 +0100 Subject: [PATCH 4/4] config.txt: describe handling of whitespace further Make it more clear what the whitespace characters are in the context of git configuration files, and significantly improve the description of the leading and trailing whitespace handling, especially how it works out together with the presence of inline comments. Helped-by: Junio C Hamano Helped-by: Eric Sunshine Signed-off-by: Dragan Simic Signed-off-by: Junio C Hamano --- Documentation/config.txt | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 229b63a454..d8dd1a71e7 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -22,9 +22,10 @@ multivalued. Syntax ~~~~~~ -The syntax is fairly flexible and permissive; whitespaces are mostly -ignored. The '#' and ';' characters begin comments to the end of line, -blank lines are ignored. +The syntax is fairly flexible and permissive. Whitespace characters, +which in this context are the space character (SP) and the horizontal +tabulation (HT), are mostly ignored. The '#' and ';' characters begin +comments to the end of line. Blank lines are ignored. The file consists of sections and variables. A section begins with the name of the section in square brackets and continues until the next @@ -63,16 +64,17 @@ the variable is the boolean "true"). The variable names are case-insensitive, allow only alphanumeric characters and `-`, and must start with an alphabetic character. -A line that defines a value can be continued to the next line by -ending it with a `\`; the backslash and the end-of-line are -stripped. Leading whitespaces after 'name =', the remainder of the -line after the first comment character '#' or ';', and trailing -whitespaces of the line are discarded unless they are enclosed in -double quotes. Internal whitespaces within the value are retained -verbatim. +Whitespace characters surrounding `name`, `=` and `value` are discarded. +Internal whitespace characters within 'value' are retained verbatim. +Comments starting with either `#` or `;` and extending to the end of line +are discarded. A line that defines a value can be continued to the next +line by ending it with a backslash (`\`); the backslash and the end-of-line +characters are discarded. -Inside double quotes, double quote `"` and backslash `\` characters -must be escaped: use `\"` for `"` and `\\` for `\`. +If `value` needs to contain leading or trailing whitespace characters, +it must be enclosed in double quotation marks (`"`). Inside double quotation +marks, double quote (`"`) and backslash (`\`) characters must be escaped: +use `\"` for `"` and `\\` for `\`. The following escape sequences (beside `\"` and `\\`) are recognized: `\n` for newline character (NL), `\t` for horizontal tabulation (HT, TAB)