pathspec: allow escaped query values

In our own .gitattributes file we have attributes such as:

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

When querying for attributes we want to be able to ask for the exact
value, i.e.

    git ls-files :(attr:whitespace=indent,trail,space)

should work, but the commas are used in the attr magic to introduce
the next attr, such that this query currently fails with

fatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'

This change allows escaping characters by a backslash, such that the query

    git ls-files :(attr:whitespace=indent\,trail\,space)

will match all path that have the value "indent,trail,space" for the
whitespace attribute. To accomplish this, we need to modify two places.
First `parse_long_magic` needs to not stop early upon seeing a comma or
closing paren that is escaped. As a second step we need to remove any
escaping from the attr value.

Based on a patch by Stefan Beller <sbeller@google.com>

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/pathspec.c b/pathspec.c
index 608511c..303efda 100644 (file)
--- a/pathspec.c
+++ b/pathspec.c
@@ -89,6 +89,51 @@ static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
        strbuf_addf(sb, ",prefix:%d)", prefixlen);
 }
 
+static size_t strcspn_escaped(const char *s, const char *stop)
+{
+       const char *i;
+
+       for (i = s; *i; i++) {
+               /* skip the escaped character */
+               if (i[0] == '\\' && i[1]) {
+                       i++;
+                       continue;
+               }
+
+               if (strchr(stop, *i))
+                       break;
+       }
+       return i - s;
+}
+
+static inline int invalid_value_char(const char ch)
+{
+       if (isalnum(ch) || strchr(",-_", ch))
+               return 0;
+       return -1;
+}
+
+static char *attr_value_unescape(const char *value)
+{
+       const char *src;
+       char *dst, *ret;
+
+       ret = xmallocz(strlen(value));
+       for (src = value, dst = ret; *src; src++, dst++) {
+               if (*src == '\\') {
+                       if (!src[1])
+                               die(_("Escape character '\\' not allowed as "
+                                     "last character in attr value"));
+                       src++;
+               }
+               if (invalid_value_char(*src))
+                       die("cannot use '%c' for value matching", *src);
+               *dst = *src;
+       }
+       *dst = '\0';
+       return ret;
+}
+
 static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)
 {
        struct string_list_item *si;
@@ -131,10 +176,9 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va
                        if (attr[attr_len] != '=')
                                am->match_mode = MATCH_SET;
                        else {
+                               const char *v = &attr[attr_len + 1];
                                am->match_mode = MATCH_VALUE;
-                               am->value = xstrdup(&attr[attr_len + 1]);
-                               if (strchr(am->value, '\\'))
-                                       die(_("attr spec values must not contain backslashes"));
+                               am->value = attr_value_unescape(v);
                        }
                        break;
                }
@@ -239,7 +283,7 @@ static const char *parse_long_magic(unsigned *magic, int *prefix_len,
        const char *nextat;
 
        for (pos = elem + 2; *pos && *pos != ')'; pos = nextat) {
-               size_t len = strcspn(pos, ",)");
+               size_t len = strcspn_escaped(pos, ",)");
                int i;
 
                if (pos[len] == ',')

diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index 3f06fcf..77b8cef 100755 (executable)
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -178,4 +178,23 @@ test_expect_success 'abort on asking for wrong magic' '
        test_must_fail git ls-files . ":(attr:!label=foo)"
 '
 
+test_expect_success 'check attribute list' '
+       cat <<-EOF >>.gitattributes &&
+       * whitespace=indent,trail,space
+       EOF
+       git ls-files ":(attr:whitespace=indent\,trail\,space)" >actual &&
+       git ls-files >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'backslash cannot be the last character' '
+       test_must_fail git ls-files ":(attr:label=foo\\ labelA=bar)" 2>actual &&
+       test_i18ngrep "not allowed as last character in attr value" actual
+'
+
+test_expect_success 'backslash cannot be used as a value' '
+       test_must_fail git ls-files ":(attr:label=f\\\oo)" 2>actual &&
+       test_i18ngrep "for value matching" actual
+'
+
 test_done