Re: [PATCH] gitweb: fix support for repository directories with spaces

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 17, 2008, 21:27 UTC
Message-ID: <7vk5gn69cq.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <7vd4mg9824.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 37 quoted lines

> Lea Wiemann <lewiemann@gmail.com> writes:
>
>> git_cmd_str does not quote the directory names without this patch.
>>
>> Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
>> ---
>> git_cmd_str is really really bad from a security POV: Where it is
>> used, command lines are passed to the shell, which (I believe) just
>> *happen* to open no security holes.  Hence the function should
>> ultimately go away.  However, let's make the tests work for the
>> meantime while it's still there.
>>
>>  gitweb/gitweb.perl |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 07e64da..0bddc31 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -1502,7 +1502,7 @@ sub git_cmd {
>>  
>>  # returns path to the core git executable and the --git-dir parameter as string
>>  sub git_cmd_str {
>> -	return join(' ', git_cmd());
>> +	return join ' ', map("'$_'", git_cmd());
>>  }
>
> What happens to a path or parameter that has a sq in it?
>
> You are returing this from git_cmd():
>
> 	return $GIT, '--git-dir='.$git_dir;
>
> How is this cmd_str() gets used?  If you absolutely have to have a single
> string that can be safely passed to the shell, the easiest would be to
> quote mechanically in sq following the pattern illustrated at the
> beginning of quote.c

Just to be a tad more helpful, that would be something like:

	join(' ', map {
        	s/\047/\047\134\047\047/g;
                "'$_'";
	} git_cmd()));