{"thread":{"id":"16362","subject":"[PATCH] gitweb: fix encode handling for site_{header,footer}","startedAt":"2008-11-17T07:53:07Z","lastAt":"2008-12-08T17:05:47Z","messageCount":9,"participants":["Tatsuki Sugiura","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"96011","messageId":"87vdumbxgc.wl@vaj-k-334-sugi.local.valinux.co.jp","threadId":"16362","inReplyTo":null,"subject":"[PATCH] gitweb: fix encode handling for site_{header,footer}","fromName":"Tatsuki Sugiura","fromEmail":"sugi@nemui.org","sentAt":"2008-11-17T07:53:07Z","receivedAt":"2008-11-17T07:53:07Z","isPatch":true,"sender":{"key":"sugi@nemui.org","avatar":null},"body":"All non-ASCII chars of site_{header,footer} will be broken\nby perl IO layer without decoding to utf8.\n\nHere is a fix to just call to_utf8 for inputs from these files.\n\nSigned-off-by: Tatsuki Sugiura \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 933e137..79ca5c2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2929,7 +2929,7 @@ EOF\n \n \tif (-f $site_header) {\n \t\topen (my $fd, $site_header);\n-\t\tprint <$fd>;\n+\t\tprint map {to_utf8($_)} <$fd>;\n \t\tclose $fd;\n \t}\n \n@@ -3018,7 +3018,7 @@ sub git_footer_html {\n \n \tif (-f $site_footer) {\n \t\topen (my $fd, $site_footer);\n-\t\tprint <$fd>;\n+\t\tprint map {to_utf8($_)} <$fd>;\n \t\tclose $fd;\n \t}\n \n-- \n1.5.6.5\n"},{"id":"96022","messageId":"7v8wrizlxo.fsf@gitster.siamese.dyndns.org","threadId":"16362","inReplyTo":"87vdumbxgc.wl@vaj-k-334-sugi.local.valinux.co.jp","subject":"Re: [PATCH] gitweb: fix encode handling for site_{header,footer}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-17T10:28:03Z","receivedAt":"2008-11-17T10:28:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tatsuki Sugiura writes:\n\n> All non-ASCII chars of site_{header,footer} will be broken\n> by perl IO layer without decoding to utf8.\n>\n> Here is a fix to just call to_utf8 for inputs from these files.\n>\n> Signed-off-by: Tatsuki Sugiura \n\nLooks good, thanks.\n\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 933e137..79ca5c2 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2929,7 +2929,7 @@ EOF\n> \n> \tif (-f $site_header) {\n> \t\topen (my $fd, $site_header);\n> -\t\tprint <$fd>;\n> +\t\tprint map {to_utf8($_)} <$fd>;\n> \t\tclose $fd;\n> \t}\n> \n> @@ -3018,7 +3018,7 @@ sub git_footer_html {\n> \n> \tif (-f $site_footer) {\n> \t\topen (my $fd, $site_footer);\n> -\t\tprint <$fd>;\n> +\t\tprint map {to_utf8($_)} <$fd>;\n> \t\tclose $fd;\n> \t}\n> \n> -- \n> 1.5.6.5\n"},{"id":"96025","messageId":"200811171140.45884.jnareb@gmail.com","threadId":"16362","inReplyTo":"7v8wrizlxo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: fix encode handling for site_{header,footer}","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-11-17T10:40:44Z","receivedAt":"2008-11-17T10:40:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 17 Nov 2008, Junio C Hamano wrote:\n> Tatsuki Sugiura writes:\n> \n> > All non-ASCII chars of site_{header,footer} will be broken\n> > by perl IO layer without decoding to utf8.\n> >\n> > Here is a fix to just call to_utf8 for inputs from these files.\n> >\n> > Signed-off-by: Tatsuki Sugiura \n> \n> Looks good, thanks.\n\nHmmm... it is good _partial_ solution. I wonder if gitweb doesn't\nhave the same problem with per-project README.html file...\n\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 933e137..79ca5c2 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -2929,7 +2929,7 @@ EOF\n> > \n> > \tif (-f $site_header) {\n> > \t\topen (my $fd, $site_header);\n> > -\t\tprint <$fd>;\n> > +\t\tprint map {to_utf8($_)} <$fd>;\n> > \t\tclose $fd;\n> > \t}\n> > \n> > @@ -3018,7 +3018,7 @@ sub git_footer_html {\n> > \n> > \tif (-f $site_footer) {\n> > \t\topen (my $fd, $site_footer);\n> > -\t\tprint <$fd>;\n> > +\t\tprint map {to_utf8($_)} <$fd>;\n> > \t\tclose $fd;\n> > \t}\n> > \n> > -- \n> > 1.5.6.5\n\nThere was some patch by Recai Oktaş, or Gerrit Pape (or me)\n\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4c104d2..8b5fcc1 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -16,7 +16,7 @@ use Encode;\n use Fcntl ':mode';\n use File::Find qw();\n use File::Basename qw(basename);\n-binmode STDOUT, ':utf8';\n+use open qw(:std :utf8);\n \n BEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n\n\nThen to_utf8() wouldn't be necessary. But I'd like for Perl experts to \ncheck it first; for example this doesn't allow $fallback_encoding.\n\n-- \nJakub Narębski\nPoland\n"},{"id":"96880","messageId":"20081201175741.11133.82393.stgit@localhost.localdomain","threadId":"16362","inReplyTo":"200811171140.45884.jnareb@gmail.com","subject":"[PATCH] gitweb: Fix handling of non-ASCII characters in inserted HTML files","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-12-01T18:01:42Z","receivedAt":"2008-12-01T18:01:42Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use new insert_file() subroutine to insert HTML chunks from external\nfiles: $site_header, $home_text (by default indextext.html),\n$site_footer, and $projectroot/$project/REAME.html.\n\nAll non-ASCII chars of those files will be broken by Perl IO layer\nwithout decoding to utf8, so insert_file() does to_utf8() on each\nprinted line; alternate solution would be to open those files with\n\"binmode $fh, ':utf8'\", or even all files with \"use open qw(:std :utf8)\".\n\nNote that inserting README.html lost one of checks for simplicity.\n\nNoticed-by: Tatsuki Sugiura \nSigned-off-by: Jakub Narebski \n---\nThis is more complete solution that the one provided by Tatsuki Sugiura\nin original patch\n\n [PATCH] gitweb: fix encode handling for site_{header,footer}\n Msg-Id: <87vdumbxgc.wl@vaj-k-334-sugi.local.valinux.co.jp>\n http://thread.gmane.org/gmane.comp.version-control.git/101199\n\nbut it is in principle the same solution.\n\nI think this one as it is a bugfix should go in git 1.6.1\n\n gitweb/gitweb.perl | 33 +++++++++++++++++----------------\n 1 files changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 933e137..82262a3 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2740,6 +2740,15 @@ sub get_file_owner {\n \treturn to_utf8($owner);\n }\n \n+# assume that file exists\n+sub insert_file {\n+\tmy $filename = shift;\n+\n+\topen my $fd, '<', $filename;\n+\tprint map(to_utf8, <$fd>);\n+\tclose $fd;\n+}\n+\n ## ......................................................................\n ## mimetype related functions\n \n@@ -2928,9 +2937,7 @@ EOF\n \t \"\\n\";\n \n \tif (-f $site_header) {\n-\t\topen (my $fd, $site_header);\n-\t\tprint <$fd>;\n-\t\tclose $fd;\n+\t\tinsert_file($site_header);\n \t}\n \n \tprint \"
\\n\" .\n@@ -3017,9 +3024,7 @@ sub git_footer_html {\n \tprint \"
\\n\"; # class=\"page_footer\"\n \n \tif (-f $site_footer) {\n-\t\topen (my $fd, $site_footer);\n-\t\tprint <$fd>;\n-\t\tclose $fd;\n+\t\tinsert_file($site_footer);\n \t}\n \n \tprint \"\\n\" .\n@@ -4358,9 +4363,7 @@ sub git_project_list {\n \tgit_header_html();\n \tif (-f $home_text) {\n \t\tprint \"
\\n\";\n-\t\topen (my $fd, $home_text);\n-\t\tprint <$fd>;\n-\t\tclose $fd;\n+\t\tinsert_file($home_text);\n \t\tprint \"
\\n\";\n \t}\n \tprint $cgi->startform(-method => \"get\") .\n@@ -4472,13 +4475,11 @@ sub git_summary {\n \tprint \"\\n\";\n \n \tif (-s \"$projectroot/$project/README.html\") {\n-\t\tif (open my $fd, \"$projectroot/$project/README.html\") {\n-\t\t\tprint \"
readme
\\n\" .\n-\t\t\t \"
\\n\";\n-\t\t\tprint $_ while (<$fd>);\n-\t\t\tprint \"\\n
\\n\"; # class=\"readme\"\n-\t\t\tclose $fd;\n-\t\t}\n+\t\tprint \"
readme
\\n\" .\n+\t\t \"
\\n\";\n+\t\tinsert_file(\"$projectroot/$project/README.html\");\n+\t\tprint \"\\n
\\n\"; # class=\"readme\"\n+\t\tclose $fd;\n \t}\n \n \t# we need to request one more than 16 (0..15) to check if\n"},{"id":"97016","messageId":"7v63m1j4ke.fsf@gitster.siamese.dyndns.org","threadId":"16362","inReplyTo":"20081201175741.11133.82393.stgit@localhost.localdomain","subject":"Re: [PATCH] gitweb: Fix handling of non-ASCII characters in inserted HTML files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-03T03:55:13Z","receivedAt":"2008-12-03T03:55:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Use new insert_file() subroutine to insert HTML chunks from external\n> files: $site_header, $home_text (by default indextext.html),\n> $site_footer, and $projectroot/$project/REAME.html.\n>\n> All non-ASCII chars of those files will be broken by Perl IO layer\n> without decoding to utf8, so insert_file() does to_utf8() on each\n> printed line; alternate solution would be to open those files with\n> \"binmode $fh, ':utf8'\", or even all files with \"use open qw(:std :utf8)\".\n>\n> Note that inserting README.html lost one of checks for simplicity.\n>\n> Noticed-by: Tatsuki Sugiura \n> Signed-off-by: Jakub Narebski \n> ---\n> This is more complete solution that the one provided by Tatsuki Sugiura\n> in original patch\n>\n> [PATCH] gitweb: fix encode handling for site_{header,footer}\n> Msg-Id: <87vdumbxgc.wl@vaj-k-334-sugi.local.valinux.co.jp>\n> http://thread.gmane.org/gmane.comp.version-control.git/101199\n\nIt may be more complete but it is obviously untested. Please help me\ntrust you better with your future patches. Because I personally do not\nrun gitweb myself, I really need a trustworthy lieutenant(s) in the area.\n\n[Wed Dec 3 01:52:07 2008] gitweb.perl: Global symbol \"$fd\" requires explicit package name at /git.git/t/../gitweb/gitweb.perl line 4500.\n[Wed Dec 3 01:52:07 2008] gitweb.perl: Execution of /git.git/t/../gitweb/gitweb.perl aborted due to compilation errors.\n\n> but it is in principle the same solution.\n>\n> I think this one as it is a bugfix should go in git 1.6.1\n\nTrading a gitweb with a small bug with a gitweb that does not even pass\nits test script does not feel like a good change to me.\n\nI think the breakage is the \"close $fd\" at the end of this hunk:\n\n> @@ -4472,13 +4475,11 @@ sub git_summary {\n> \tprint \"\\n\";\n> \n> \tif (-s \"$projectroot/$project/README.html\") {\n> -\t\tif (open my $fd, \"$projectroot/$project/README.html\") {\n> -\t\t\tprint \"
readme
\\n\" .\n> -\t\t\t \"
\\n\";\n> -\t\t\tprint $_ while (<$fd>);\n> -\t\t\tprint \"\\n
\\n\"; # class=\"readme\"\n> -\t\t\tclose $fd;\n> -\t\t}\n> +\t\tprint \"
readme
\\n\" .\n> +\t\t \"
\\n\";\n> +\t\tinsert_file(\"$projectroot/$project/README.html\");\n> +\t\tprint \"\\n
\\n\"; # class=\"readme\"\n> +\t\tclose $fd;\n> \t}\n> \n> \t# we need to request one more than 16 (0..15) to check if\n\nI'll queue it to 'pu', with the \"close $fd\" removed, for now.\n"},{"id":"97045","messageId":"200812031121.43606.jnareb@gmail.com","threadId":"16362","inReplyTo":"7v63m1j4ke.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: Fix handling of non-ASCII characters in inserted HTML files","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-12-03T10:21:42Z","receivedAt":"2008-12-03T10:21:42Z","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> > Use new insert_file() subroutine to insert HTML chunks from external\n> > files: $site_header, $home_text (by default indextext.html),\n> > $site_footer, and $projectroot/$project/REAME.html.\n> >\n> > All non-ASCII chars of those files will be broken by Perl IO layer\n> > without decoding to utf8, so insert_file() does to_utf8() on each\n> > printed line; alternate solution would be to open those files with\n> > \"binmode $fh, ':utf8'\", or even all files with \"use open qw(:std :utf8)\".\n\n> > This is more complete solution that the one provided by Tatsuki Sugiura\n> > in original patch\n> >\n> > [PATCH] gitweb: fix encode handling for site_{header,footer}\n> > Msg-Id: <87vdumbxgc.wl@vaj-k-334-sugi.local.valinux.co.jp>\n> > http://thread.gmane.org/gmane.comp.version-control.git/101199\n> \n> It may be more complete but it is obviously untested. Please help me\n> trust you better with your future patches. Because I personally do not\n> run gitweb myself, I really need a trustworthy lieutenant(s) in the area.\n> \n> [Wed Dec 3 01:52:07 2008] gitweb.perl: Global symbol \"$fd\" requires explicit package name at /git.git/t/../gitweb/gitweb.perl line 4500.\n> [Wed Dec 3 01:52:07 2008] gitweb.perl: Execution of /git.git/t/../gitweb/gitweb.perl aborted due to compilation errors.\n> \n> > but it is in principle the same solution.\n> >\n> > I think this one as it is a bugfix should go in git 1.6.1\n> \n> Trading a gitweb with a small bug with a gitweb that does not even pass\n> its test script does not feel like a good change to me.\n> \n> I think the breakage is the \"close $fd\" at the end of this hunk:\n[...]\n> I'll queue it to 'pu', with the \"close $fd\" removed, for now.\n\nI'm very sorry about that. It was a bit of time since my last patch\nsent, and I forgot that nevermind how obvious and simple the change,\none should run relevant parts of test suite, or at least try to run\ngitweb / changed command.\n\nWith \"close $fd\" removed the patch is correct (and patches t9500*).\n-- \nJakub Narebski\nPoland\n"},{"id":"97096","messageId":"7vfxl5c66h.fsf@gitster.siamese.dyndns.org","threadId":"16362","inReplyTo":"200812031121.43606.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Fix handling of non-ASCII characters in inserted HTML files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-03T21:14:30Z","receivedAt":"2008-12-03T21:14:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> With \"close $fd\" removed the patch is correct (and patches t9500*).\n\nYeah, it passes that test here, too. Let's unleash it to 'master' and in\nan unlikely case where it still has bugs we know which commit to revert\n;-).\n\nThanks.\n"},{"id":"97344","messageId":"20081208130905.15803.83149.stgit@localhost.localdomain","threadId":"16362","inReplyTo":"7vfxl5c66h.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] gitweb: Fix bug in insert_file() subroutine","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-12-08T13:13:21Z","receivedAt":"2008-12-08T13:13:21Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"In insert_file() subroutine (which is used to insert HTML fragments as\ncustom header, footer, hometext (for projects list view), and per\nproject README.html (for summary view)) we used:\n\n map(to_utf8, <$fd>);\n\nThis doesn't work, and other form has to be used:\n\n map { to_utf8($_) } <$fd>;\n\nNow with test for t9600 added, for $GIT_DIR/README.html.\n\nSigned-off-by: Jakub Narebski \n---\nJunio C Hamano wrote:\n> Jakub Narebski writes:\n>> \n>> With \"close $fd\" removed the patch is correct (and patches t9500*).\n> \n> Yeah, it passes that test here, too. Let's unleash it to 'master' and in\n> an unlikely case where it still has bugs we know which commit to revert\n> ;-).\n\nI'm extremely sorry about that, but it still had bugs. I thought\nI could use \"map EXPR,LIST\" form instead of \"map BLOCK LIST\" here,\ni.e. \"map(to_utf8, <$fd>)\" instead of \"map {to_utf8($_)} <$fd>;\"\nin original patch by Tatsuki Sugiura.\n\nJust in case I have added check for this in t9500 gitweb test.\n\nOnce again, I'm very sorry for my buggy patches...\n\n gitweb/gitweb.perl | 2 +-\n t/t9500-gitweb-standalone-no-errors.sh | 10 ++++++++++\n 2 files changed, 11 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 9517392..6eb370d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2764,7 +2764,7 @@ sub insert_file {\n \tmy $filename = shift;\n \n \topen my $fd, '<', $filename;\n-\tprint map(to_utf8, <$fd>);\n+\tprint map { to_utf8($_) } <$fd>;\n \tclose $fd;\n }\n \ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 64c4cce..43cd6ee 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -673,4 +673,14 @@ test_expect_success \\\n \t gitweb_run \"p=.git;a=tree\"'\n test_debug 'cat gitweb.log'\n \n+# ----------------------------------------------------------------------\n+# non-ASCII in README.html\n+\n+test_expect_success \\\n+\t'README.html with non-ASCII characters (utf-8)' \\\n+\t'echo \"UTF-8 example:
\" > .git/README.html &&\n+\t cat \"$TEST_DIRECTORY\"/t3900/1-UTF-8.txt >> .git/README.html &&\n+\t gitweb_run \"p=.git;a=summary\"'\n+test_debug 'cat gitweb.log'\n+\n test_done\n\n-- \nStacked GIT 0.14.3\ngit version 1.6.0.4\n"},{"id":"97364","messageId":"7vr64i7g2c.fsf@gitster.siamese.dyndns.org","threadId":"16362","inReplyTo":"20081208130905.15803.83149.stgit@localhost.localdomain","subject":"Re: [PATCH] gitweb: Fix bug in insert_file() subroutine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-08T17:05:47Z","receivedAt":"2008-12-08T17:05:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> +# ----------------------------------------------------------------------\n> +# non-ASCII in README.html\n> +\n> +test_expect_success \\\n> +\t'README.html with non-ASCII characters (utf-8)' \\\n> +\t'echo \"UTF-8 example:
\" > .git/README.html &&\n> +\t cat \"$TEST_DIRECTORY\"/t3900/1-UTF-8.txt >> .git/README.html &&\n> +\t gitweb_run \"p=.git;a=summary\"'\n> +test_debug 'cat gitweb.log'\n> +\n\nYup, a new test would certainly help to catch breakages ;-)\n\nThanks.\n"}]}