{"thread":{"id":"29781","subject":"[PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search","startedAt":"2012-02-28T18:41:47Z","lastAt":"2012-03-06T12:40:19Z","messageCount":18,"participants":["Jakub Narebski","Junio C Hamano","Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"185647","messageId":"20120228183919.26435.86795.stgit@localhost.localdomain","threadId":"29781","inReplyTo":null,"subject":"[PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-28T18:41:47Z","receivedAt":"2012-02-28T18:41:47Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"When using regexp search ('sr' parameter / $search_use_regexp variable\nis true), check first that regexp is valid.\n\nWithout this patch we would get an error from Perl during search (if\nsearching is performed by gitweb), or highlighting matches substring\n(if applicable), if user provided invalid regexp... which means broken\nHTML, with error page (including HTTP headers) generated after gitweb\nalready produced some output.\n\nAdd test that illustrates such error: for example for regexp \"*\\.git\"\nwe would get the following error:\n\n Quantifier follows nothing in regex; marked by <-- HERE in m/* <-- HERE \\.git/\n at /var/www/cgi-bin/gitweb.cgi line 3084.\n\nReported-by: Ramsay Jones \nSigned-off-by: Jakub Narebski \n---\nSee \"Re: gitweb: (potential) problems with new installation\"\nhttp://thread.gmane.org/gmane.comp.version-control.git/191746\n\n gitweb/gitweb.perl | 11 ++++++++++-\n t/t9501-gitweb-standalone-http-status.sh | 10 ++++++++++\n 2 files changed, 20 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1fc5361..22ad279 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1081,7 +1081,16 @@ sub evaluate_and_validate_params {\n \t\tif (length($searchtext) < 2) {\n \t\t\tdie_error(403, \"At least two characters are required for search parameter\");\n \t\t}\n-\t\t$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;\n+\t\tif ($search_use_regexp) {\n+\t\t\t$search_regexp = $searchtext;\n+\t\t\tif (!eval { qr/$search_regexp/; 1; }) {\n+\t\t\t\t(my $error = $@) =~ s/ at \\S+ line \\d+.*\\n?//;\n+\t\t\t\tdie_error(400, \"Invalid search regexp '$search_regexp'\",\n+\t\t\t\t esc_html($error));\n+\t\t\t}\n+\t\t} else {\n+\t\t\t$search_regexp = quotemeta $searchtext;\n+\t\t}\n \t}\n }\n \ndiff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh\nindex 26102ee..31076ed 100755\n--- a/t/t9501-gitweb-standalone-http-status.sh\n+++ b/t/t9501-gitweb-standalone-http-status.sh\n@@ -134,4 +134,14 @@ our $maxload = undef;\n EOF\n \n \n+# ----------------------------------------------------------------------\n+# invalid arguments\n+\n+test_expect_success 'invalid arguments: invalid regexp (in project search)' '\n+\tgitweb_run \"a=project_list;s=*\\.git;sr=1\" &&\n+\tgrep \"Status: 400\" gitweb.headers &&\n+\tgrep \"400 - Invalid.*regexp\" gitweb.body\n+'\n+test_debug 'cat gitweb.headers'\n+\n test_done\n"},{"id":"185665","messageId":"7vwr76yc6w.fsf@alter.siamese.dyndns.org","threadId":"29781","inReplyTo":"20120228183919.26435.86795.stgit@localhost.localdomain","subject":"Re: [PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-28T19:45:11Z","receivedAt":"2012-02-28T19:45:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> When using regexp search ('sr' parameter / $search_use_regexp variable\n> is true), check first that regexp is valid.\n\nThanks.\n\nHow old is this bug? Should it go to older maitenance tracks like 1.7.6?\n"},{"id":"185723","messageId":"201202291656.36222.jnareb@gmail.com","threadId":"29781","inReplyTo":"7vwr76yc6w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-29T15:56:35Z","receivedAt":"2012-02-29T15:56:35Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> > When using regexp search ('sr' parameter / $search_use_regexp variable\n> > is true), check first that regexp is valid.\n> \n> Thanks.\n> \n> How old is this bug? Should it go to older maitenance tracks like 1.7.6?\n\n>From what I examined this bug is from the very beginning when gitweb\nstarted to distinguish regexp search and fixed string search in\n\n 0e55991 (gitweb: Clearly distinguish regexp / exact match searches, 2008-02-26)\n\nIt was present in 1.5.5 (including beginnings of match highlighting, which\ntrigger this bug).\n\n\nThis bug was present so long without detection because circumstances must\nbe quite specific: you have to select regexp search, and to provide invalid\nregexp. If you know what regexp is, you probably write correct ones...\nbut there always room for mistake.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"185945","messageId":"4F512327.3050504@ramsay1.demon.co.uk","threadId":"29781","inReplyTo":"20120228183919.26435.86795.stgit@localhost.localdomain","subject":"Re: [PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-03-02T19:44:39Z","receivedAt":"2012-03-02T19:44:39Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jakub Narebski wrote:\n> When using regexp search ('sr' parameter / $search_use_regexp variable\n> is true), check first that regexp is valid.\n> \n> Without this patch we would get an error from Perl during search (if\n> searching is performed by gitweb), or highlighting matches substring\n> (if applicable), if user provided invalid regexp... which means broken\n> HTML, with error page (including HTTP headers) generated after gitweb\n> already produced some output.\n> \n> Add test that illustrates such error: for example for regexp \"*\\.git\"\n> we would get the following error:\n> \n> Quantifier follows nothing in regex; marked by <-- HERE in m/* <-- HERE \\.git/\n> at /var/www/cgi-bin/gitweb.cgi line 3084.\n> \n> Reported-by: Ramsay Jones \n> Signed-off-by: Jakub Narebski \n> ---\n> See \"Re: gitweb: (potential) problems with new installation\"\n> http://thread.gmane.org/gmane.comp.version-control.git/191746\n\nThis patch solves the problem for me when using a regex search\n(re checkbox checked), but *not* for a non-regex search.\n\nIf you have a leading '*' or '+', in the non-regex case, then you\nstill get the above complaint (and xml error page etc.), although\nthe line number has changed slightly from that given above.\n\nATB,\nRamsay Jones\n"},{"id":"185948","messageId":"201203022334.25544.jnareb@gmail.com","threadId":"29781","inReplyTo":"4F512327.3050504@ramsay1.demon.co.uk","subject":"[PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-02T22:34:24Z","receivedAt":"2012-03-02T22:34:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use $search_regexp, where regex metacharacters are quoted, for\nsearching projects list, rather than $searchtext, which contains\noriginal search term.\n\nReported-by: Ramsay Jones \nSigned-off-by: Jakub Narebski \n---\nI think this bug was here from the very beginning of adding project\nsearch, i.e. from v1.6.0.2-446-g0d1d154 (gitweb: Support for simple\nproject search form, 2008-10-03) which was present since 1.6.1\n\nOn Fri, 2 Mar 2012, Ramsay Jones wrote:\n> Jakub Narebski wrote:\n\n> > When using regexp search ('sr' parameter / $search_use_regexp variable\n> > is true), check first that regexp is valid.\n> > \n> > Without this patch we would get an error from Perl during search (if\n> > searching is performed by gitweb), or highlighting matches substring\n> > (if applicable), if user provided invalid regexp... which means broken\n> > HTML, with error page (including HTTP headers) generated after gitweb\n> > already produced some output.\n> > \n> > Add test that illustrates such error: for example for regexp \"*\\.git\"\n> > we would get the following error:\n> > \n> > Quantifier follows nothing in regex; marked by <-- HERE in m/* <-- HERE \\.git/\n> > at /var/www/cgi-bin/gitweb.cgi line 3084.\n> > \n> > Reported-by: Ramsay Jones \n> > Signed-off-by: Jakub Narebski \n> > ---\n> > See \"Re: gitweb: (potential) problems with new installation\"\n> > http://thread.gmane.org/gmane.comp.version-control.git/191746\n> \n> This patch solves the problem for me when using a regex search\n> (re checkbox checked), but *not* for a non-regex search.\n> \n> If you have a leading '*' or '+', in the non-regex case, then you\n> still get the above complaint (and xml error page etc.), although\n> the line number has changed slightly from that given above.\n\nRamsay, please provide those line number in the future, together with\nline and if possible some context.\n\nThe line is different because it is different bug: this is about not\nusing quotemeta'ed string for search for fixed-string search.\n\n gitweb/gitweb.perl | 22 +++++++++++-----------\n 1 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 22ad279..7398be1 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3072,16 +3072,16 @@ sub filter_forks_from_projects_list {\n # for 'descr_long' and 'ctags' to be filled\n sub search_projects_list {\n \tmy ($projlist, %opts) = @_;\n-\tmy $tagfilter = $opts{'tagfilter'};\n-\tmy $searchtext = $opts{'searchtext'};\n+\tmy $tagfilter = $opts{'tagfilter'};\n+\tmy $search_re = $opts{'search_regexp'};\n \n \treturn @$projlist\n-\t\tunless ($tagfilter || $searchtext);\n+\t\tunless ($tagfilter || $search_re);\n \n \t# searching projects require filling to be run before it;\n \tfill_project_list_info($projlist,\n-\t $tagfilter ? 'ctags' : (),\n-\t $searchtext ? ('path', 'descr') : ());\n+\t $tagfilter ? 'ctags' : (),\n+\t $search_re ? ('path', 'descr') : ());\n \tmy @projects;\n PROJECT:\n \tforeach my $pr (@$projlist) {\n@@ -3092,10 +3092,10 @@ sub search_projects_list {\n \t\t\t\tgrep { lc($_) eq lc($tagfilter) } keys %{$pr->{'ctags'}};\n \t\t}\n \n-\t\tif ($searchtext) {\n+\t\tif ($search_re) {\n \t\t\tnext unless\n-\t\t\t\t$pr->{'path'} =~ /$searchtext/ ||\n-\t\t\t\t$pr->{'descr_long'} =~ /$searchtext/;\n+\t\t\t\t$pr->{'path'} =~ /$search_re/ ||\n+\t\t\t\t$pr->{'descr_long'} =~ /$search_re/;\n \t\t}\n \n \t\tpush @projects, $pr;\n@@ -5498,9 +5498,9 @@ sub git_project_list_body {\n \t\tif ($check_forks);\n \t# search_projects_list pre-fills required info\n \t@projects = search_projects_list(\\@projects,\n-\t 'searchtext' => $searchtext,\n-\t 'tagfilter' => $tagfilter)\n-\t\tif ($tagfilter || $searchtext);\n+\t 'search_regexp' => $search_regexp,\n+\t 'tagfilter' => $tagfilter)\n+\t\tif ($tagfilter || $search_regexp);\n \t# fill the rest\n \t@projects = fill_project_list_info(\\@projects);\n \n-- \n1.7.9\n"},{"id":"185954","messageId":"7vwr72a6m6.fsf@alter.siamese.dyndns.org","threadId":"29781","inReplyTo":"201203022334.25544.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-03T00:08:33Z","receivedAt":"2012-03-03T00:08:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Use $search_regexp, where regex metacharacters are quoted, for\n> searching projects list, rather than $searchtext, which contains\n> original search term.\n>\n> Reported-by: Ramsay Jones \n> Signed-off-by: Jakub Narebski \n> ---\n> I think this bug was here from the very beginning of adding project\n> search, i.e. from v1.6.0.2-446-g0d1d154 (gitweb: Support for simple\n> project search form, 2008-10-03) which was present since 1.6.1\n>\n> On Fri, 2 Mar 2012, Ramsay Jones wrote:\n> \n>> This patch solves the problem for me when using a regex search\n>> (re checkbox checked), but *not* for a non-regex search.\n>> \n\nThis patch depends on the more recent changes than the regexp fix, no? I\nwas hoping that we could merge the earlier fix for the regexp case to\nolder maintenance tracks later, but if we were going to do so, we would\nwant to do the same for a fix for fixed-string case.\n\nI am fine with not to worrying too much about older maintenance tracks,\nand applying this directly to 'master', but just wanted to see what your\npreference is.\n\nThanks.\n\n>> If you have a leading '*' or '+', in the non-regex case, then you\n>> still get the above complaint (and xml error page etc.), although\n>> the line number has changed slightly from that given above.\n>\n> Ramsay, please provide those line number in the future, together with\n> line and if possible some context.\n>\n> The line is different because it is different bug: this is about not\n> using quotemeta'ed string for search for fixed-string search.\n>\n> gitweb/gitweb.perl | 22 +++++++++++-----------\n> 1 files changed, 11 insertions(+), 11 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 22ad279..7398be1 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3072,16 +3072,16 @@ sub filter_forks_from_projects_list {\n> # for 'descr_long' and 'ctags' to be filled\n> sub search_projects_list {\n> \tmy ($projlist, %opts) = @_;\n> -\tmy $tagfilter = $opts{'tagfilter'};\n> -\tmy $searchtext = $opts{'searchtext'};\n> +\tmy $tagfilter = $opts{'tagfilter'};\n> +\tmy $search_re = $opts{'search_regexp'};\n> \n> \treturn @$projlist\n> -\t\tunless ($tagfilter || $searchtext);\n> +\t\tunless ($tagfilter || $search_re);\n> \n> \t# searching projects require filling to be run before it;\n> \tfill_project_list_info($projlist,\n> -\t $tagfilter ? 'ctags' : (),\n> -\t $searchtext ? ('path', 'descr') : ());\n> +\t $tagfilter ? 'ctags' : (),\n> +\t $search_re ? ('path', 'descr') : ());\n> \tmy @projects;\n> PROJECT:\n> \tforeach my $pr (@$projlist) {\n> @@ -3092,10 +3092,10 @@ sub search_projects_list {\n> \t\t\t\tgrep { lc($_) eq lc($tagfilter) } keys %{$pr->{'ctags'}};\n> \t\t}\n> \n> -\t\tif ($searchtext) {\n> +\t\tif ($search_re) {\n> \t\t\tnext unless\n> -\t\t\t\t$pr->{'path'} =~ /$searchtext/ ||\n> -\t\t\t\t$pr->{'descr_long'} =~ /$searchtext/;\n> +\t\t\t\t$pr->{'path'} =~ /$search_re/ ||\n> +\t\t\t\t$pr->{'descr_long'} =~ /$search_re/;\n> \t\t}\n> \n> \t\tpush @projects, $pr;\n> @@ -5498,9 +5498,9 @@ sub git_project_list_body {\n> \t\tif ($check_forks);\n> \t# search_projects_list pre-fills required info\n> \t@projects = search_projects_list(\\@projects,\n> -\t 'searchtext' => $searchtext,\n> -\t 'tagfilter' => $tagfilter)\n> -\t\tif ($tagfilter || $searchtext);\n> +\t 'search_regexp' => $search_regexp,\n> +\t 'tagfilter' => $tagfilter)\n> +\t\tif ($tagfilter || $search_regexp);\n> \t# fill the rest\n> \t@projects = fill_project_list_info(\\@projects);\n"},{"id":"185982","messageId":"201203031156.00948.jnareb@gmail.com","threadId":"29781","inReplyTo":"7vwr72a6m6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-03T10:55:59Z","receivedAt":"2012-03-03T10:55:59Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 3 Mar 2012, Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n>> Use $search_regexp, where regex metacharacters are quoted, for\n>> searching projects list, rather than $searchtext, which contains\n>> original search term.\n>>\n>> Reported-by: Ramsay Jones \n>> Signed-off-by: Jakub Narebski \n>> ---\n>> I think this bug was here from the very beginning of adding project\n>> search, i.e. from v1.6.0.2-446-g0d1d154 (gitweb: Support for simple\n>> project search form, 2008-10-03) which was present since 1.6.1\n>>\n>> On Fri, 2 Mar 2012, Ramsay Jones wrote:\n>> \n>>> This patch solves the problem for me when using a regex search\n>>> (re checkbox checked), but *not* for a non-regex search.\n>>> \n> \n> This patch depends on the more recent changes than the regexp fix, no? I\n> was hoping that we could merge the earlier fix for the regexp case to\n> older maintenance tracks later, but if we were going to do so, we would\n> want to do the same for a fix for fixed-string case.\n\nThe regexp and non-regexp bugs and fixes are different.\n\nThe regexp \"bug\" was just us forgetting that regexp is provided by user\ninput, and should be validated. The bug as reported by Ramsay was here\nfrom the very beginning, i.e. commit 0e55991 (gitweb: Clearly distinguish\nregexp / exact match searches, 2008-02-26), which was present in v1.5.1\nif I have checked correctly. The fix is about adding new code and should\napply cleanly to 'maint' and even to older versions; the only trouble\nwith older version might be whitespace issue related to refactoring\ncode into subroutines.\n\nThe non-regexp project search bug was using $searchtext instead of\n$search_regexp as search regexp in gitweb. The bug was present from\nthe very addition of project search, namely commit 0d1d154 (gitweb:\nSupport for simple project search form, 2008-10-03), which was present\nin v1.5.1 if I have checked correctly. Unfortunately the fix affects\ncode that was changed recently in a1e1b2d (gitweb: improve usability\nof projects search form, 2012-01-31); I'll try to come up with equivalent\npatch to 'maint' soon (if the current one does not apply, and I guess it\ndoesn't).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"186025","messageId":"201203041035.03133.jnareb@gmail.com","threadId":"29781","inReplyTo":"201203031156.00948.jnareb@gmail.com","subject":"[PATCH (for maint)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-04T09:35:02Z","receivedAt":"2012-03-04T09:35:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 3 Mar 2012, Jakub Narebski wrote:\n> On Sat, 3 Mar 2012, Junio C Hamano wrote:\n>> Jakub Narebski writes:\n>> \n>>> Use $search_regexp, where regex metacharacters are quoted, for\n>>> searching projects list, rather than $searchtext, which contains\n>>> original search term.\n>>>\n>>> Reported-by: Ramsay Jones \n>>> Signed-off-by: Jakub Narebski \n>>> ---\n>>> I think this bug was here from the very beginning of adding project\n>>> search, i.e. from v1.6.0.2-446-g0d1d154 (gitweb: Support for simple\n>>> project search form, 2008-10-03) which was present since 1.6.1\n>>>\n>>> On Fri, 2 Mar 2012, Ramsay Jones wrote:\n>>> \n>>>> This patch solves the problem for me when using a regex search\n>>>> (re checkbox checked), but *not* for a non-regex search.\n>>>> \n>> \n>> This patch depends on the more recent changes than the regexp fix, no? I\n>> was hoping that we could merge the earlier fix for the regexp case to\n>> older maintenance tracks later, but if we were going to do so, we would\n>> want to do the same for a fix for fixed-string case.\n> \n> The regexp and non-regexp bugs and fixes are different.\n[...]\n> The non-regexp project search bug was using $searchtext instead of\n> $search_regexp as search regexp in gitweb. The bug was present from\n> the very addition of project search, namely commit 0d1d154 (gitweb:\n> Support for simple project search form, 2008-10-03), which was present\n> in v1.5.1 if I have checked correctly. Unfortunately the fix affects\n> code that was changed recently in a1e1b2d (gitweb: improve usability\n> of projects search form, 2012-01-31); I'll try to come up with equivalent\n> patch to 'maint' soon (if the current one does not apply, and I guess it\n> doesn't).\n\nAnd here is the patch for maint\n-->8-- -------------------------------------------------------- -->8--\nSubject: gitweb: Fix fixed string (non-regexp) project search\n\nUse $search_regexp, where regex metacharacters are quoted, for\nsearching projects list, rather than $searchtext, which contains\noriginal search term.\n\nReported-by: Ramsay Jones \nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 20 +++++++++++---------\n 1 files changed, 11 insertions(+), 9 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex d5dbd64..e248792 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2968,11 +2968,11 @@ sub filter_forks_from_projects_list {\n # for 'descr_long' and 'ctags' to be filled\n sub search_projects_list {\n \tmy ($projlist, %opts) = @_;\n-\tmy $tagfilter = $opts{'tagfilter'};\n-\tmy $searchtext = $opts{'searchtext'};\n+\tmy $tagfilter = $opts{'tagfilter'};\n+\tmy $search_re = $opts{'search_regexp'};\n \n \treturn @$projlist\n-\t\tunless ($tagfilter || $searchtext);\n+\t\tunless ($tagfilter || $search_re);\n \n \tmy @projects;\n PROJECT:\n@@ -2984,10 +2984,10 @@ sub search_projects_list {\n \t\t\t\tgrep { lc($_) eq lc($tagfilter) } keys %{$pr->{'ctags'}};\n \t\t}\n \n-\t\tif ($searchtext) {\n+\t\tif ($search_re) {\n \t\t\tnext unless\n-\t\t\t\t$pr->{'path'} =~ /$searchtext/ ||\n-\t\t\t\t$pr->{'descr_long'} =~ /$searchtext/;\n+\t\t\t\t$pr->{'path'} =~ /$search_re/ ||\n+\t\t\t\t$pr->{'descr_long'} =~ /$search_re/;\n \t\t}\n \n \t\tpush @projects, $pr;\n@@ -5290,9 +5290,11 @@ sub git_project_list_body {\n \t@projects = fill_project_list_info(\\@projects);\n \t# searching projects require filling to be run before it\n \t@projects = search_projects_list(\\@projects,\n-\t 'searchtext' => $searchtext,\n-\t 'tagfilter' => $tagfilter)\n-\t\tif ($tagfilter || $searchtext);\n+\t 'search_regexp' => $search_regexp,\n+\t 'tagfilter' => $tagfilter)\n+\t\tif ($tagfilter || $search_regexp);\n+\t# fill the rest\n+\t@projects = fill_project_list_info(\\@projects);\n \n \t$order ||= $default_projects_order;\n \t$from = 0 unless defined $from;\n-- \n1.7.9\n"},{"id":"186043","messageId":"201203041900.32114.jnareb@gmail.com","threadId":"29781","inReplyTo":"201203031156.00948.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-04T18:00:31Z","receivedAt":"2012-03-04T18:00:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n> On Sat, 3 Mar 2012, Junio C Hamano wrote:\n>> Jakub Narebski writes:\n>> \n>>> Use $search_regexp, where regex metacharacters are quoted, for\n>>> searching projects list, rather than $searchtext, which contains\n>>> original search term.\n>>>\n>>> Reported-by: Ramsay Jones \n>>> Signed-off-by: Jakub Narebski \n>>> ---\n>>> I think this bug was here from the very beginning of adding project\n>>> search, i.e. from v1.6.0.2-446-g0d1d154 (gitweb: Support for simple\n>>> project search form, 2008-10-03) which was present since 1.6.1\n>>>\n>>> On Fri, 2 Mar 2012, Ramsay Jones wrote:\n>>> \n>>>> This patch solves the problem for me when using a regex search\n>>>> (re checkbox checked), but *not* for a non-regex search.\n>>>> \n>> \n>> This patch depends on the more recent changes than the regexp fix, no? I\n>> was hoping that we could merge the earlier fix for the regexp case to\n>> older maintenance tracks later, but if we were going to do so, we would\n>> want to do the same for a fix for fixed-string case.\n> \n> The regexp and non-regexp bugs and fixes are different.\n> \n> The regexp \"bug\" was just us forgetting that regexp is provided by user\n> input, and should be validated. The bug as reported by Ramsay was here\n> from the very beginning, i.e. commit 0e55991 (gitweb: Clearly distinguish\n> regexp / exact match searches, 2008-02-26), which was present in v1.5.1\n> if I have checked correctly. The fix is about adding new code and should\n> apply cleanly to 'maint' and even to older versions; the only trouble\n> with older version might be whitespace issue related to refactoring\n> code into subroutines.\n> \n> The non-regexp project search bug was using $searchtext instead of\n> $search_regexp as search regexp in gitweb. The bug was present from\n> the very addition of project search, namely commit 0d1d154 (gitweb:\n> Support for simple project search form, 2008-10-03), which was present\n> in v1.5.1 if I have checked correctly. Unfortunately the fix affects\n> code that was changed recently in a1e1b2d (gitweb: improve usability\n> of projects search form, 2012-01-31); I'll try to come up with equivalent\n> patch to 'maint' soon (if the current one does not apply, and I guess it\n> doesn't).\n\nIn other words: while \"*foo\" is invalid regular expression, it is\nperfectly valid fixed string search term (which translates to \"\\*foo\"\nregexp).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"186055","messageId":"7vaa3w55i3.fsf@alter.siamese.dyndns.org","threadId":"29781","inReplyTo":"201203031156.00948.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-04T23:08:20Z","receivedAt":"2012-03-04T23:08:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> .... The fix is about adding new code and should\n> apply cleanly to 'maint' and even to older versions; the only trouble\n> with older version might be whitespace issue related to refactoring\n> code into subroutines.\n\nOK, so the global $searchtext is what came from form submit from the end\nuser, while the global $search_regexp is what the code should be using\nfor matching throughout the program, prepared by eval-and-validate-params.\n\nHere is a hand-ported version of your patch that should apply to 1.7.6.6;\ndoes it look sane?\n\n gitweb/gitweb.perl | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 50a835a..d1698b7 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2905,10 +2905,10 @@ sub filter_forks_from_projects_list {\n sub search_projects_list {\n \tmy ($projlist, %opts) = @_;\n \tmy $tagfilter = $opts{'tagfilter'};\n-\tmy $searchtext = $opts{'searchtext'};\n+\tmy $search_re = $opts{'search_regexp'};\n \n \treturn @$projlist\n-\t\tunless ($tagfilter || $searchtext);\n+\t\tunless ($tagfilter || $search_re);\n \n \tmy @projects;\n PROJECT:\n@@ -2920,10 +2920,10 @@ sub search_projects_list {\n \t\t\t\tgrep { lc($_) eq lc($tagfilter) } keys %{$pr->{'ctags'}};\n \t\t}\n \n-\t\tif ($searchtext) {\n+\t\tif ($search_re) {\n \t\t\tnext unless\n-\t\t\t\t$pr->{'path'} =~ /$searchtext/ ||\n-\t\t\t\t$pr->{'descr_long'} =~ /$searchtext/;\n+\t\t\t\t$pr->{'path'} =~ /$search_re/ ||\n+\t\t\t\t$pr->{'descr_long'} =~ /$search_re/;\n \t\t}\n \n \t\tpush @projects, $pr;\n@@ -5097,7 +5097,7 @@ sub git_project_list_body {\n \t@projects = fill_project_list_info(\\@projects);\n \t# searching projects require filling to be run before it\n \t@projects = search_projects_list(\\@projects,\n-\t 'searchtext' => $searchtext,\n+\t 'search_regexp' => $search_regexp,\n \t 'tagfilter' => $tagfilter)\n \t\tif ($tagfilter || $searchtext);\n \n"},{"id":"186082","messageId":"7vfwdn631i.fsf@alter.siamese.dyndns.org","threadId":"29781","inReplyTo":"201203041035.03133.jnareb@gmail.com","subject":"Re: [PATCH (for maint)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T05:16:09Z","receivedAt":"2012-03-05T05:16:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> And here is the patch for maint\n> -->8-- -------------------------------------------------------- -->8--\n> Subject: gitweb: Fix fixed string (non-regexp) project search\n>\n> Use $search_regexp, where regex metacharacters are quoted, for\n> searching projects list, rather than $searchtext, which contains\n> original search term.\n>\n> Reported-by: Ramsay Jones \n> Signed-off-by: Jakub Narebski \n> ---\n> gitweb/gitweb.perl | 20 +++++++++++---------\n> 1 files changed, 11 insertions(+), 9 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index d5dbd64..e248792 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -5290,9 +5290,11 @@ sub git_project_list_body {\n> \t@projects = fill_project_list_info(\\@projects);\n> \t# searching projects require filling to be run before it\n> \t@projects = search_projects_list(\\@projects,\n> -\t 'searchtext' => $searchtext,\n> -\t 'tagfilter' => $tagfilter)\n> -\t\tif ($tagfilter || $searchtext);\n> +\t 'search_regexp' => $search_regexp,\n> +\t 'tagfilter' => $tagfilter)\n> +\t\tif ($tagfilter || $search_regexp);\n> +\t# fill the rest\n> +\t@projects = fill_project_list_info(\\@projects);\n\nHmph, didn't you already call fill_project_list_info(\\@projects) before\nsearch_projects_list() already?\n"},{"id":"186091","messageId":"201203050959.47966.jnareb@gmail.com","threadId":"29781","inReplyTo":"7vfwdn631i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH (for maint)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-05T08:59:47Z","receivedAt":"2012-03-05T08:59:47Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> > And here is the patch for maint\n> > -->8-- -------------------------------------------------------- -->8--\n> > Subject: gitweb: Fix fixed string (non-regexp) project search\n> >\n> > Use $search_regexp, where regex metacharacters are quoted, for\n> > searching projects list, rather than $searchtext, which contains\n> > original search term.\n> >\n> > Reported-by: Ramsay Jones \n> > Signed-off-by: Jakub Narebski \n> > ---\n> > gitweb/gitweb.perl | 20 +++++++++++---------\n> > 1 files changed, 11 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index d5dbd64..e248792 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -5290,9 +5290,11 @@ sub git_project_list_body {\n> > \t@projects = fill_project_list_info(\\@projects);\n> > \t# searching projects require filling to be run before it\n> > \t@projects = search_projects_list(\\@projects,\n> > -\t 'searchtext' => $searchtext,\n> > -\t 'tagfilter' => $tagfilter)\n> > -\t\tif ($tagfilter || $searchtext);\n> > +\t 'search_regexp' => $search_regexp,\n> > +\t 'tagfilter' => $tagfilter)\n> > +\t\tif ($tagfilter || $search_regexp);\n> > +\t# fill the rest\n> > +\t@projects = fill_project_list_info(\\@projects);\n> \n> Hmph, didn't you already call fill_project_list_info(\\@projects) before\n> search_projects_list() already?\n\nTrue. Sorry about that. \n\nCan you fix that, or should I resend?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"186092","messageId":"201203051003.10605.jnareb@gmail.com","threadId":"29781","inReplyTo":"7vaa3w55i3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-05T09:03:10Z","receivedAt":"2012-03-05T09:03:10Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> > .... The fix is about adding new code and should\n> > apply cleanly to 'maint' and even to older versions; the only trouble\n> > with older version might be whitespace issue related to refactoring\n> > code into subroutines.\n> \n> OK, so the global $searchtext is what came from form submit from the end\n> user, while the global $search_regexp is what the code should be using\n> for matching throughout the program, prepared by eval-and-validate-params.\n> \n> Here is a hand-ported version of your patch that should apply to 1.7.6.6;\n> does it look sane?\n> \n> gitweb/gitweb.perl | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 50a835a..d1698b7 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2905,10 +2905,10 @@ sub filter_forks_from_projects_list {\n> sub search_projects_list {\n> \tmy ($projlist, %opts) = @_;\n> \tmy $tagfilter = $opts{'tagfilter'};\n> -\tmy $searchtext = $opts{'searchtext'};\n> +\tmy $search_re = $opts{'search_regexp'};\n> \n> \treturn @$projlist\n> -\t\tunless ($tagfilter || $searchtext);\n> +\t\tunless ($tagfilter || $search_re);\n> \n> \tmy @projects;\n> PROJECT:\n> @@ -2920,10 +2920,10 @@ sub search_projects_list {\n> \t\t\t\tgrep { lc($_) eq lc($tagfilter) } keys %{$pr->{'ctags'}};\n> \t\t}\n> \n> -\t\tif ($searchtext) {\n> +\t\tif ($search_re) {\n> \t\t\tnext unless\n> -\t\t\t\t$pr->{'path'} =~ /$searchtext/ ||\n> -\t\t\t\t$pr->{'descr_long'} =~ /$searchtext/;\n> +\t\t\t\t$pr->{'path'} =~ /$search_re/ ||\n> +\t\t\t\t$pr->{'descr_long'} =~ /$search_re/;\n> \t\t}\n> \n> \t\tpush @projects, $pr;\n> @@ -5097,7 +5097,7 @@ sub git_project_list_body {\n> \t@projects = fill_project_list_info(\\@projects);\n> \t# searching projects require filling to be run before it\n> \t@projects = search_projects_list(\\@projects,\n> -\t 'searchtext' => $searchtext,\n> +\t 'search_regexp' => $search_regexp,\n> \t 'tagfilter' => $tagfilter)\n> \t\tif ($tagfilter || $searchtext);\n> \n\nIt looks sane, though \n\n \t@projects = search_projects_list(\\@projects,\n -\t 'searchtext' => $searchtext,\n +\t 'search_regexp' => $search_regexp,\n \t 'tagfilter' => $tagfilter)\n \t\tif ($tagfilter || $searchtext);\n\nshould be better written as\n\n \t@projects = search_projects_list(\\@projects,\n -\t 'searchtext' => $searchtext,\n +\t 'search_regexp' => $search_regexp,\n \t 'tagfilter' => $tagfilter)\n - \t\tif ($tagfilter || $searchtext);\n + \t\tif ($tagfilter || $search_regexp);\n\nIt is functionally the same, because $search_regexp is derived from\n$searchtext, but IMHO it is more clear.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"186123","messageId":"7vd38r2d8y.fsf@alter.siamese.dyndns.org","threadId":"29781","inReplyTo":"201203050959.47966.jnareb@gmail.com","subject":"Re: [PATCH (for maint)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T17:01:33Z","receivedAt":"2012-03-05T17:01:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Junio C Hamano wrote:\n>> Jakub Narebski writes:\n>> \n>> > And here is the patch for maint\n>> ...\n>> > +\t\tif ($tagfilter || $search_regexp);\n>> > +\t# fill the rest\n>> > +\t@projects = fill_project_list_info(\\@projects);\n>> \n>> Hmph, didn't you already call fill_project_list_info(\\@projects) before\n>> search_projects_list() already?\n>\n> True. Sorry about that. \n>\n> Can you fix that, or should I resend?\n\nCould you check the following two diffs?\n\n$ git show debd1c2\n\nThis is jn/maint-do-not-match-with-unsanitized-searchtext that\nshould be merged to maintenance track that lack the lazy filling.\n\nAnd then\n\n$ git show --first-parent d4b52c2\n\nThis is how the above was merged to 'pu' and the conflict resolution\nshould be the same when we merge it to 'master'. As our @projects may\nstill be only sparsely filled when search_projects_list() returns,\nwe do call fill_project_list_info(\\@projects) ourselves with the\nlazy filling codebase.\n\nThere are a few places I noticed that check $searchtext to see if we\nare running a search, and techinically $search_regexp might be a\nmore correct thing to use, but I do not think it matters that much.\n\nThanks.\n"},{"id":"186136","messageId":"4F550EAE.5030006@ramsay1.demon.co.uk","threadId":"29781","inReplyTo":"201203022334.25544.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-03-05T19:06:22Z","receivedAt":"2012-03-05T19:06:22Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jakub Narebski wrote:\n>> This patch solves the problem for me when using a regex search\n>> (re checkbox checked), but *not* for a non-regex search.\n>>\n>> If you have a leading '*' or '+', in the non-regex case, then you\n>> still get the above complaint (and xml error page etc.), although\n>> the line number has changed slightly from that given above.\n> \n> Ramsay, please provide those line number in the future, together with\n> line and if possible some context.\n\nYeah, sorry about that; when I wrote that I didn't have the information\nreadily available (I would have had to shutdown Windows, boot Linux, ...)\nand I was about to go out. So, the choice was to wait about 24hrs to report\nwith full info, or provide the feedback earlier; I chose the latter. ;-)\n\n> gitweb/gitweb.perl | 22 +++++++++++-----------\n> 1 files changed, 11 insertions(+), 11 deletions(-)\n\n[patch snipped]\n\nThis patch works great for me. Thanks!\n\nATB,\nRamsay Jones\n"},{"id":"186156","messageId":"7vhay2y6fd.fsf@alter.siamese.dyndns.org","threadId":"29781","inReplyTo":"7vd38r2d8y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH (for maint)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T23:27:50Z","receivedAt":"2012-03-05T23:27:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jakub Narebski writes:\n>\n>>> Hmph, didn't you already call fill_project_list_info(\\@projects) before\n>>> search_projects_list() already?\n>>\n>> True. Sorry about that. \n>>\n>> Can you fix that, or should I resend?\n>\n> Could you check the following two diffs?\n>\n> $ git show debd1c2\n>\n> This is jn/maint-do-not-match-with-unsanitized-searchtext that\n> should be merged to maintenance track that lack the lazy filling.\n>\n> And then\n>\n> $ git show --first-parent d4b52c2\n>\n> This is how the above was merged to 'pu' and the conflict resolution\n> should be the same when we merge it to 'master'. As our @projects may\n> still be only sparsely filled when search_projects_list() returns,\n> we do call fill_project_list_info(\\@projects) ourselves with the\n> lazy filling codebase.\n\nThe latter is now\n\n$ git show --first-parent 657c6d0\n\non today's 'pu'.\n\nThanks.\n"},{"id":"186203","messageId":"201203061259.16714.jnareb@gmail.com","threadId":"29781","inReplyTo":"7vhay2y6fd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH (for maint)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-06T11:59:15Z","receivedAt":"2012-03-06T11:59:15Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Junio C Hamano writes:\n>> Jakub Narebski writes:\n>>\n>>>> Hmph, didn't you already call fill_project_list_info(\\@projects) before\n>>>> search_projects_list() already?\n>>>\n>>> True. Sorry about that. \n>>>\n>>> Can you fix that, or should I resend?\n>>\n>> Could you check the following two diffs?\n>>\n>> $ git show debd1c2\n>>\n>> This is jn/maint-do-not-match-with-unsanitized-searchtext that\n>> should be merged to maintenance track that lack the lazy filling.\n>>\n>> And then\n>>\n>> $ git show --first-parent d4b52c2\n>>\n>> This is how the above was merged to 'pu' and the conflict resolution\n>> should be the same when we merge it to 'master'. As our @projects may\n>> still be only sparsely filled when search_projects_list() returns,\n>> we do call fill_project_list_info(\\@projects) ourselves with the\n>> lazy filling codebase.\n> \n> The latter is now\n> \n> $ git show --first-parent 657c6d0\n> \n> on today's 'pu'.\n> \n> Thanks.\n\nBoth look all right (the only difference in diff is use of $searchtext\nvs $search_regexp global variable to check if search is on, but for that\npurpose those variables are equivalent).\n\nThanks.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"186205","messageId":"201203061340.19547.jnareb@gmail.com","threadId":"29781","inReplyTo":"4F550EAE.5030006@ramsay1.demon.co.uk","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix fixed string (non-regexp) project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-06T12:40:19Z","receivedAt":"2012-03-06T12:40:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Ramsay Jones wrote:\n> Jakub Narebski wrote:\n\n>>> This patch solves the problem for me when using a regex search\n>>> (re checkbox checked), but *not* for a non-regex search.\n>>>\n>>> If you have a leading '*' or '+', in the non-regex case, then you\n>>> still get the above complaint (and xml error page etc.), although\n>>> the line number has changed slightly from that given above.\n>> \n>> Ramsay, please provide those line number in the future, together with\n>> line and if possible some context.\n> \n> Yeah, sorry about that; when I wrote that I didn't have the information\n> readily available (I would have had to shutdown Windows, boot Linux, ...)\n> and I was about to go out.\n\nYou could have told us _that_...\n\n> So, the choice was to wait about 24hrs to report \n> with full info, or provide the feedback earlier; I chose the latter. ;-)\n\nThat's understandable.\n\n\nThanks again for reporting these issues.\n\n-- \nJakub Narebski\nPoland\n"}]}