Invalid search parameter in webinterface
Hello.
An 'author' search string like "torvalds@linux" at
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git
generates a 'next' link due to the huge number of commits.
This link has an incorrect escaping for the @ sign. The backslash does not work, it generates an error:
403 Forbidden - Invalid search parameter
It should be s=torvalds%40linux instead of s=torvalds\@linux
Olaf
PATCH 1/1 gitweb: escape searchtext and parameters for replay
Search texts may very likely include characters like '@' when grepping for author names. Currently, gitweb produces first/prev/next links with incorrectly escaped characters.
Make gitweb escape searchtext and parameters which are reused in href() when replay is set. (cgi params are de-escaped when put into the parameter dictionary and need to be re-escaped when reused.)
---
Maybe something like this? Highly untested!
Cheers,
Michael
gitweb/gitweb.perl | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
gitweb/gitweb.perl +2 -2
@@ -848,7 +848,7 @@ sub href (%) { if ($params{-replay}) { while (my ($name, $symbol) = each %cgi_param_mapping) { if (!exists $params{$name}) { - $params{$name} = $input_params{$name}; + $params{$name} = esc_url($input_params{$name}); } } }
@@ -5775,7 +5775,7 @@ sub git_search { if ($page > 0) { $paging_nav .= $cgi->a({-href => href(action=>"search", hash=>$hash, - searchtext=>$searchtext, + searchtext=>esc_url($searchtext), searchtype=>$searchtype)}, "first"); $paging_nav .= " ⋅ " .
Signature · 1 line
1.6.3.rc3
Re: [PATCH] gitweb: escape searchtext and parameters for replay
Quoted 4 lines · Michael J Gruber
Search texts may very likely include characters like '@' when grepping for author names. Currently, gitweb produces first/prev/next links with incorrectly escaped characters.
Quoted 19 lines
Make gitweb escape searchtext and parameters which are reused in
href()when replay is set. (cgi params are de-escaped when put into the parameter dictionary and need to be re-escaped when reused.)Signed-off-by Michael J Gruber <[email protected]>---
Quoted 10 lines · Olaf Hering
An 'author' search string like "torvalds@linux" at
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git
generates a 'next' link due to the huge number of commits.This link has an incorrect escaping for the @ sign. The backslash does not work, it generates an error:
403 Forbidden - Invalid search parameter
It should be s=torvalds%40linux instead of s=torvalds\@linux
If you by hand edit URL changing '\@' to simply '@', does changed gitweb URL works correctly?
Quoted 2 lines
Maybe something like this? Highly untested!
No, the problem is not with lack of URI escaping in 's' parameter ('@' character is not forbidden for query string; it has special meaning and has to be escaped only in the hostname part), but with passing value _quoted for Perl_ (quotemeta) to href() subroutine.
I'll try to come with the solution soon.
Quoted 17 lines
gitweb/gitweb.perl | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)gitweb/gitweb.perl +1
-1@@ -848,7 +848,7 @@ sub href (%) { if ($params{-replay}) { while (my ($name, $symbol) = each %cgi_param_mapping) { if (!exists $params{$name}) { - $params{$name} = $input_params{$name}; + $params{$name} = esc_url($input_params{$name}); } } }
gitweb/gitweb.perl sub git_search { · line 5781
Quoted 8 lines
gitweb/gitweb.perl +1
-1@@ -5775,7 +5775,7 @@ sub git_search { if ($page > 0) { $paging_nav .= $cgi->a({-href => href(action=>"search", hash=>$hash, - searchtext=>$searchtext, + searchtext=>esc_url($searchtext), searchtype=>$searchtype)}, "first"); $paging_nav .= " ⋅ " .
This would result in double URI escaping, as at the end of href() subroutine we have:
push @result, $symbol . "=" . esc_param($params{$name});[...]
$href .= "?" . join(';', @result) if scalar @result;return $href;
Note that gitweb uses esc_param() and not esc_url() here; the rules are slightly different (you need to quote/escape ';', '?', '=' etc. for query parameter).
Signature · 2 lines
Jakub Narebski Poland
Re: [PATCH] gitweb: escape searchtext and parameters for replay
Am 29.04.2009 um 14:28 schrieb Jakub Narebski:
Quoted 6 lines · Michael J Gruber
Quoted 2 lines
Quoted 2 lines
It should be s=torvalds%40linux instead of s=torvalds\@linux
If you by hand edit URL changing '\@' to simply '@', does changed gitweb URL works correctly?
I tried [email protected], and @ and . was escaped with a backslash. Removing both, and using the plain mail address worked.
Olaf
Re: [PATCH] gitweb: escape searchtext and parameters for replay
Quoted 10 lines · Olaf Hering
Am 29.04.2009 um 14:28 schrieb Jakub Narebski:
Quoted 6 lines · Michael J Gruber
Quoted 2 lines
Quoted 2 lines
It should be s=torvalds%40linux instead of s=torvalds\@linux
If you by hand edit URL changing '\@' to simply '@', does changed gitweb URL works correctly?
I tried [email protected], and @ and . was escaped with a backslash. Removing both, and using the plain mail address worked.
This problem was fixed in 7e431ef (gitweb: Separate search regexp from search text), but this fix is present in git 1.5.2 and later, and kernel.org gitweb is a fork (adding caching mechanism) of 1.4.5-rc0, according to info in git.kernel.org HTML source, and it does not have this fix.
http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary
I have CC-ed J.H. on this.
Signature · 2 lines
Jakub Narebski Poland
Re: [PATCH] gitweb: escape searchtext and parameters for replay
Quoted 6 lines · Jakub Narebski
Quoted 5 lines · Olaf Hering
Am 29.04.2009 um 14:28 schrieb Jakub Narebski:
Quoted 4 lines · Michael J Gruber
Quoted 2 lines
Quoted 2 lines
It should be s=torvalds%40linux instead of s=torvalds\@linux
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
If you by hand edit URL changing '\@' to simply '@', does changed gitweb URL works correctly?
I tried [email protected], and @ and . was escaped with a backslash. Removing both, and using the plain mail address worked.
This problem was fixed in 7e431ef (gitweb: Separate search regexp from search text), but this fix is present in git 1.5.2 and later,
I have just checked that current gitweb (1.6.2.rc1.20.g8c5b) does not have this bug...
Quoted 7 lines
and kernel.org gitweb is a fork (adding caching mechanism) of 1.4.5-rc0 (according to info in git.kernel.org HTML source), and it does not have this fix.
http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary
I have CC-ed J.H. on this.
... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org should backport this fix to kernel.org's fork of gitweb sources.
Signature · 2 lines
Jakub Narebski Poland
Re: [PATCH] gitweb: escape searchtext and parameters for replay
Jakub Narebski venit, vidit, dixit 29.04.2009 21:36:
Quoted 10 lines · Jakub Narebski
Quoted 9 lines · Olaf Hering
Quoted 8 lines
Am 29.04.2009 um 14:28 schrieb Jakub Narebski:
Quoted 6 lines · Michael J Gruber
Quoted 2 lines
Quoted 2 lines
It should be s=torvalds%40linux instead of s=torvalds\@linux
If you by hand edit URL changing '\@' to simply '@', does changed gitweb URL works correctly?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
I tried [email protected], and @ and . was escaped with a backslash. Removing both, and using the plain mail address worked.
This problem was fixed in 7e431ef (gitweb: Separate search regexp from search text), but this fix is present in git 1.5.2 and later,
I have just checked that current gitweb (1.6.2.rc1.20.g8c5b) does not have this bug...
Yes, I should have tested before suggesting a patch blindly. Sorry!
Quoted 12 lines
Quoted 8 lines
and kernel.org gitweb is a fork (adding caching mechanism) of 1.4.5-rc0 (according to info in git.kernel.org HTML source), and it does not have this fix.
http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary
I have CC-ed J.H. on this.
... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org should backport this fix to kernel.org's fork of gitweb sources.
Re: [PATCH] gitweb: escape searchtext and parameters for replay
In reply to Jakub NarebskiQuoted 18 lines · Jakub Narebski
Quoted 9 lines · Jakub Narebski
Quoted 7 lines · Olaf Hering
Am 29.04.2009 um 14:28 schrieb Jakub Narebski:
Quoted 4 lines · Michael J Gruber
Quoted 1 line
Quoted 1 line
It should be s=torvalds%40linux instead of s=torvalds\@linux
If you by hand edit URL changing '\@' to simply '@', does changed gitweb URL works correctly?
I tried [email protected], and @ and . was escaped with a backslash. Removing both, and using the plain mail address worked.
This problem was fixed in 7e431ef (gitweb: Separate search regexp from search text), but this fix is present in git 1.5.2 and later,
I have just checked that current gitweb (1.6.2.rc1.20.g8c5b) does not have this bug...
Quoted 4 lines
and kernel.org gitweb is a fork (adding caching mechanism) of 1.4.5-rc0 (according to info in git.kernel.org HTML source), and it does not have this fix.
Quoted 6 lines
Quoted 3 lines
http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary
I have CC-ed J.H. on this.
... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org should backport this fix to kernel.org's fork of gitweb sources.
Patches, as always, are welcome - I've got a few queued up already, but if I have to do it I won't be getting back around to it for a bit (there are other things on my priority list right now).
- John "Warthog9" Hawley
PATCH 1/1 gitweb: Separate search regexp from search text
Separate search text, which is saved in $searchtext global variable, and is used in links, as default value for the textfield in search form, and for pickaxe search, from search regexp, which is saved in $search_regexp global variable, and is used as parameter to --grep, --committer or --author options to git-rev-list, and for searching commit body in gitweb. For now $search_regexp is unconditionally equal to quotemeta($searchtext), meaning that we always search for fixed string.
This fixes bug where 'next page' links for 'search' view didn't work for searchtext containing quotable characters, like `@'.
(manually cherry picked from 7e431ef9ab933d7ff899a999e40627ab49774f3a)
Olaf Hering noticed that this bug was still present, and this bugfix was not ported to kernel.org's fork of gitweb.
---
Quoted 4 lines · J.H.
Quoted 2 lines · Jakub Narebski
I have just checked that current gitweb (1.6.2.rc1.20.g8c5b) does not have this bug...
[...]
Quoted 6 lines
Quoted 2 lines
... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org should backport this fix to kernel.org's fork of gitweb sources.
Patches, as always, are welcome - I've got a few queued up already, but if I have to do it I won't be getting back around to it for a bit (there are other things on my priority list right now).
Here you have backport of 7e431ef (gitweb: Separate search regexp from search text) on top of 'master' branch of kernel.org's fork of gitweb:
git://git.kernel.org/pub/scm/git/warthog9/gitweb.git
The only difference is that git_search_grep_body is in separate gitweb/gitweb/search.pm file in kernel.org's fork. The change is otherwise identical.
Not tested!
gitweb/gitweb.perl | 5 +++-- gitweb/gitweb/search.pm | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-)
gitweb/gitweb.perl +3 -2
@@ -231,6 +231,7 @@ if (defined $page) { } our $searchtext = $cgi->param('s'); +our $search_regexp; if (defined $searchtext) { if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) { die_error(undef, "Invalid search parameter");
@@ -238,7 +239,7 @@ if (defined $searchtext) { if (length($searchtext) < 2) { die_error(undef, "At least two characters are required for search parameter"); } - $searchtext = quotemeta $searchtext; + $search_regexp = quotemeta $searchtext; } our $searchtype = $cgi->param('st');
@@ -1702,7 +1703,7 @@ sub git_search { } elsif ($searchtype eq 'committer') { $greptype = "--committer="; } - $greptype .= $searchtext; + $greptype .= $search_regexp; my @commitlist = parse_commits($hash, 101, (100 * $page), $greptype); my $paging_nav = '';
gitweb/gitweb/search.pm +1 -1
@@ -33,7 +33,7 @@ sub git_search_grep_body { esc_html(chop_str($co{'title'}, 50)) . "<br/>"); my $comment = $co{'comment'}; foreach my $line (@$comment) { - if ($line =~ m/^(.*)($searchtext)(.*)$/i) { + if ($line =~ m/^(.*)($search_regexp)(.*)$/i) { my $lead = esc_html($1) || ""; $lead = chop_str($lead, 30, 10); my $match = esc_html($2) || "";
Signature · 1 line
1.6.2