From e0ca364551fcde3b3d825f93f4a4d44d7ab9eb35 Mon Sep 17 00:00:00 2001 From: Jakub Narebski Date: Wed, 22 Jun 2011 17:28:52 +0200 Subject: gitweb: Check permissions first in git_search Check first if relevant features: 'search', 'pickaxe', 'grep', as appropriate, are enabled before doing anything else in git_search. This should make git_search code more clear. While at it, expand a bit error message (e.g. 'Pickaxe' -> 'Pickaxe search'). Signed-off-by: Jakub Narebski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) (limited to 'gitweb') diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 2fd438905..cde39131f 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -313,6 +313,10 @@ our %feature = ( # Enable text search, which will list the commits which match author, # committer or commit text to a given string. Enabled by default. # Project specific override is not supported. + # + # Note that this controls all search features, which means that if + # it is disabled, then 'grep' and 'pickaxe' search would also be + # disabled. 'search' => { 'override' => 0, 'default' => [1]}, @@ -6787,7 +6791,23 @@ sub git_history { } sub git_search { - gitweb_check_feature('search') or die_error(403, "Search is disabled"); + $searchtype ||= 'commit'; + + # check if appropriate features are enabled + gitweb_check_feature('search') + or die_error(403, "Search is disabled"); + if ($searchtype eq 'pickaxe') { + # pickaxe may take all resources of your box and run for several minutes + # with every query - so decide by yourself how public you make this feature + gitweb_check_feature('pickaxe') + or die_error(403, "Pickaxe search is disabled"); + } + if ($searchtype eq 'grep') { + # grep search might be potentially CPU-intensive, too + gitweb_check_feature('grep') + or die_error(403, "Grep search is disabled"); + } + if (!defined $searchtext) { die_error(400, "Text field is empty"); } @@ -6802,18 +6822,6 @@ sub git_search { $page = 0; } - $searchtype ||= 'commit'; - if ($searchtype eq 'pickaxe') { - # pickaxe may take all resources of your box and run for several minutes - # with every query - so decide by yourself how public you make this feature - gitweb_check_feature('pickaxe') - or die_error(403, "Pickaxe is disabled"); - } - if ($searchtype eq 'grep') { - gitweb_check_feature('grep') - or die_error(403, "Grep is disabled"); - } - git_header_html(); if ($searchtype eq 'commit' or $searchtype eq 'author' or $searchtype eq 'committer') { -- cgit v1.2.1 From 16f20725bbcb7abbbc11e740050197b7fe8f843a Mon Sep 17 00:00:00 2001 From: Jakub Narebski Date: Wed, 22 Jun 2011 17:28:53 +0200 Subject: gitweb: Split body of git_search into subroutines Create separate subroutines for handling each of aspects of searching the repository: * git_search_message ('commit', 'author', 'committer') * git_search_changes ('pickaxe') * git_search_content_of_files ('grep') Almost pure code movement (and unindent), which you can check e.g. via $ git blame -w --date=short -C -C HEAD^..HEAD -- gitweb/gitweb.perl | grep -C 3 -e '^[^^]' | less -S No functional changes intended. Signed-off-by: Jakub Narebski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 381 ++++++++++++++++++++++++++++------------------------- 1 file changed, 199 insertions(+), 182 deletions(-) (limited to 'gitweb') diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index cde39131f..9d9dc0149 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -5254,6 +5254,197 @@ sub git_remotes_body { } } +sub git_search_message { + my %co = @_; + + my $greptype; + if ($searchtype eq 'commit') { + $greptype = "--grep="; + } elsif ($searchtype eq 'author') { + $greptype = "--author="; + } elsif ($searchtype eq 'committer') { + $greptype = "--committer="; + } + $greptype .= $searchtext; + my @commitlist = parse_commits($hash, 101, (100 * $page), undef, + $greptype, '--regexp-ignore-case', + $search_use_regexp ? '--extended-regexp' : '--fixed-strings'); + + my $paging_nav = ''; + if ($page > 0) { + $paging_nav .= + $cgi->a({-href => href(action=>"search", hash=>$hash, + searchtext=>$searchtext, + searchtype=>$searchtype)}, + "first"); + $paging_nav .= " ⋅ " . + $cgi->a({-href => href(-replay=>1, page=>$page-1), + -accesskey => "p", -title => "Alt-p"}, "prev"); + } else { + $paging_nav .= "first"; + $paging_nav .= " ⋅ prev"; + } + my $next_link = ''; + if ($#commitlist >= 100) { + $next_link = + $cgi->a({-href => href(-replay=>1, page=>$page+1), + -accesskey => "n", -title => "Alt-n"}, "next"); + $paging_nav .= " ⋅ $next_link"; + } else { + $paging_nav .= " ⋅ next"; + } + + git_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav); + git_print_header_div('commit', esc_html($co{'title'}), $hash); + if ($page == 0 && !@commitlist) { + print "

No match.

\n"; + } else { + git_search_grep_body(\@commitlist, 0, 99, $next_link); + } +} + +sub git_search_changes { + my %co = @_; + + git_print_page_nav('','', $hash,$co{'tree'},$hash); + git_print_header_div('commit', esc_html($co{'title'}), $hash); + + print "\n"; + my $alternate = 1; + local $/ = "\n"; + open my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts, + '--pretty=format:%H', '--no-abbrev', '--raw', "-S$searchtext", + ($search_use_regexp ? '--pickaxe-regex' : ()); + undef %co; + my @files; + while (my $line = <$fd>) { + chomp $line; + next unless $line; + + my %set = parse_difftree_raw_line($line); + if (defined $set{'commit'}) { + # finish previous commit + if (%co) { + print "\n" . + "\n" . + "\n"; + } + + if ($alternate) { + print "\n"; + } else { + print "\n"; + } + $alternate ^= 1; + %co = parse_commit($set{'commit'}); + my $author = chop_and_escape_str($co{'author_name'}, 15, 5); + print "\n" . + "\n" . + "\n" . + "\n" . + "\n"; + } + + print "
" . + $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, "commit") . + " | " . + $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, hash_base=>$co{'id'})}, "tree"); + print "
$co{'age_string_date'}$author" . + $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}), + -class => "list subject"}, + chop_and_escape_str($co{'title'}, 50) . "
"); + } elsif (defined $set{'to_id'}) { + next if ($set{'to_id'} =~ m/^0{40}$/); + + print $cgi->a({-href => href(action=>"blob", hash_base=>$co{'id'}, + hash=>$set{'to_id'}, file_name=>$set{'to_file'}), + -class => "list"}, + "" . esc_path($set{'file'}) . "") . + "
\n"; + } + } + close $fd; + + # finish last commit (warning: repetition!) + if (%co) { + print "		" . + $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, "commit") . + " | " . + $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, hash_base=>$co{'id'})}, "tree"); + print "
\n"; +} + +sub git_search_files { + my %co = @_; + + git_print_page_nav('','', $hash,$co{'tree'},$hash); + git_print_header_div('commit', esc_html($co{'title'}), $hash); + + print "\n"; + my $alternate = 1; + my $matches = 0; + local $/ = "\n"; + open my $fd, "-|", git_cmd(), 'grep', '-n', + $search_use_regexp ? ('-E', '-i') : '-F', + $searchtext, $co{'tree'}; + my $lastfile = ''; + while (my $line = <$fd>) { + chomp $line; + my ($file, $lno, $ltext, $binary); + last if ($matches++ > 1000); + if ($line =~ /^Binary file (.+) matches$/) { + $file = $1; + $binary = 1; + } else { + (undef, $file, $lno, $ltext) = split(/:/, $line, 4); + } + if ($file ne $lastfile) { + $lastfile and print "\n"; + if ($alternate++) { + print "\n"; + } else { + print "\n"; + } + print "\n"; + if ($matches > 1000) { + print "
Too many matches, listing trimmed
\n"; + } + } else { + print "
No matches found
\n"; + } + close $fd; + + print "
". + $cgi->a({-href => href(action=>"blob", hash=>$co{'hash'}, + file_name=>"$file"), + -class => "list"}, esc_path($file)); + print "\n"; + $lastfile = $file; + } + if ($binary) { + print "
Binary file
\n"; + } else { + $ltext = untabify($ltext); + if ($ltext =~ m/^(.*)($search_regexp)(.*)$/i) { + $ltext = esc_html($1, -nbsp=>1); + $ltext .= ''; + $ltext .= esc_html($2, -nbsp=>1); + $ltext .= ''; + $ltext .= esc_html($3, -nbsp=>1); + } else { + $ltext = esc_html($ltext, -nbsp=>1); + } + print "
" . + $cgi->a({-href => href(action=>"blob", hash=>$co{'hash'}, + file_name=>"$file").'#l'.$lno, + -class => "linenr"}, sprintf('%4i', $lno)) + . ' ' . $ltext . "
\n"; + } + } + if ($lastfile) { + print "
\n"; +} + sub git_search_grep_body { my ($commitlist, $from, $to, $extra) = @_; $from = 0 unless defined $from; @@ -6824,190 +7015,16 @@ sub git_search { git_header_html(); - if ($searchtype eq 'commit' or $searchtype eq 'author' or $searchtype eq 'committer') { - my $greptype; - if ($searchtype eq 'commit') { - $greptype = "--grep="; - } elsif ($searchtype eq 'author') { - $greptype = "--author="; - } elsif ($searchtype eq 'committer') { - $greptype = "--committer="; - } - $greptype .= $searchtext; - my @commitlist = parse_commits($hash, 101, (100 * $page), undef, - $greptype, '--regexp-ignore-case', - $search_use_regexp ? '--extended-regexp' : '--fixed-strings'); - - my $paging_nav = ''; - if ($page > 0) { - $paging_nav .= - $cgi->a({-href => href(action=>"search", hash=>$hash, - searchtext=>$searchtext, - searchtype=>$searchtype)}, - "first"); - $paging_nav .= " ⋅ " . - $cgi->a({-href => href(-replay=>1, page=>$page-1), - -accesskey => "p", -title => "Alt-p"}, "prev"); - } else { - $paging_nav .= "first"; - $paging_nav .= " ⋅ prev"; - } - my $next_link = ''; - if ($#commitlist >= 100) { - $next_link = - $cgi->a({-href => href(-replay=>1, page=>$page+1), - -accesskey => "n", -title => "Alt-n"}, "next"); - $paging_nav .= " ⋅ $next_link"; - } else { - $paging_nav .= " ⋅ next"; - } - - git_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav); - git_print_header_div('commit', esc_html($co{'title'}), $hash); - if ($page == 0 && !@commitlist) { - print "

No match.

\n"; - } else { - git_search_grep_body(\@commitlist, 0, 99, $next_link); - } + if ($searchtype eq 'commit' || + $searchtype eq 'author' || + $searchtype eq 'committer') { + git_search_message(%co); + } elsif ($searchtype eq 'pickaxe') { + git_search_changes(%co); + } elsif ($searchtype eq 'grep') { + git_search_files(%co); } - if ($searchtype eq 'pickaxe') { - git_print_page_nav('','', $hash,$co{'tree'},$hash); - git_print_header_div('commit', esc_html($co{'title'}), $hash); - - print "\n"; - my $alternate = 1; - local $/ = "\n"; - open my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts, - '--pretty=format:%H', '--no-abbrev', '--raw', "-S$searchtext", - ($search_use_regexp ? '--pickaxe-regex' : ()); - undef %co; - my @files; - while (my $line = <$fd>) { - chomp $line; - next unless $line; - - my %set = parse_difftree_raw_line($line); - if (defined $set{'commit'}) { - # finish previous commit - if (%co) { - print "\n" . - "\n" . - "\n"; - } - - if ($alternate) { - print "\n"; - } else { - print "\n"; - } - $alternate ^= 1; - %co = parse_commit($set{'commit'}); - my $author = chop_and_escape_str($co{'author_name'}, 15, 5); - print "\n" . - "\n" . - "\n" . - "\n" . - "\n"; - } - - print "
" . - $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, "commit") . - " | " . - $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, hash_base=>$co{'id'})}, "tree"); - print "
$co{'age_string_date'}$author" . - $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}), - -class => "list subject"}, - chop_and_escape_str($co{'title'}, 50) . "
"); - } elsif (defined $set{'to_id'}) { - next if ($set{'to_id'} =~ m/^0{40}$/); - - print $cgi->a({-href => href(action=>"blob", hash_base=>$co{'id'}, - hash=>$set{'to_id'}, file_name=>$set{'to_file'}), - -class => "list"}, - "" . esc_path($set{'file'}) . "") . - "
\n"; - } - } - close $fd; - - # finish last commit (warning: repetition!) - if (%co) { - print "		" . - $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, "commit") . - " | " . - $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, hash_base=>$co{'id'})}, "tree"); - print "
\n"; - } - - if ($searchtype eq 'grep') { - git_print_page_nav('','', $hash,$co{'tree'},$hash); - git_print_header_div('commit', esc_html($co{'title'}), $hash); - - print "\n"; - my $alternate = 1; - my $matches = 0; - local $/ = "\n"; - open my $fd, "-|", git_cmd(), 'grep', '-n', - $search_use_regexp ? ('-E', '-i') : '-F', - $searchtext, $co{'tree'}; - my $lastfile = ''; - while (my $line = <$fd>) { - chomp $line; - my ($file, $lno, $ltext, $binary); - last if ($matches++ > 1000); - if ($line =~ /^Binary file (.+) matches$/) { - $file = $1; - $binary = 1; - } else { - (undef, $file, $lno, $ltext) = split(/:/, $line, 4); - } - if ($file ne $lastfile) { - $lastfile and print "\n"; - if ($alternate++) { - print "\n"; - } else { - print "\n"; - } - print "\n"; - if ($matches > 1000) { - print "
Too many matches, listing trimmed
\n"; - } - } else { - print "
No matches found
\n"; - } - close $fd; - - print "
". - $cgi->a({-href => href(action=>"blob", hash=>$co{'hash'}, - file_name=>"$file"), - -class => "list"}, esc_path($file)); - print "\n"; - $lastfile = $file; - } - if ($binary) { - print "
Binary file
\n"; - } else { - $ltext = untabify($ltext); - if ($ltext =~ m/^(.*)($search_regexp)(.*)$/i) { - $ltext = esc_html($1, -nbsp=>1); - $ltext .= ''; - $ltext .= esc_html($2, -nbsp=>1); - $ltext .= ''; - $ltext .= esc_html($3, -nbsp=>1); - } else { - $ltext = esc_html($ltext, -nbsp=>1); - } - print "
" . - $cgi->a({-href => href(action=>"blob", hash=>$co{'hash'}, - file_name=>"$file").'#l'.$lno, - -class => "linenr"}, sprintf('%4i', $lno)) - . ' ' . $ltext . "
\n"; - } - } - if ($lastfile) { - print "
\n"; - } git_footer_html(); } -- cgit v1.2.1 From 882541b87d8f7bedca824228a8ad14aed69c904e Mon Sep 17 00:00:00 2001 From: Jakub Narebski Date: Wed, 22 Jun 2011 17:28:54 +0200 Subject: gitweb: Clean up code in git_search_* subroutines Replace sequence of $foo .= "bar"; $foo .= "baz"; with $foo .= "bar" . "baz"; Use href(-replay=>1, -page=>undef) for first page of a multipl-page view. Wrap some lines to reduce their length. Some lines still have more than 80 characters, but lines are shorter now. No functional changes intended. Signed-off-by: Jakub Narebski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) (limited to 'gitweb') diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 9d9dc0149..80cbe66ea 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -5273,16 +5273,13 @@ sub git_search_message { my $paging_nav = ''; if ($page > 0) { $paging_nav .= - $cgi->a({-href => href(action=>"search", hash=>$hash, - searchtext=>$searchtext, - searchtype=>$searchtype)}, - "first"); - $paging_nav .= " ⋅ " . + $cgi->a({-href => href(-replay=>1, page=>undef)}, + "first") . + " ⋅ " . $cgi->a({-href => href(-replay=>1, page=>$page-1), -accesskey => "p", -title => "Alt-p"}, "prev"); } else { - $paging_nav .= "first"; - $paging_nav .= " ⋅ prev"; + $paging_nav .= "first ⋅ prev"; } my $next_link = ''; if ($#commitlist >= 100) { @@ -5327,10 +5324,13 @@ sub git_search_changes { if (%co) { print "\n" . "" . - $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, "commit") . + $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, + "commit") . " | " . - $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, hash_base=>$co{'id'})}, "tree"); - print "\n" . + $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, + hash_base=>$co{'id'})}, + "tree") . + "\n" . "\n"; } @@ -5364,10 +5364,13 @@ sub git_search_changes { if (%co) { print "\n" . "" . - $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, "commit") . + $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, + "commit") . " | " . - $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, hash_base=>$co{'id'})}, "tree"); - print "\n" . + $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, + hash_base=>$co{'id'})}, + "tree") . + "\n" . "\n"; } -- cgit v1.2.1 From 1ae05be4aa448163a381e131396fd52dcf6f83eb Mon Sep 17 00:00:00 2001 From: Jakub Narebski Date: Wed, 22 Jun 2011 17:28:55 +0200 Subject: gitweb: Make git_search_* subroutines render whole pages Move git_header_html() and git_footer_html() invocation from git_search() to individual git_search_* subroutines. While at it, reorganize search-related code a bit, moving invoking of git commands before any output is generated. This has the following advantages: * gitweb now shows an error page if there was unknown search type (evaluate_and_validate_params checks only that it looks sanely); remember that we shouldn't call die_error after any output. * git_search_message is now safe agains die_error in parse_commits (though this is very unlikely). * gitweb now can check errors while invoking git commands and show error page (again, quite unlikely). Signed-off-by: Jakub Narebski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) (limited to 'gitweb') diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 80cbe66ea..fc27477b6 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -5291,6 +5291,8 @@ sub git_search_message { $paging_nav .= " ⋅ next"; } + git_header_html(); + git_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav); git_print_header_div('commit', esc_html($co{'title'}), $hash); if ($page == 0 && !@commitlist) { @@ -5298,20 +5300,26 @@ sub git_search_message { } else { git_search_grep_body(\@commitlist, 0, 99, $next_link); } + + git_footer_html(); } sub git_search_changes { my %co = @_; + local $/ = "\n"; + open my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts, + '--pretty=format:%H', '--no-abbrev', '--raw', "-S$searchtext", + ($search_use_regexp ? '--pickaxe-regex' : ()) + or die_error(500, "Open git-log failed"); + + git_header_html(); + git_print_page_nav('','', $hash,$co{'tree'},$hash); git_print_header_div('commit', esc_html($co{'title'}), $hash); print "\n"; my $alternate = 1; - local $/ = "\n"; - open my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts, - '--pretty=format:%H', '--no-abbrev', '--raw', "-S$searchtext", - ($search_use_regexp ? '--pickaxe-regex' : ()); undef %co; my @files; while (my $line = <$fd>) { @@ -5375,21 +5383,27 @@ sub git_search_changes { } print "
\n"; + + git_footer_html(); } sub git_search_files { my %co = @_; + local $/ = "\n"; + open my $fd, "-|", git_cmd(), 'grep', '-n', + $search_use_regexp ? ('-E', '-i') : '-F', + $searchtext, $co{'tree'} + or die_error(500, "Open git-grep failed"); + + git_header_html(); + git_print_page_nav('','', $hash,$co{'tree'},$hash); git_print_header_div('commit', esc_html($co{'title'}), $hash); print "\n"; my $alternate = 1; my $matches = 0; - local $/ = "\n"; - open my $fd, "-|", git_cmd(), 'grep', '-n', - $search_use_regexp ? ('-E', '-i') : '-F', - $searchtext, $co{'tree'}; my $lastfile = ''; while (my $line = <$fd>) { chomp $line; @@ -5446,6 +5460,8 @@ sub git_search_files { close $fd; print "
\n"; + + git_footer_html(); } sub git_search_grep_body { @@ -7016,8 +7032,6 @@ sub git_search { $page = 0; } - git_header_html(); - if ($searchtype eq 'commit' || $searchtype eq 'author' || $searchtype eq 'committer') { @@ -7026,9 +7040,9 @@ sub git_search { git_search_changes(%co); } elsif ($searchtype eq 'grep') { git_search_files(%co); + } else { + die_error(400, "Unknown search type"); } - - git_footer_html(); } sub git_search_help { -- cgit v1.2.1