Re: [RFC/PATCH] pathspec: allow escaped query values

From: Stefan Beller <sbeller@google.com>
Date: Jun 2, 2016, 19:04 UTC
Message-ID: <CAGZ79ka3ku7x7BhBZJ_uHjS0pNLT7NFACUVqNKi=DvtOkSN_bg@mail.gmail.com>
In-Reply-To: <57507DFD.6010800@ramsayjones.plus.com>

On Thu, Jun 2, 2016 at 11:42 AM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

Show 51 quoted lines

>
>
> On 02/06/16 17:10, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>>
>>> So, at risk of annoying you, let me continue in my ignorance a little
>>> longer and ask: even if you have to protect all of this 'magic' from
>>> the shell with '/" quoting, could you not use (nested) quotes to
>>> protect the <value> part of an <attr>? For example:
>>>
>>>     git ls-files ':(attr:whitespace="indent,trail,space",icase)'
>>
>> That would be workable, I would think.  Before attr:VAR=VAL
>> extention, supported pathspec <magic> were only single lowercase-ascii
>> alphabet tokens, so nobody would have used " as a part of magic.  So
>> quting with double-quote pair would work.
>
> I was thinking about both ' and ", so that you could do:
>
>    $ ./args ':(attr:whitespace="indent,trail,space",icase)'
>     1::(attr:whitespace="indent,trail,space",icase)
>
>    $ ./args ":(attr:whitespace='indent,trail,space',icase)"
>     1::(attr:whitespace='indent,trail,space',icase)
>
>    $ p=':(attr:whitespace="indent,trail,space",icase)'
>    $ ./args "$p"
>     1::(attr:whitespace="indent,trail,space",icase)
>
>    $ p=":(attr:whitespace=\"indent,trail,space\",icase)"
>    $ ./args "$p"
>     1::(attr:whitespace="indent,trail,space",icase)
>
> but limiting it to " would probably be OK too.
>
>> You'd need to come up with a way to quote a double quote that
>> happens to be a part of VAL somehow, though.
>
> Yes I was assuming \ quoting as well - I just want to reduce the
> need for such quoting (especially on windows).
>
>>                                              I think attribute
>> value is limited to a string with non-whitespace letters; even
>> though the built-in attributes that have defined meaning to the Git
>> itself may not use values with letters beyond [-a-zA-Z0-9,], end
>> users and projects can add arbitrary values within the allowed
>> syntax, so it is not unconceivable that some project may have a
>> custom attribute that lists forbidden characters in a path with
>>
>>       === .gitattributes ===
>>         *.txt forbidden=`"

We restrict the 'forbidden' to follow [-a-zA-Z0-9,], so we could enforce it for the values, too.

>
>    $ ./args ":(attr:*.txt forbidden=\'\\\",icase)"
>     1::(attr:*.txt forbidden=\'\",icase)

You should lose the *.txt in there, but put it at the back

>  $ ./args ":(attr:forbidden=\'\\\",icase)*.txt"

>
>    $ ./args ':(attr:*.txt forbidden=\'\''\",icase)'
>     1::(attr:*.txt forbidden=\'\",icase)

I see, so quoting by " or ' is preferred. What if the user
wants to do a
    forbidden=',"

so we have to escape those in there, such as

    ./args ':(attr:"forbidden=\',\"")'

We need to escape both ' and ", one for the outer shell and one for Git parsing.

Does that seem ok?