{"thread":{"id":"29309","subject":"[PATCH/RFC] gitweb: Fix actionless dispatch for non-existent objects","startedAt":"2012-01-07T10:47:38Z","lastAt":"2012-01-09T22:05:09Z","messageCount":3,"participants":["Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"182059","messageId":"20120107104552.26867.41282.stgit@localhost.localdomain","threadId":"29309","inReplyTo":null,"subject":"[PATCH/RFC] gitweb: Fix actionless dispatch for non-existent objects","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-01-07T10:47:38Z","receivedAt":"2012-01-07T10:47:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"When gitweb URL does not provide action explicitly, e.g.\n\n  http://git.example.org/repo.git/branch\n\ndispatch() tries to guess action (view to be used) based on remaining\nparameters.  Among others it is based on the type of requested object,\nwhich gave problems when asking for non-existent branch or file (for\nexample misspelt name).\n\nNow undefined $action from dispatch() should not result in problems.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nI'm not sure if this is the way to fix it, by erroring-out in\ndispatch() and leaving $action undefined.\n\nTestsuite passes, but I have not examined output intensively.\n\n gitweb/gitweb.perl                     |    4 +++-\n t/t9500-gitweb-standalone-no-errors.sh |    8 ++++++++\n 2 files changed, 11 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex f884dfe..e2e04df 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1123,8 +1123,10 @@ sub dispatch {\n \tif (!defined $action) {\n \t\tif (defined $hash) {\n \t\t\t$action = git_get_type($hash);\n+\t\t\t$action or die_error(404, \"Object does not exist\");\n \t\t} elsif (defined $hash_base && defined $file_name) {\n \t\t\t$action = git_get_type(\"$hash_base:$file_name\");\n+\t\t\t$action or die_error(404, \"File or directory does not exist\");\n \t\t} elsif (defined $project) {\n \t\t\t$action = 'summary';\n \t\t} else {\n@@ -2391,7 +2393,7 @@ sub get_feed_info {\n \treturn unless (defined $project);\n \t# some views should link to OPML, or to generic project feed,\n \t# or don't have specific feed yet (so they should use generic)\n-\treturn if ($action =~ /^(?:tags|heads|forks|tag|search)$/x);\n+\treturn if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);\n \n \tmy $branch;\n \t# branches refs uses 'refs/heads/' prefix (fullname) to differentiate\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex ab24917..0f771c6 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -475,6 +475,14 @@ test_expect_success \\\n \t'gitweb_run \"\" \"/.git/master:foo/\"'\n \n test_expect_success \\\n+\t'path_info: project/branch (non-existent)' \\\n+\t'gitweb_run \"\" \"/.git/non-existent\"'\n+\n+test_expect_success \\\n+\t'path_info: project/branch:filename (non-existent branch)' \\\n+\t'gitweb_run \"\" \"/.git/non-existent:non-existent\"'\n+\n+test_expect_success \\\n \t'path_info: project/branch:file (non-existent)' \\\n \t'gitweb_run \"\" \"/.git/master:non-existent\"'\n \n"},{"id":"182182","messageId":"7v8vlgef7o.fsf@alter.siamese.dyndns.org","threadId":"29309","inReplyTo":"20120107104552.26867.41282.stgit@localhost.localdomain","subject":"Re: [PATCH/RFC] gitweb: Fix actionless dispatch for non-existent objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-09T21:30:35Z","receivedAt":"2012-01-09T21:30:35Z","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> When gitweb URL does not provide action explicitly, e.g.\n>\n>   http://git.example.org/repo.git/branch\n>\n> dispatch() tries to guess action (view to be used) based on remaining\n> parameters.  Among others it is based on the type of requested object,\n> which gave problems when asking for non-existent branch or file (for\n> example misspelt name).\n\nOk. \"gave problems\" is a bit unclear to see why explicitly calling\ndie_error() is an improvement, though. What is the nature of the\n\"problems\"? Giving a server error 500 because later codepaths tried to\ncall an undefined subroutine?\n\n> Now undefined $action from dispatch() should not result in problems.\n\nAgain, unspecified \"problems\" here. I'd like this sentence to end with\n\"should not result in X but gives an explicit '404 not found' error\".\n\nThanks.\n"},{"id":"182183","messageId":"201201092305.10517.jnareb@gmail.com","threadId":"29309","inReplyTo":"7v8vlgef7o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] gitweb: Fix actionless dispatch for non-existent objects","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-01-09T22:05:09Z","receivedAt":"2012-01-09T22:05:09Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > When gitweb URL does not provide action explicitly, e.g.\n> >\n> >   http://git.example.org/repo.git/branch\n> >\n> > dispatch() tries to guess action (view to be used) based on remaining\n> > parameters.  Among others it is based on the type of requested object,\n> > which gave problems when asking for non-existent branch or file (for\n> > example misspelt name)\n\n                          , because git_get_type() returns undef when\nrequested object does not exist, and $action was left undefined.\n\nThis resulted in Perl generating the \"Use of unitialized value\" warnings,\nwhich made it in error.log.  Additionally gitweb returned \"400 Bad Request\"\nerror instead of more informative \"404 Not Found\".\n \n> \n> Ok. \"gave problems\" is a bit unclear to see why explicitly calling\n> die_error() is an improvement, though. What is the nature of the\n> \"problems\"? Giving a server error 500 because later codepaths tried to\n> call an undefined subroutine?\n> \n> > Now undefined $action from dispatch() should not result in problems.\n> \n> Again, unspecified \"problems\" here. I'd like this sentence to end with\n> \"should not result in X but gives an explicit '404 not found' error\".\n\nThis is about second chunk of change to gitweb/gitweb.perl, which is\nresponsible about silencing this warning:\n\n   gitweb.perl: Use of uninitialized value in pattern match (m//) at ../gitweb.perl line 2397.\n\nIt was present even with the '$action or die_error(404,...)' short-circut,\nas this was in the part responsible for generating page header, which is\nthe same for normal page and for error page.\n\nPerhaps better solution would be to set action to 'object' if requested\nobject is not found (a valid action in itself).  Hmmm...\n\n-- \nJakub Narebski\nPoland\n"}]}