{"thread":{"id":"42523","subject":"[PATCHv4] pathspec: allow escaped query values","startedAt":"2016-06-03T19:19:30Z","lastAt":"2016-06-03T20:39:23Z","messageCount":2,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"288288","messageId":"20160603191930.16973-1-sbeller@google.com","threadId":"42523","inReplyTo":null,"subject":"[PATCHv4] pathspec: allow escaped query values","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-03T19:19:30Z","receivedAt":"2016-06-03T19:19:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In our own .gitattributes file we have attributes such as:\n\n    *.[ch] whitespace=indent,trail,space\n\nWhen querying for attributes we want to be able to ask for the exact\nvalue, i.e.\n\n    git ls-files :(attr:whitespace=indent,trail,space)\n\nshould work, but the commas are used in the attr magic to introduce\nthe next attr, such that this query currently fails with\n\nfatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'\n\nThis change allows escaping characters by a backslash, such that the query\n\n    git ls-files :(attr:whitespace=indent\\,trail\\,space)\n\nwill match all path that have the value \"indent,trail,space\" for the\nwhitespace attribute. To accomplish this, we need to modify two places.\nFirst `eat_long_magic` needs to not stop early upon seeing a comma or\nclosing paren that is escaped. As a second step we need to remove any\nescaping from the attr value.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\ndifferences to v3 (http://thread.gmane.org/gmane.comp.version-control.git/296259):\n* lose the *src check before invalid_value_char\n* remove a stray new line in parse_pathspec_attr_match\n\nThanks,\nStefan\n\n pathspec.c                      | 53 +++++++++++++++++++++++++++++++++++++----\n t/t6134-pathspec-with-labels.sh | 10 ++++++++\n 2 files changed, 58 insertions(+), 5 deletions(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 326863a..a32e379 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -89,12 +89,56 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,\n \tstrbuf_addf(sb, \",prefix:%d)\", prefixlen);\n }\n \n+static size_t strcspn_escaped(const char *s, const char *stop)\n+{\n+\tconst char *i;\n+\n+\tfor (i = s; *i; i++) {\n+\t\t/* skip the escaped character */\n+\t\tif (i[0] == '\\\\' && i[1]) {\n+\t\t\ti++;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (strchr(stop, *i))\n+\t\t\tbreak;\n+\t}\n+\treturn i - s;\n+}\n+\n+static inline int invalid_value_char(const char ch)\n+{\n+\tif (isalnum(ch) || strchr(\",-_\", ch))\n+\t\treturn 0;\n+\treturn -1;\n+}\n+\n+static char *attr_value_unescape(const char *value)\n+{\n+\tconst char *src;\n+\tchar *dst, *ret;\n+\n+\tret = xmallocz(strlen(value));\n+\tfor (src = value, dst = ret; *src; src++, dst++) {\n+\t\tif (*src == '\\\\') {\n+\t\t\tif (!src[1])\n+\t\t\t\tdie(_(\"Escape character '\\\\' not allowed as \"\n+\t\t\t\t      \"last character in attr value\"));\n+\t\t\tsrc++;\n+\t\t}\n+\t\tif (invalid_value_char(*src))\n+\t\t\tdie(\"cannot use '%c' for value matching\", *src);\n+\t\t*dst = *src;\n+\t}\n+\t*dst = '\\0';\n+\treturn ret;\n+}\n+\n static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)\n {\n \tstruct string_list_item *si;\n \tstruct string_list list = STRING_LIST_INIT_DUP;\n \n-\n \tif (!value || !strlen(value))\n \t\tdie(_(\"attr spec must not be empty\"));\n \n@@ -131,10 +175,9 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va\n \t\t\tif (attr[attr_len] != '=')\n \t\t\t\tam->match_mode = MATCH_SET;\n \t\t\telse {\n+\t\t\t\tconst char *v = &attr[attr_len + 1];\n \t\t\t\tam->match_mode = MATCH_VALUE;\n-\t\t\t\tam->value = xstrdup(&attr[attr_len + 1]);\n-\t\t\t\tif (strchr(am->value, '\\\\'))\n-\t\t\t\t\tdie(_(\"attr spec values must not contain backslashes\"));\n+\t\t\t\tam->value = attr_value_unescape(v);\n \t\t\t}\n \t\t\tbreak;\n \t\t}\n@@ -166,7 +209,7 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,\n \tfor (copyfrom = elt + 2;\n \t     *copyfrom && *copyfrom != ')';\n \t     copyfrom = nextat) {\n-\t\tsize_t len = strcspn(copyfrom, \",)\");\n+\t\tsize_t len = strcspn_escaped(copyfrom, \",)\");\n \t\tif (copyfrom[len] == ',')\n \t\t\tnextat = copyfrom + len + 1;\n \t\telse\ndiff --git a/t/t6134-pathspec-with-labels.sh b/t/t6134-pathspec-with-labels.sh\nindex a5c9632..f1e355f 100755\n--- a/t/t6134-pathspec-with-labels.sh\n+++ b/t/t6134-pathspec-with-labels.sh\n@@ -163,4 +163,14 @@ test_expect_success 'abort on asking for wrong magic' '\n \ttest_must_fail git ls-files . \":(attr:!label=foo)\"\n '\n \n+test_expect_success 'check attribute list' '\n+\tcat <<-EOF >>.gitattributes &&\n+\t* whitespace=indent,trail,space\n+\tEOF\n+\tcat .gitattributes &&\n+\tgit ls-files \":(attr:whitespace=indent\\,trail\\,space)\" >actual &&\n+\tgit ls-files >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.8.2.126.g9068a9d.dirty\n"},{"id":"288291","messageId":"xmqqzir2nh1g.fsf@gitster.mtv.corp.google.com","threadId":"42523","inReplyTo":"20160603191930.16973-1-sbeller@google.com","subject":"Re: [PATCHv4] pathspec: allow escaped query values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-03T20:39:23Z","receivedAt":"2016-06-03T20:39:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> In our own .gitattributes file we have attributes such as:\n>\n>     *.[ch] whitespace=indent,trail,space\n>\n> When querying for attributes we want to be able to ask for the exact\n> value, i.e.\n>\n>     git ls-files :(attr:whitespace=indent,trail,space)\n>\n> should work, but the commas are used in the attr magic to introduce\n> the next attr, such that this query currently fails with\n>\n> fatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'\n>\n> This change allows escaping characters by a backslash, such that the query\n>\n>     git ls-files :(attr:whitespace=indent\\,trail\\,space)\n>\n> will match all path that have the value \"indent,trail,space\" for the\n> whitespace attribute. To accomplish this, we need to modify two places.\n> First `eat_long_magic` needs to not stop early upon seeing a comma or\n> closing paren that is escaped. As a second step we need to remove any\n> escaping from the attr value.\n>\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n> differences to v3 (http://thread.gmane.org/gmane.comp.version-control.git/296259):\n> * lose the *src check before invalid_value_char\n> * remove a stray new line in parse_pathspec_attr_match\n>\n> Thanks,\n> Stefan\n\nExcellent.  Thanks.\n"}]}