Re: Segfault in the attr stack

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2016, 22:11 UTC
Message-ID: <xmqqpos0wodp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79ka_4vZfNhgOyMeFKdossO-S5Q7RVnvEzB8YAJNc1YQ+uQ@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 5 quoted lines

> In the Gerrit repo I did
>     $ echo "/plugins/commit-message-length-validator sub-default"
>>>.gitattributes
>     $ git status ":(attr:sub-default)"
> Segmentation fault (core dumped)

Thanks.  I can reproduce this easily with git.git, e.g.

	$ cat >>.git/info/attributes <<-\EOF
	Documentation/git-merge.txt conflict-marker-size=32
        Documentation/user-manual.txt conflict-marker-size=32
	EOF
	$ git status ':(attr:conflict-marker-size=32)'

would die (presumably) the same way.

By the way, I just noticed that the <specification> part of the ':(attr:<specification>)' syntax would need to be rethought. In the .gitattributes file everybody has, we see these lines:

        *.[ch] whitespace=indent,trail,space
        *.sh whitespace=indent,trail,space

but because comma is a special separator in the pathspec magic system, we cannot do

	$ git status ':(attr:whitespace=indent,trail,space)'

I think we should introduce a quoting mechanism to hide these commas from the pathspec magic splitter, e.g.

where attr_value_unquote() would copy string while unquoting some special characters (i.e. at least ' ' and ',' because they are used as syntactic elements in the higher level; there might be others).

diff --git a/pathspec.c b/pathspec.c
index 0a02255..fb22f28 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -132,7 +132,7 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va
 				am->match_mode = MATCH_SET;
 			else {
 				am->match_mode = MATCH_VALUE;
-				am->value = xstrdup(&attr[attr_len + 1]);
+				am->value = attr_value_unquote(&attr[attr_len + 1]);
 				if (strchr(am->value, '\\'))
 					die(_("attr spec values must not contain backslashes"));
 			}