Re: [PATCH] check for search permission

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 17, 2017, 20:18 UTC
Message-ID: <xmqqd18yhl5v.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <0102015d5203f2ae-c7f1daeb-a6ab-413d-9036-c8da816c4c18-000000@eu-west-1.amazonses.com>

Hielke Christian Braun <hcb@unco.de> writes:

> ---
>  gitweb/gitweb.perl | 2 ++
>  1 file changed, 2 insertions(+)

Thanks for trying to help and welcome to Git development community. But

 (1) Please double-check the title of your change.  Imagine that the
     title appears in a list of 600 other commits that goes in a
     single release in "git shortlog --no-merges" output.  Does it
     tell readers of the list what the change is about?  We cannot
     even guess that it is about the project list that appears in
     gitweb output.

 (2) Please explain what problem this is trying to solve; that is
     what the blank space before "---" line we see up above is for.
     What happens in the current code under what condition, until we
     do not apply this patch, and why is it a bad thing to happen?
     Once we apply this patch, in what way the situation gets
     improved?

 (3) Please sign-off your patch (see SubmittingPatches in
     Documentation).

Thanks.

Show 17 quoted lines

>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 3d4a8ee27c96a..9208f42ed1753 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3071,6 +3071,8 @@ sub git_get_projects_list {
>  				return if (m!^[/.]$!);
>  				# only directories can be git repositories
>  				return unless (-d $_);
> +				# need search permission
> +				return unless (-x $_);
>  				# don't traverse too deep (Find is super slow on os x)
>  				# $project_maxdepth excludes depth of $projectroot
>  				if (($File::Find::name =~ tr!/!!) - $pfxdepth > $project_maxdepth) {
>
> --
> https://github.com/git/git/pull/384