{"thread":{"id":"16882","subject":"[PATCH] gitweb: Fix export check in git_get_projects_list","startedAt":"2008-12-27T09:39:31Z","lastAt":"2008-12-29T12:29:25Z","messageCount":4,"participants":["Devin Doucette","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"98782","messageId":"a899d7ef0812270139u7dd28ee1q45a8c05f7c95db2f@mail.gmail.com","threadId":"16882","inReplyTo":null,"subject":"[PATCH] gitweb: Fix export check in git_get_projects_list","fromName":"Devin Doucette","fromEmail":"devin@doucette.cc","sentAt":"2008-12-27T09:39:31Z","receivedAt":"2008-12-27T09:39:31Z","isPatch":true,"sender":{"key":"devin@doucette.cc","avatar":"https://gravatar.com/avatar/d5f21493801a4a508fd101dd9e432b02ac95fb58226f3d3f3417fbf147c8a1e3?d=mp&s=160"},"body":"When $filter was empty, the path passed to check_export_ok would\ncontain an extra '/', which some implementations of export_auth_hook\nare sensitive to.\n\nIt makes more sense to fix this here than to handle the special case\nin each implementation of export_auth_hook.\n\nSigned-off-by: Devin Doucette <devin@doucette.cc>\n---\n gitweb/gitweb.perl |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8f574c7..99f71b4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2147,8 +2147,9 @@ sub git_get_projects_list {\n\n \t\t\t\tmy $subdir = substr($File::Find::name, $pfxlen + 1);\n \t\t\t\t# we check related file in $projectroot\n-\t\t\t\tif (check_export_ok(\"$projectroot/$filter/$subdir\")) {\n-\t\t\t\t\tpush @list, { path => ($filter ? \"$filter/\" : '') . $subdir };\n+\t\t\t\tmy $path = ($filter ? \"$filter/\" : '') . $subdir;\n+\t\t\t\tif (check_export_ok(\"$projectroot/$path\")) {\n+\t\t\t\t\tpush @list, { path => $path };\n \t\t\t\t\t$File::Find::prune = 1;\n \t\t\t\t}\n \t\t\t},\n-- \n1.6.1.rc4\n"},{"id":"98829","messageId":"200812280312.02615.jnareb@gmail.com","threadId":"16882","inReplyTo":"a899d7ef0812270139u7dd28ee1q45a8c05f7c95db2f@mail.gmail.com","subject":"Re: [PATCH] gitweb: Fix export check in git_get_projects_list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-12-28T02:12:01Z","receivedAt":"2008-12-28T02:12:01Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 27 Dec 2008, Devin Doucette wrote:\n\n> When $filter was empty, the path passed to check_export_ok would\n> contain an extra '/', which some implementations of export_auth_hook\n> are sensitive to.\n> \n> It makes more sense to fix this here than to handle the special case\n> in each implementation of export_auth_hook.\n> \n> Signed-off-by: Devin Doucette <devin@doucette.cc>\n\nGood catch. Thanks.\n\nAcked-by: Jakub Narebski <jnareb@gmail.com>\n\n> ---\n>  gitweb/gitweb.perl |    5 +++--\n>  1 files changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 8f574c7..99f71b4 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2147,8 +2147,9 @@ sub git_get_projects_list {\n> \n>  \t\t\t\tmy $subdir = substr($File::Find::name, $pfxlen + 1);\n>  \t\t\t\t# we check related file in $projectroot\n> -\t\t\t\tif (check_export_ok(\"$projectroot/$filter/$subdir\")) {\n> -\t\t\t\t\tpush @list, { path => ($filter ? \"$filter/\" : '') . $subdir };\n> +\t\t\t\tmy $path = ($filter ? \"$filter/\" : '') . $subdir;\n\nNice and clear, but wouldn't be better to use slightly more effective\n\n+\t\t\t\tmy $path = $filter ? \"$filter/$subdir\" : $subdir;\n\nOr even, taking care of the case when $filter eq '0'\n\n+\t\t\t\tmy $path = defined $filter && $filter ne '' ? \"$filter/$subdir\" : $subdir;\n\n> +\t\t\t\tif (check_export_ok(\"$projectroot/$path\")) {\n> +\t\t\t\t\tpush @list, { path => $path };\n>  \t\t\t\t\t$File::Find::prune = 1;\n>  \t\t\t\t}\n>  \t\t\t},\n> -- \n> 1.6.1.rc4\n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"98832","messageId":"7vk59kaixf.fsf@gitster.siamese.dyndns.org","threadId":"16882","inReplyTo":"200812280312.02615.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Fix export check in git_get_projects_list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-28T06:54:36Z","receivedAt":"2008-12-28T06:54:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> On Sat, 27 Dec 2008, Devin Doucette wrote:\n>\n>> When $filter was empty, the path passed to check_export_ok would\n>> contain an extra '/', which some implementations of export_auth_hook\n>> are sensitive to.\n>> \n>> It makes more sense to fix this here than to handle the special case\n>> in each implementation of export_auth_hook.\n>> \n>> Signed-off-by: Devin Doucette <devin@doucette.cc>\n>\n> Good catch. Thanks.\n\nThanks, both.  Will queue for 'maint'.\n"},{"id":"98886","messageId":"200812291329.28701.jnareb@gmail.com","threadId":"16882","inReplyTo":"200812280312.02615.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Fix export check in git_get_projects_list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-12-29T12:29:25Z","receivedAt":"2008-12-29T12:29:25Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 28 Dec 2008, Jakub Narebski wrote:\n> On Sat, 27 Dec 2008, Devin Doucette wrote:\n\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 8f574c7..99f71b4 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -2147,8 +2147,9 @@ sub git_get_projects_list {\n> > \n> >  \t\t\t\tmy $subdir = substr($File::Find::name, $pfxlen + 1);\n> >  \t\t\t\t# we check related file in $projectroot\n> > -\t\t\t\tif (check_export_ok(\"$projectroot/$filter/$subdir\")) {\n> > -\t\t\t\t\tpush @list, { path => ($filter ? \"$filter/\" : '') . $subdir };\n> > +\t\t\t\tmy $path = ($filter ? \"$filter/\" : '') . $subdir;\n> \n> Nice and clear, but wouldn't be better to use slightly more effective\n> \n> +\t\t\t\tmy $path = $filter ? \"$filter/$subdir\" : $subdir;\n> \n> Or even, taking care of the case when $filter eq '0'\n> \n> +\t\t\t\tmy $path = defined $filter && $filter ne '' ? \"$filter/$subdir\" : $subdir;\n\nErr, actually with \"$filter ||= '';\" earlier in git_get_projects_list\nthis is not necessary, and is not an improvement.\n \n-- \nJakub Narebski\nPoland\n"}]}