threads / patch / 46410

patchcheck for search permission

Subject: [PATCH] check for search permission

## tl;dr

4 messages between Jul 17, 2017 and Jul 18, 2017. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Hielke Christian Braun· Jul 17, 2017, 19:26 UTC · lore

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

Show changes to gitweb/gitweb.perl +2 −1

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

Junio C Hamano· Jul 17, 2017, 20:18 UTC · re: Hielke Christian Braun · lore

Re: [PATCH] check for search permission

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

Hielke Christian Braun· Jul 18, 2017, 08:41 UTC · re: Junio C Hamano · lore

[PATCH] gitweb bugfix - check for search permission on sub-directories while scanning project root to prevent program termination

Hi,

gitweb terminates and shows no project list, if it can not access a sub-directory in the project root directory. It should show a list of the projects it can access. Patch corrects this by skipping inaccessible directories.

Signed-off-by: Hielke Christian Braun <hcb@unco.de>

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

Show changes to gitweb/gitweb.perl +2 −1

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






On 2017-07-17 22:18, Junio C Hamano wrote:
> 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.
> 
>>
>> 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

Junio C Hamano· Jul 18, 2017, 21:06 UTC · re: Hielke Christian Braun · lore

Re: [PATCH] gitweb bugfix - check for search permission on sub-directories while scanning project root to prevent program termination

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

Show 31 quoted lines

> Hi,
>
> gitweb terminates and shows no project list, if it can not access a
> sub-directory in the project root directory. It should show a list of
> the projects it can access. Patch corrects this by skipping inaccessible
> directories.
>
>
> Signed-off-by: Hielke Christian Braun <hcb@unco.de>
>
>
> ---
>  gitweb/gitweb.perl | 2 ++
>  1 file changed, 2 insertions(+)
>
> 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

I've tweaked the title and the log message further before queuing the patch.

Thanks.