# [PATCH] check for search permission 4 messages from 2017-07-17 to 2017-07-18. Participants: Hielke Christian Braun, Junio C Hamano. Thread: https://gitlist.dev/t/46410 ## Hielke Christian Braun, 2017-07-17 19:26 Subject: [PATCH] check for search permission Message-ID: <0102015d5203f2ae-c7f1daeb-a6ab-413d-9036-c8da816c4c18-000000@eu-west-1.amazonses.com> URL: https://gitlist.dev/e/0102015d5203f2ae-c7f1daeb-a6ab-413d-9036-c8da816c4c18-000000%40eu-west-1.amazonses.com ``` --- 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 ``` ## Junio C Hamano, 2017-07-17 20:18 Subject: Re: [PATCH] check for search permission Message-ID: URL: https://gitlist.dev/e/xmqqd18yhl5v.fsf%40gitster.mtv.corp.google.com In-Reply-To: <0102015d5203f2ae-c7f1daeb-a6ab-413d-9036-c8da816c4c18-000000@eu-west-1.amazonses.com> ``` Hielke Christian Braun 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 ``` ## Hielke Christian Braun, 2017-07-18 08:41 Subject: [PATCH] gitweb bugfix - check for search permission on sub-directories while scanning project root to prevent program termination Message-ID: <8e4adda7-11df-8a26-71a8-2e6429b04c67@unco.de> URL: https://gitlist.dev/e/8e4adda7-11df-8a26-71a8-2e6429b04c67%40unco.de In-Reply-To: ``` 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 --- 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 On 2017-07-17 22:18, Junio C Hamano wrote: > Hielke Christian Braun 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, 2017-07-18 21:06 Subject: Re: [PATCH] gitweb bugfix - check for search permission on sub-directories while scanning project root to prevent program termination Message-ID: URL: https://gitlist.dev/e/xmqq7ez5o3p1.fsf%40gitster.mtv.corp.google.com In-Reply-To: <8e4adda7-11df-8a26-71a8-2e6429b04c67@unco.de> ``` Hielke Christian Braun writes: > 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 > > > --- > 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. ```