{"thread":{"id":"19118","subject":"Invalid search parameter in webinterface","startedAt":"2009-04-29T09:12:54Z","lastAt":"2009-04-30T10:17:25Z","messageCount":9,"participants":["Olaf Hering","Michael J Gruber","Jakub Narebski","J.H."],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"112617","messageId":"337ECD47-D343-40F2-9E41-9E7D8531C5DF@aepfle.de","threadId":"19118","inReplyTo":null,"subject":"Invalid search parameter in webinterface","fromName":"Olaf Hering","fromEmail":"olaf@aepfle.de","sentAt":"2009-04-29T09:12:54Z","receivedAt":"2009-04-29T09:12:54Z","isPatch":false,"sender":{"key":"olaf@aepfle.de","avatar":"https://avatars.githubusercontent.com/u/942324?v=4"},"body":"\nHello.\n\nAn 'author' search string like \"torvalds@linux\" at\nhttp://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git\ngenerates a 'next' link due to the huge number of commits.\n\nThis link has an incorrect escaping for the @ sign.\nThe backslash does not work, it generates an error:\n\n403 Forbidden - Invalid search parameter\n\nIt should be s=torvalds%40linux instead of s=torvalds\\@linux\n\nOlaf\n"},{"id":"112619","messageId":"1241005459-17311-1-git-send-email-git@drmicha.warpmail.net","threadId":"19118","inReplyTo":"337ECD47-D343-40F2-9E41-9E7D8531C5DF@aepfle.de","subject":"[PATCH] gitweb: escape searchtext and parameters for replay","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-29T11:44:19Z","receivedAt":"2009-04-29T11:44:19Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Search texts may very likely include characters like '@' when grepping\nfor author names. Currently, gitweb produces first/prev/next links with\nincorrectly escaped characters.\n\nMake gitweb escape searchtext and parameters which are reused in href()\nwhen replay is set. (cgi params are de-escaped when put into the\nparameter dictionary and need to be re-escaped when reused.)\n\nSigned-off-by: Michael J Gruber \n---\nMaybe something like this? Highly untested!\n\nCheers,\nMichael\n\n\n gitweb/gitweb.perl | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3f99361..e1b09f8 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -848,7 +848,7 @@ sub href (%) {\n \tif ($params{-replay}) {\n \t\twhile (my ($name, $symbol) = each %cgi_param_mapping) {\n \t\t\tif (!exists $params{$name}) {\n-\t\t\t\t$params{$name} = $input_params{$name};\n+\t\t\t\t$params{$name} = esc_url($input_params{$name});\n \t\t\t}\n \t\t}\n \t}\n@@ -5775,7 +5775,7 @@ sub git_search {\n \t\tif ($page > 0) {\n \t\t\t$paging_nav .=\n \t\t\t\t$cgi->a({-href => href(action=>\"search\", hash=>$hash,\n-\t\t\t\t searchtext=>$searchtext,\n+\t\t\t\t searchtext=>esc_url($searchtext),\n \t\t\t\t searchtype=>$searchtype)},\n \t\t\t\t \"first\");\n \t\t\t$paging_nav .= \" ⋅ \" .\n-- \n1.6.3.rc3\n"},{"id":"112624","messageId":"200904291428.46244.jnareb@gmail.com","threadId":"19118","inReplyTo":"1241005459-17311-1-git-send-email-git@drmicha.warpmail.net","subject":"Re: [PATCH] gitweb: escape searchtext and parameters for replay","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-29T12:28:44Z","receivedAt":"2009-04-29T12:28:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 29 April 2009, Michael J Gruber wrote:\n\n> Search texts may very likely include characters like '@' when grepping\n> for author names. Currently, gitweb produces first/prev/next links with\n> incorrectly escaped characters.\n> \n> Make gitweb escape searchtext and parameters which are reused in href()\n> when replay is set. (cgi params are de-escaped when put into the\n> parameter dictionary and need to be re-escaped when reused.)\n> \n> Signed-off-by: Michael J Gruber \n> ---\n\n> Olaf Hering wrote:\n>\n>> An 'author' search string like \"torvalds@linux\" at\n>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git\n>> generates a 'next' link due to the huge number of commits.\n>>\n>> This link has an incorrect escaping for the @ sign.\n>> The backslash does not work, it generates an error:\n>>\n>> 403 Forbidden - Invalid search parameter\n>>\n>> It should be s=torvalds%40linux instead of s=torvalds\\@linux\n\nIf you by hand edit URL changing '\\@' to simply '@', does changed\ngitweb URL works correctly?\n\n>\n> Maybe something like this? Highly untested!\n\nNo, the problem is not with lack of URI escaping in 's' parameter\n('@' character is not forbidden for query string; it has special\nmeaning and has to be escaped only in the hostname part), but with\npassing value _quoted for Perl_ (quotemeta) to href() subroutine.\n\nI'll try to come with the solution soon.\n\n> gitweb/gitweb.perl | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 3f99361..e1b09f8 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -848,7 +848,7 @@ sub href (%) {\n> \tif ($params{-replay}) {\n> \t\twhile (my ($name, $symbol) = each %cgi_param_mapping) {\n> \t\t\tif (!exists $params{$name}) {\n> -\t\t\t\t$params{$name} = $input_params{$name};\n> +\t\t\t\t$params{$name} = esc_url($input_params{$name});\n> \t\t\t}\n> \t\t}\n> \t}\n> @@ -5775,7 +5775,7 @@ sub git_search {\n> \t\tif ($page > 0) {\n> \t\t\t$paging_nav .=\n> \t\t\t\t$cgi->a({-href => href(action=>\"search\", hash=>$hash,\n> -\t\t\t\t searchtext=>$searchtext,\n> +\t\t\t\t searchtext=>esc_url($searchtext),\n> \t\t\t\t searchtype=>$searchtype)},\n> \t\t\t\t \"first\");\n> \t\t\t$paging_nav .= \" ⋅ \" .\n\nThis would result in double URI escaping, as at the end of href()\nsubroutine we have:\n\n\tpush @result, $symbol . \"=\" . esc_param($params{$name});\n\n [...]\n\n $href .= \"?\" . join(';', @result) if scalar @result;\n\n return $href;\n\nNote that gitweb uses esc_param() and not esc_url() here; the rules are\nslightly different (you need to quote/escape ';', '?', '=' etc. for\nquery parameter).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"112625","messageId":"0626233F-50D5-4F7E-9009-506FEA77571D@aepfle.de","threadId":"19118","inReplyTo":"200904291428.46244.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: escape searchtext and parameters for replay","fromName":"Olaf Hering","fromEmail":"olaf@aepfle.de","sentAt":"2009-04-29T12:52:27Z","receivedAt":"2009-04-29T12:52:27Z","isPatch":true,"sender":{"key":"olaf@aepfle.de","avatar":"https://avatars.githubusercontent.com/u/942324?v=4"},"body":"Am 29.04.2009 um 14:28 schrieb Jakub Narebski:\n\n> On Wed, 29 April 2009, Michael J Gruber wrote:\n\n>>> It should be s=torvalds%40linux instead of s=torvalds\\@linux\n>\n> If you by hand edit URL changing '\\@' to simply '@', does changed\n> gitweb URL works correctly?\n\nI tried akpm@osdl.org, and @ and . was escaped with a backslash.\nRemoving both, and using the plain mail address worked.\n\nOlaf\n"},{"id":"112633","messageId":"200904291514.11331.jnareb@gmail.com","threadId":"19118","inReplyTo":"0626233F-50D5-4F7E-9009-506FEA77571D@aepfle.de","subject":"Re: [PATCH] gitweb: escape searchtext and parameters for replay","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-29T13:14:10Z","receivedAt":"2009-04-29T13:14:10Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 29 Apr 2009, Olaf Hering wrote:\n> Am 29.04.2009 um 14:28 schrieb Jakub Narebski: \n>> On Wed, 29 April 2009, Michael J Gruber wrote:\n> \n>>>> It should be s=torvalds%40linux instead of s=torvalds\\@linux\n>>\n>> If you by hand edit URL changing '\\@' to simply '@', does changed\n>> gitweb URL works correctly?\n> \n> I tried akpm@osdl.org, and @ and . was escaped with a backslash.\n> Removing both, and using the plain mail address worked.\n\nThis problem was fixed in 7e431ef (gitweb: Separate search regexp from \nsearch text), but this fix is present in git 1.5.2 and later, and \nkernel.org gitweb is a fork (adding caching mechanism) of 1.4.5-rc0,\naccording to info in git.kernel.org HTML source, and it does not have \nthis fix.\n\n http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary\n\nI have CC-ed J.H. on this.\n-- \nJakub Narebski\nPoland\n"},{"id":"112674","messageId":"200904292136.36987.jnareb@gmail.com","threadId":"19118","inReplyTo":"200904291514.11331.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: escape searchtext and parameters for replay","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-29T19:36:35Z","receivedAt":"2009-04-29T19:36:35Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 29 Apr 2009, Jakub Narebski wrote:\n> On Wed, 29 Apr 2009, Olaf Hering wrote:\n>> Am 29.04.2009 um 14:28 schrieb Jakub Narebski: \n>>> On Wed, 29 April 2009, Michael J Gruber wrote:\n>>>>> \n>>>>> It should be s=torvalds%40linux instead of s=torvalds\\@linux\n>>>\n>>> If you by hand edit URL changing '\\@' to simply '@', does changed\n>>> gitweb URL works correctly?\n>> \n>> I tried akpm@osdl.org, and @ and . was escaped with a backslash.\n>> Removing both, and using the plain mail address worked.\n> \n> This problem was fixed in 7e431ef (gitweb: Separate search regexp from \n> search text), but this fix is present in git 1.5.2 and later,\n\nI have just checked that current gitweb (1.6.2.rc1.20.g8c5b)\ndoes not have this bug...\n\n> and kernel.org gitweb is a fork (adding caching mechanism) of\n> 1.4.5-rc0 (according to info in git.kernel.org HTML source), and\n> it does not have this fix.\n> \n> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary\n> \n> I have CC-ed J.H. on this.\n\n... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org\nshould backport this fix to kernel.org's fork of gitweb sources.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"112680","messageId":"49F8B36B.6040505@drmicha.warpmail.net","threadId":"19118","inReplyTo":"200904292136.36987.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: escape searchtext and parameters for replay","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-29T20:07:07Z","receivedAt":"2009-04-29T20:07:07Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jakub Narebski venit, vidit, dixit 29.04.2009 21:36:\n> On Wed, 29 Apr 2009, Jakub Narebski wrote:\n>> On Wed, 29 Apr 2009, Olaf Hering wrote:\n>>> Am 29.04.2009 um 14:28 schrieb Jakub Narebski: \n>>>> On Wed, 29 April 2009, Michael J Gruber wrote:\n>>>>>>\n>>>>>> It should be s=torvalds%40linux instead of s=torvalds\\@linux\n>>>>\n>>>> If you by hand edit URL changing '\\@' to simply '@', does changed\n>>>> gitweb URL works correctly?\n>>>\n>>> I tried akpm@osdl.org, and @ and . was escaped with a backslash.\n>>> Removing both, and using the plain mail address worked.\n>>\n>> This problem was fixed in 7e431ef (gitweb: Separate search regexp from \n>> search text), but this fix is present in git 1.5.2 and later,\n> \n> I have just checked that current gitweb (1.6.2.rc1.20.g8c5b)\n> does not have this bug...\n\nYes, I should have tested before suggesting a patch blindly. Sorry!\n\n> \n>> and kernel.org gitweb is a fork (adding caching mechanism) of\n>> 1.4.5-rc0 (according to info in git.kernel.org HTML source), and\n>> it does not have this fix.\n>>\n>> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary\n>>\n>> I have CC-ed J.H. on this.\n> \n> ... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org\n> should backport this fix to kernel.org's fork of gitweb sources.\n> \n"},{"id":"112721","messageId":"49F8DEB4.40909@eaglescrag.net","threadId":"19118","inReplyTo":"200904292136.36987.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: escape searchtext and parameters for replay","fromName":"J.H.","fromEmail":"warthog19@eaglescrag.net","sentAt":"2009-04-29T23:11:48Z","receivedAt":"2009-04-29T23:11:48Z","isPatch":true,"sender":{"key":"warthog19@eaglescrag.net","avatar":null},"body":"Jakub Narebski wrote:\n> On Wed, 29 Apr 2009, Jakub Narebski wrote:\n>> On Wed, 29 Apr 2009, Olaf Hering wrote:\n>>> Am 29.04.2009 um 14:28 schrieb Jakub Narebski: \n>>>> On Wed, 29 April 2009, Michael J Gruber wrote:\n>>>>>> It should be s=torvalds%40linux instead of s=torvalds\\@linux\n>>>> If you by hand edit URL changing '\\@' to simply '@', does changed\n>>>> gitweb URL works correctly?\n>>> I tried akpm@osdl.org, and @ and . was escaped with a backslash.\n>>> Removing both, and using the plain mail address worked.\n>> This problem was fixed in 7e431ef (gitweb: Separate search regexp from \n>> search text), but this fix is present in git 1.5.2 and later,\n> \n> I have just checked that current gitweb (1.6.2.rc1.20.g8c5b)\n> does not have this bug...\n> \n>> and kernel.org gitweb is a fork (adding caching mechanism) of\n>> 1.4.5-rc0 (according to info in git.kernel.org HTML source), and\n>> it does not have this fix.\n>>\n>> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=summary\n>>\n>> I have CC-ed J.H. on this.\n> \n> ... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org\n> should backport this fix to kernel.org's fork of gitweb sources.\n\nPatches, as always, are welcome - I've got a few queued up already, but \nif I have to do it I won't be getting back around to it for a bit (there \nare other things on my priority list right now).\n\n- John \"Warthog9\" Hawley\n"},{"id":"112757","messageId":"200904301217.27154.jnareb@gmail.com","threadId":"19118","inReplyTo":"49F8DEB4.40909@eaglescrag.net","subject":"[PATCH (backport for warthog9/gitweb.git)] gitweb: Separate search regexp from search text","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-30T10:17:25Z","receivedAt":"2009-04-30T10:17:25Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Separate search text, which is saved in $searchtext global variable,\nand is used in links, as default value for the textfield in search\nform, and for pickaxe search, from search regexp, which is saved in\n$search_regexp global variable, and is used as parameter to --grep,\n--committer or --author options to git-rev-list, and for searching\ncommit body in gitweb. For now $search_regexp is unconditionally\nequal to quotemeta($searchtext), meaning that we always search for\nfixed string.\n\nThis fixes bug where 'next page' links for 'search' view didn't work\nfor searchtext containing quotable characters, like `@'.\n\n(manually cherry picked from 7e431ef9ab933d7ff899a999e40627ab49774f3a)\n\nOlaf Hering noticed that this bug was still present, and this bugfix\nwas not ported to kernel.org's fork of gitweb.\n\nSigned-off-by: Jakub Narebski \n---\nOn Thu, 30 Apr 2009, J.H. wrote:\n> Jakub Narebski wrote:\n \n> > I have just checked that current gitweb (1.6.2.rc1.20.g8c5b)\n> > does not have this bug...\n[...]\n> > ... so J.H., or Lea Wiemann, or whoever manages gitweb on kernel.org\n> > should backport this fix to kernel.org's fork of gitweb sources.\n> \n> Patches, as always, are welcome - I've got a few queued up already, but \n> if I have to do it I won't be getting back around to it for a bit (there \n> are other things on my priority list right now).\n\nHere you have backport of 7e431ef (gitweb: Separate search regexp from \nsearch text) on top of 'master' branch of kernel.org's fork of gitweb:\n git://git.kernel.org/pub/scm/git/warthog9/gitweb.git\n\nThe only difference is that git_search_grep_body is in separate\ngitweb/gitweb/search.pm file in kernel.org's fork. The change\nis otherwise identical.\n\nNot tested!\n\n gitweb/gitweb.perl | 5 +++--\n gitweb/gitweb/search.pm | 2 +-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 984161c..aee9239 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -231,6 +231,7 @@ if (defined $page) {\n }\n \n our $searchtext = $cgi->param('s');\n+our $search_regexp;\n if (defined $searchtext) {\n \tif ($searchtext =~ m/[^a-zA-Z0-9_\\.\\/\\-\\+\\:\\@ ]/) {\n \t\tdie_error(undef, \"Invalid search parameter\");\n@@ -238,7 +239,7 @@ if (defined $searchtext) {\n \tif (length($searchtext) < 2) {\n \t\tdie_error(undef, \"At least two characters are required for search parameter\");\n \t}\n-\t$searchtext = quotemeta $searchtext;\n+\t$search_regexp = quotemeta $searchtext;\n }\n \n our $searchtype = $cgi->param('st');\n@@ -1702,7 +1703,7 @@ sub git_search {\n \t\t} elsif ($searchtype eq 'committer') {\n \t\t\t$greptype = \"--committer=\";\n \t\t}\n-\t\t$greptype .= $searchtext;\n+\t\t$greptype .= $search_regexp;\n \t\tmy @commitlist = parse_commits($hash, 101, (100 * $page), $greptype);\n \n \t\tmy $paging_nav = '';\ndiff --git a/gitweb/gitweb/search.pm b/gitweb/gitweb/search.pm\nindex f1aea0a..a381f4e 100644\n--- a/gitweb/gitweb/search.pm\n+++ b/gitweb/gitweb/search.pm\n@@ -33,7 +33,7 @@ sub git_search_grep_body {\n \t\t\t\tesc_html(chop_str($co{'title'}, 50)) . \"
\");\n \t\tmy $comment = $co{'comment'};\n \t\tforeach my $line (@$comment) {\n-\t\t\tif ($line =~ m/^(.*)($searchtext)(.*)$/i) {\n+\t\t\tif ($line =~ m/^(.*)($search_regexp)(.*)$/i) {\n \t\t\t\tmy $lead = esc_html($1) || \"\";\n \t\t\t\t$lead = chop_str($lead, 30, 10);\n \t\t\t\tmy $match = esc_html($2) || \"\";\n-- \n1.6.2\n"}]}