{"thread":{"id":"29279","subject":"[BUG] gitweb generates wrong links in grep search results (git_search_files)","startedAt":"2012-01-02T13:29:01Z","lastAt":"2012-01-13T19:18:42Z","messageCount":7,"participants":["Thomas Perl","Junio C Hamano","Jakub Narębski","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"181839","messageId":"CA+uOhx6i-07kW8K0y3Co++2ABD=Lmaq3r4h1hN4YLskAE+hR1Q@mail.gmail.com","threadId":"29279","inReplyTo":null,"subject":"[BUG] gitweb generates wrong links in grep search results (git_search_files)","fromName":"Thomas Perl","fromEmail":"th.perl@gmail.com","sentAt":"2012-01-02T13:29:01Z","receivedAt":"2012-01-02T13:29:01Z","isPatch":false,"sender":{"key":"th.perl@gmail.com","avatar":"https://gravatar.com/avatar/fa809414192358405166d421a442112e8d9ee706e69da3113ca428b3ef84e9eb?d=mp&s=160"},"body":"Hi,\n\nI think I found a bug in gitweb when grep'ing for text in a branch\ndifferent from \"master\". Here's how to reproduce it:\n\n1. Have a project with a master branch and a branch different from master\n2. Start gitweb for that project (e.g. using \"git instaweb\") and open\nit in a web browser\n3. Switch to the non-master branch (e.g.\nhttp://127.0.0.1:1234/?p=.git;a=shortlog;h=refs/heads/mynonmasterbranch)\n4. In the top right search box, select \"grep\" in the combo box and\nenter a text that only appears in the non-master branch\n5. Submit the search by pressing enter, you should be at:\nhttp://127.0.0.1:1234/?p=.git&a=search&h=refs%2Fheads%2Fmynonmasterbranch&st=grep&s=somesearchtext\n\nACTUAL RESULT\nIn that list of results, you should now see some files matching the\nsearch - note that the links for the file names and the line numbers\ngo to e.g. http://127.0.0.1:1234/?p=.git;a=blob;f=somefile.txt for a\nfile \"somefile.txt\". The links therefore go to the master branch,\nwhile the search results refer to the non-master branch.\n\nEXPECTED RESULT\nThe link should (presumably) go to\nhttp://127.0.0.1:1234/?p=.git;a=blob;hb=refs%2Fheads%2Fmynonmasterbranch;f=somefile.txt\nso that when the link is clicked, the right file (somefile.txt in\nmynonmasterbranch) is shown.\n\nI also investigated a bit in where the problem happens, and nailed it\ndown to: gitweb/gitweb.perl, sub git_search_files, line 5871 in commit\n17b4e93d5b849293e6a3659bbc4075ed8a6e97e2 (current master tip of\nhttps://github.com/gitster/git). I haven't looked at the intrinsics of\nthe \"href\" sub, but I believe that it should somehow get the \"h\"\nparameter from the original page and incorporate it into the final\nlink (as \"hb\" parameter?) to the file. The same fix that is applied\nthere then also needs to be applied at line 5891 (same commit, same\nfile).\n\nNo patch, because after several tries, I didn't get it to work, my\nPerl foo might not be up to the task, and I believe that someone more\nfamiliar with gitweb's code base might have an easier time to fix\nthis.\n\nThanks,\nThomas\n"},{"id":"181908","messageId":"7vhb0cqpix.fsf@alter.siamese.dyndns.org","threadId":"29279","inReplyTo":"CA+uOhx6i-07kW8K0y3Co++2ABD=Lmaq3r4h1hN4YLskAE+hR1Q@mail.gmail.com","subject":"Re: [BUG] gitweb generates wrong links in grep search results (git_search_files)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-04T00:28:54Z","receivedAt":"2012-01-04T00:28:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Perl writes:\n\n> I think I found a bug in gitweb when grep'ing for text in a branch\n> different from \"master\". Here's how to reproduce it:\n\nThanks for a detailed report (and thanks for gpodder ;-).\n\nJakub, care to take a look?\n\n>\n> 1. Have a project with a master branch and a branch different from master\n> 2. Start gitweb for that project (e.g. using \"git instaweb\") and open\n> it in a web browser\n> 3. Switch to the non-master branch (e.g.\n> http://127.0.0.1:1234/?p=.git;a=shortlog;h=refs/heads/mynonmasterbranch)\n> 4. In the top right search box, select \"grep\" in the combo box and\n> enter a text that only appears in the non-master branch\n> 5. Submit the search by pressing enter, you should be at:\n> http://127.0.0.1:1234/?p=.git&a=search&h=refs%2Fheads%2Fmynonmasterbranch&st=grep&s=somesearchtext\n>\n> ACTUAL RESULT\n> In that list of results, you should now see some files matching the\n> search - note that the links for the file names and the line numbers\n> go to e.g. http://127.0.0.1:1234/?p=.git;a=blob;f=somefile.txt for a\n> file \"somefile.txt\". The links therefore go to the master branch,\n> while the search results refer to the non-master branch.\n>\n> EXPECTED RESULT\n> The link should (presumably) go to\n> http://127.0.0.1:1234/?p=.git;a=blob;hb=refs%2Fheads%2Fmynonmasterbranch;f=somefile.txt\n> so that when the link is clicked, the right file (somefile.txt in\n> mynonmasterbranch) is shown.\n>\n> I also investigated a bit in where the problem happens, and nailed it\n> down to: gitweb/gitweb.perl, sub git_search_files, line 5871 in commit\n> 17b4e93d5b849293e6a3659bbc4075ed8a6e97e2 (current master tip of\n> https://github.com/gitster/git). I haven't looked at the intrinsics of\n> the \"href\" sub, but I believe that it should somehow get the \"h\"\n> parameter from the original page and incorporate it into the final\n> link (as \"hb\" parameter?) to the file. The same fix that is applied\n> there then also needs to be applied at line 5891 (same commit, same\n> file).\n>\n> No patch, because after several tries, I didn't get it to work, my\n> Perl foo might not be up to the task, and I believe that someone more\n> familiar with gitweb's code base might have an easier time to fix\n> this.\n>\n> Thanks,\n> Thomas\n"},{"id":"181929","messageId":"CANQwDwfnp167Uth5TLbCD6OR-Xe6JD-2vENiJVnipi1YdjnMPQ@mail.gmail.com","threadId":"29279","inReplyTo":"7vhb0cqpix.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] gitweb generates wrong links in grep search results (git_search_files)","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2012-01-04T16:21:25Z","receivedAt":"2012-01-04T16:21:25Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, Jan 4, 2012 at 1:28 AM, Junio C Hamano wrote:\n> Thomas Perl writes:\n>\n>> I think I found a bug in gitweb when grep'ing for text in a branch\n>> different from \"master\". Here's how to reproduce it:\n>\n> Thanks for a detailed report (and thanks for gpodder ;-).\n>\n> Jakub, care to take a look?\n\nI see the bug: it should be 'hash_base' not 'hash' in href()\ncreating link to \"blob\" view in git_search_files().\n\nI'll try to send a fix soon...\n-- \nJakub Narebski\n"},{"id":"181988","messageId":"201201052126.49087.jnareb@gmail.com","threadId":"29279","inReplyTo":"CANQwDwfnp167Uth5TLbCD6OR-Xe6JD-2vENiJVnipi1YdjnMPQ@mail.gmail.com","subject":"[PATCH 1/2] gitweb: Fix file links in \"grep\" search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-01-05T20:26:48Z","receivedAt":"2012-01-05T20:26:48Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"There were two bugs in generating file links (links to \"blob\" view),\none hidden by the other. The correct way of generating file link is\n\n\thref(action=>\"blob\", hash_base=>$co{'id'},\n\t file_name=>$file);\n\nIt was $co{'hash'} (this key does not exist, and therefore this is\nundef), and 'hash' instead of 'hash_base'.\n\nTo have this fix applied in single place, this commit also reduces\ncode duplication by saving file link (which is used for line links) in\n$file_href.\n\nReported-by: Thomas Perl \nSigned-off-by: Jakub Narebski \n---\nOn Wed, 4 Jan 2012, Jakub Narębski wrote:\n> On Wed, Jan 4, 2012 at 1:28 AM, Junio C Hamano wrote:\n>> Thomas Perl writes:\n>>\n>>> I think I found a bug in gitweb when grep'ing for text in a branch\n>>> different from \"master\". Here's how to reproduce it:\n>>\n>> Thanks for a detailed report (and thanks for gpodder ;-).\n>>\n>> Jakub, care to take a look?\n> \n> I see the bug: it should be 'hash_base' not 'hash' in href()\n> creating link to \"blob\" view in git_search_files().\n> \n> I'll try to send a fix soon...\n\nActually there were two errors, one hiding the other...\n\n\nThomas, could you check if this fixes your issue?\n\n gitweb/gitweb.perl | 15 +++++++--------\n 1 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex fc41b07..fa58156 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5852,7 +5852,7 @@ sub git_search_files {\n \tmy $lastfile = '';\n \twhile (my $line = <$fd>) {\n \t\tchomp $line;\n-\t\tmy ($file, $lno, $ltext, $binary);\n+\t\tmy ($file, $file_href, $lno, $ltext, $binary);\n \t\tlast if ($matches++ > 1000);\n \t\tif ($line =~ /^Binary file (.+) matches$/) {\n \t\t\t$file = $1;\n@@ -5867,10 +5867,10 @@ sub git_search_files {\n \t\t\t} else {\n \t\t\t\tprint \"\\n\";\n \t\t\t}\n+\t\t\t$file_href = href(action=>\"blob\", hash_base=>$co{'id'},\n+\t\t\t file_name=>$file);\n \t\t\tprint \"\".\n-\t\t\t\t$cgi->a({-href => href(action=>\"blob\", hash=>$co{'hash'},\n-\t\t\t\t\t\t file_name=>\"$file\"),\n-\t\t\t\t\t-class => \"list\"}, esc_path($file));\n+\t\t\t\t$cgi->a({-href => $file_href, -class => \"list\"}, esc_path($file));\n \t\t\tprint \"\\n\";\n \t\t\t$lastfile = $file;\n \t\t}\n@@ -5888,10 +5888,9 @@ sub git_search_files {\n \t\t\t\t$ltext = esc_html($ltext, -nbsp=>1);\n \t\t\t}\n \t\t\tprint \"
\" .\n-\t\t\t\t$cgi->a({-href => href(action=>\"blob\", hash=>$co{'hash'},\n-\t\t\t\t\t\t file_name=>\"$file\").'#l'.$lno,\n-\t\t\t\t\t-class => \"linenr\"}, sprintf('%4i', $lno))\n-\t\t\t\t. ' ' . $ltext . \"
\\n\";\n+\t\t\t\t$cgi->a({-href => $file_href.'#l'.$lno,\n+\t\t\t\t -class => \"linenr\"}, sprintf('%4i', $lno)) .\n+\t\t\t\t' ' . $ltext . \"\\n\";\n \t\t}\n \t}\n \tif ($lastfile) {\n-- \n1.7.6\n"},{"id":"181989","messageId":"201201052132.56581.jnareb@gmail.com","threadId":"29279","inReplyTo":"201201052126.49087.jnareb@gmail.com","subject":"[PATCH 2/2] gitweb: Harden \"grep\" search against filenames with ':'","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-01-05T20:32:56Z","receivedAt":"2012-01-05T20:32:56Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Run \"git grep\" in \"grep\" search with '-z' option, to be able to parse\nresponse also for files with filename containing ':' character. The\n':' character is otherwise (without '-z') used to separate filename\nfrom line number and from matched line.\n\nNote that this does not protect files with filename containing\nembedded newline. This would be hard but doable for text files, and\nharder or even currently impossible with binary files: git does not\nquote filename in\n\n \"Binary file matches\"\n\nmessage, but new `--break` and/or `--header` options to git-grep could\nhelp here.\n\nSigned-off-by: Jakub Narebski \n---\nThis is what I did after fixing previous issue, after looking at current\ncode. Hopefully nobody sane uses filenames with embedded newlines...\n\n http://www.dwheeler.com/essays/fixing-unix-linux-filenames.html\n\n gitweb/gitweb.perl | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex fa58156..f884dfe 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5836,7 +5836,7 @@ sub git_search_files {\n \tmy %co = @_;\n \n \tlocal $/ = \"\\n\";\n-\topen my $fd, \"-|\", git_cmd(), 'grep', '-n',\n+\topen my $fd, \"-|\", git_cmd(), 'grep', '-n', '-z',\n \t\t$search_use_regexp ? ('-E', '-i') : '-F',\n \t\t$searchtext, $co{'tree'}\n \t\t\tor die_error(500, \"Open git-grep failed\");\n@@ -5858,7 +5858,8 @@ sub git_search_files {\n \t\t\t$file = $1;\n \t\t\t$binary = 1;\n \t\t} else {\n-\t\t\t(undef, $file, $lno, $ltext) = split(/:/, $line, 4);\n+\t\t\t($file, $lno, $ltext) = split(/\\0/, $line, 3);\n+\t\t\t$file =~ s/^$co{'tree'}://;\n \t\t}\n \t\tif ($file ne $lastfile) {\n \t\t\t$lastfile and print \"\\n\";\n-- \n1.7.6\n"},{"id":"182497","messageId":"CA+uOhx7QwRQJzyYBCkmVDBRCMt0i_ZqS=sfTG0VmNiVv2dVoww@mail.gmail.com","threadId":"29279","inReplyTo":"201201052126.49087.jnareb@gmail.com","subject":"Re: [PATCH 1/2] gitweb: Fix file links in \"grep\" search","fromName":"Thomas Perl","fromEmail":"th.perl@gmail.com","sentAt":"2012-01-13T14:09:27Z","receivedAt":"2012-01-13T14:09:27Z","isPatch":true,"sender":{"key":"th.perl@gmail.com","avatar":"https://gravatar.com/avatar/fa809414192358405166d421a442112e8d9ee706e69da3113ca428b3ef84e9eb?d=mp&s=160"},"body":"Hi,\n\n2012/1/5 Jakub Narebski :\n> There were two bugs in generating file links (links to \"blob\" view),\n> one hidden by the other.  The correct way of generating file link is\n>\n>        href(action=>\"blob\", hash_base=>$co{'id'},\n>             file_name=>$file);\n>\n> It was $co{'hash'} (this key does not exist, and therefore this is\n> undef), and 'hash' instead of 'hash_base'.\n> [...]\n> Thomas, could you check if this fixes your issue?\n\nSorry for taking a bit longer to respond on this one, but I just got\naround to test all problematic cases that I described with the patch\napplied - it fixes the problem for me (i.e. I can successfully grep in\nnon-master branches and then clicking the link brings me to the right\nlocation).\n\nAs far as I'm concerned, the patch can be applied and fixes the bug.\n\nThanks for the quick fix! :)\nThomas\n"},{"id":"182514","messageId":"7v62gfza0d.fsf@alter.siamese.dyndns.org","threadId":"29279","inReplyTo":"CA+uOhx7QwRQJzyYBCkmVDBRCMt0i_ZqS=sfTG0VmNiVv2dVoww@mail.gmail.com","subject":"Re: [PATCH 1/2] gitweb: Fix file links in \"grep\" search","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-13T19:18:42Z","receivedAt":"2012-01-13T19:18:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Perl writes:\n\n> As far as I'm concerned, the patch can be applied and fixes the bug.\n\nThanks.\n"}]}