Re: [PATCH] config: reject invalid VAR in 'git -c VAR=VAL command'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 21, 2017 at 10:53 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> The parsing of one-shot assignments of configuration variables that
> come from the command line historically was quite loose and allowed
> anything to pass.
>
> The configuration variable names that come from files are validated
> in git_config_parse_source(), which uses get_base_var() that grabs
> the <section> (and subsection) while making sure that <section>
> consists of iskeychar() letters, the function itself that makes sure
> that the first letter in <variable> is isalpha(), and get_value()
> that grabs the remainder of the <variable> name while making sure
> that it consists of iskeychar() letters.
>
> Perform an equivalent check in canonicalize_config_variable_name()
> to catch invalid configuration variable names that come from the
> command line.
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
>
>     Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>     > One thing I noticed is that "git config --get X" will correctly
>     > diagnose that a dot-less X is not a valid variable name, but we do
>     > not seem to diagnose "git -c X=V <cmd>" as invalid.
>     >
>     > Perhaps we should, but it is not the focus of this topic.
>
>     And here is a follow-up on that tangent.

Combining this thought with another email[1] that flew by,
do we also need to have this treatment for "git clone -c"
[1] http://public-inbox.org/git/EC270E42-9431-446C-96F9-E1A0C3E45333@xxxxxxxxx/

>
> +for VAR in a .a a. a.0b a."b c". a."b c".0d
> +do
> +       test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" '
> +               test_must_fail git -c $VAR=VAL config -l
> +       '
> +done
> +
>  test_expect_success 'git -c is not confused by empty environment' '
>         GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list

Do we also want to test obscure cases of expected success?
e.g. I suspect we never use a."b c".d in the test suite elsewhere but it
would be a valid key to be handed to git?



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]