{"thread":{"id":"20327","subject":"[PATCH/RFC] gitweb: parse_commit_text encoding fix","startedAt":"2009-08-01T08:28:43Z","lastAt":"2009-08-07T20:31:33Z","messageCount":8,"participants":["Zoltán Füzesi","Jakub Narebski","Füzesi Zoltán","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"119321","messageId":"1249115323-17974-1-git-send-email-zfuzesi@eaglet.hu","threadId":"20327","inReplyTo":null,"subject":"[PATCH/RFC] gitweb: parse_commit_text encoding fix","fromName":"Zoltán Füzesi","fromEmail":"zfuzesi@eaglet.hu","sentAt":"2009-08-01T08:28:43Z","receivedAt":"2009-08-01T08:28:43Z","isPatch":true,"sender":{"key":"zfuzesi@eaglet.hu","avatar":null},"body":"Call to_utf8 when parsing author and committer names, otherwise they will appear\nwith bad encoding if they written by using chop_and_escape_str.\n\nSigned-off-by: Zoltán Füzesi \n---\n gitweb/gitweb.perl | 9 ++++-----\n 1 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7fbd5ff..06bbf60 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2570,22 +2570,21 @@ sub parse_commit_text {\n \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ([0-9a-fA-F]{40})$/)) {\n \t\t\tpush @parents, $1;\n \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'author'} = $1;\n+\t\t\t$co{'author'} = to_utf8($1);\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'author_name'} = $1;\n+\t\t\t\t$co{'author_name'} = to_utf8($1);\n \t\t\t\t$co{'author_email'} = $2;\n \t\t\t} else {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'committer'} = $1;\n+\t\t\t$co{'committer'} = to_utf8($1);\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n-\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n-\t\t\t\t$co{'committer_name'} = $1;\n+\t\t\t\t$co{'committer_name'} = to_utf8($1);\n \t\t\t\t$co{'committer_email'} = $2;\n \t\t\t} else {\n \t\t\t\t$co{'committer_name'} = $co{'committer'};\n-- \n1.6.4.13.ge6580\n"},{"id":"119323","messageId":"m3r5vvris1.fsf@localhost.localdomain","threadId":"20327","inReplyTo":"1249115323-17974-1-git-send-email-zfuzesi@eaglet.hu","subject":"Re: [PATCH/RFC] gitweb: parse_commit_text encoding fix","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-08-01T09:21:55Z","receivedAt":"2009-08-01T09:21:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Zoltán Füzesi writes:\n\n> Call to_utf8 when parsing author and committer names, otherwise they\n> will appear with bad encoding if they written by using\n> chop_and_escape_str.\n\n[re-wrapped]\n\n> \n> Signed-off-by: Zoltán Füzesi \n\nThanks.\n\n\nStill, I do wonder if it would be possible to simply do the following:\n\n -binmode STDOUT, ':utf8';\n +use open qw(:std :utf8);\n\n...but it unfortunately doesn't work. It was tried in\n http://thread.gmane.org/gmane.comp.version-control.git/87129/focus=87135\n\nto_utf8() has at least (possible) fallback if it encounters characters\noutside of UTF-8 coding.\n\n> -\t\t\t$co{'author'} = $1;\n> +\t\t\t$co{'author'} = to_utf8($1);\n\n-- \nJakub Narebski\n\nGit User's Survey 2009: \nhttp://tinyurl.com/GitSurvey2009\n"},{"id":"119347","messageId":"9ab80d150908010955l3710c54bp9e2716570fd1d5ed@mail.gmail.com","threadId":"20327","inReplyTo":"m3r5vvris1.fsf@localhost.localdomain","subject":"Re: [PATCH/RFC] gitweb: parse_commit_text encoding fix","fromName":"Füzesi Zoltán","fromEmail":"zfuzesi@eaglet.hu","sentAt":"2009-08-01T16:55:34Z","receivedAt":"2009-08-01T16:55:34Z","isPatch":true,"sender":{"key":"zfuzesi@eaglet.hu","avatar":null},"body":"2009/8/1 Jakub Narebski :\n>\n> Thanks.\n>\n>\n> Still, I do wonder if it would be possible to simply do the following:\n>\n>  -binmode STDOUT, ':utf8';\n>  +use open qw(:std :utf8);\n>\n> ...but it unfortunately doesn't work.  It was tried in\n>  http://thread.gmane.org/gmane.comp.version-control.git/87129/focus=87135\n>\n> to_utf8() has at least (possible) fallback if it encounters characters\n> outside of UTF-8 coding.\n>\n\nThe following 2 changes in my patch are unnecessary, but please\nconfirm (I'm not familiar with perl (yet)):\n\n- $co{'author_name'} = $1;\n+ $co{'author_name'} = to_utf8($1);\n\n- $co{'committer_name'} = $1;\n+ $co{'committer_name'} = to_utf8($1);\n\nBR,\nZé\n"},{"id":"119379","messageId":"1249198944-19630-1-git-send-email-zfuzesi@eaglet.hu","threadId":"20327","inReplyTo":"9ab80d150908010955l3710c54bp9e2716570fd1d5ed@mail.gmail.com","subject":"[PATCH] gitweb: parse_commit_text encoding fix","fromName":"Zoltán Füzesi","fromEmail":"zfuzesi@eaglet.hu","sentAt":"2009-08-02T07:42:24Z","receivedAt":"2009-08-02T07:42:24Z","isPatch":true,"sender":{"key":"zfuzesi@eaglet.hu","avatar":null},"body":"Call to_utf8 when parsing author and committer names, otherwise they will appear\nwith bad encoding if they written by using chop_and_escape_str.\n\nSigned-off-by: Zoltán Füzesi \n---\n gitweb/gitweb.perl | 5 ++---\n 1 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7fbd5ff..4f05194 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2570,7 +2570,7 @@ sub parse_commit_text {\n \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ([0-9a-fA-F]{40})$/)) {\n \t\t\tpush @parents, $1;\n \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'author'} = $1;\n+\t\t\t$co{'author'} = to_utf8($1);\n \t\t\t$co{'author_epoch'} = $2;\n \t\t\t$co{'author_tz'} = $3;\n \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n@@ -2580,10 +2580,9 @@ sub parse_commit_text {\n \t\t\t\t$co{'author_name'} = $co{'author'};\n \t\t\t}\n \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n-\t\t\t$co{'committer'} = $1;\n+\t\t\t$co{'committer'} = to_utf8($1);\n \t\t\t$co{'committer_epoch'} = $2;\n \t\t\t$co{'committer_tz'} = $3;\n-\t\t\t$co{'committer_name'} = $co{'committer'};\n \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n \t\t\t\t$co{'committer_name'} = $1;\n \t\t\t\t$co{'committer_email'} = $2;\n-- \n1.6.4.13.ge6580\n"},{"id":"119470","messageId":"7viqh43vz3.fsf@alter.siamese.dyndns.org","threadId":"20327","inReplyTo":"1249198944-19630-1-git-send-email-zfuzesi@eaglet.hu","subject":"Re: [PATCH] gitweb: parse_commit_text encoding fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-04T06:59:44Z","receivedAt":"2009-08-04T06:59:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zoltán Füzesi writes:\n\n> Call to_utf8 when parsing author and committer names, otherwise they will appear\n> with bad encoding if they written by using chop_and_escape_str.\n>\n> Signed-off-by: Zoltán Füzesi \n> ---\n\nThanks, Zoltán.\n\nWe should be able to set up a script that scrapes the output to test this\nkind of thing. We may not want to have a test pattern that matches too\nstrictly for the current structure and appearance of the output\n(e.g. counting nested
s, presentation styles and such), but if we can\nrobustly scrape off HTML tags (e.g. \"elinks -dump\") and check the\nremaining payload, it might be enough.\n\nJakub what do you think? I suspect that scraping approach may turn out to\nbe too fragile for tests to be worth doing, but I am just throwing out a\nthought.\n\n> gitweb/gitweb.perl | 5 ++---\n> 1 files changed, 2 insertions(+), 3 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 7fbd5ff..4f05194 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2570,7 +2570,7 @@ sub parse_commit_text {\n> \t\t} elsif ((!defined $withparents) && ($line =~ m/^parent ([0-9a-fA-F]{40})$/)) {\n> \t\t\tpush @parents, $1;\n> \t\t} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {\n> -\t\t\t$co{'author'} = $1;\n> +\t\t\t$co{'author'} = to_utf8($1);\n> \t\t\t$co{'author_epoch'} = $2;\n> \t\t\t$co{'author_tz'} = $3;\n> \t\t\tif ($co{'author'} =~ m/^([^<]+) <([^>]*)>/) {\n> @@ -2580,10 +2580,9 @@ sub parse_commit_text {\n> \t\t\t\t$co{'author_name'} = $co{'author'};\n> \t\t\t}\n> \t\t} elsif ($line =~ m/^committer (.*) ([0-9]+) (.*)$/) {\n> -\t\t\t$co{'committer'} = $1;\n> +\t\t\t$co{'committer'} = to_utf8($1);\n> \t\t\t$co{'committer_epoch'} = $2;\n> \t\t\t$co{'committer_tz'} = $3;\n> -\t\t\t$co{'committer_name'} = $co{'committer'};\n> \t\t\tif ($co{'committer'} =~ m/^([^<]+) <([^>]*)>/) {\n> \t\t\t\t$co{'committer_name'} = $1;\n> \t\t\t\t$co{'committer_email'} = $2;\n> -- \n> 1.6.4.13.ge6580\n"},{"id":"119758","messageId":"9ab80d150908060115q4b56b2e5xb327e09cda7e2b7a@mail.gmail.com","threadId":"20327","inReplyTo":"7viqh43vz3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: parse_commit_text encoding fix","fromName":"Zoltán Füzesi","fromEmail":"zfuzesi@eaglet.hu","sentAt":"2009-08-06T08:15:25Z","receivedAt":"2009-08-06T08:15:25Z","isPatch":true,"sender":{"key":"zfuzesi@eaglet.hu","avatar":null},"body":"2009/8/4 Junio C Hamano :\n>\n> Thanks, Zoltán.\n>\n> We should be able to set up a script that scrapes the output to test this\n> kind of thing.  We may not want to have a test pattern that matches too\n> strictly for the current structure and appearance of the output\n> (e.g. counting nested
s, presentation styles and such), but if we can\n> robustly scrape off HTML tags (e.g. \"elinks -dump\") and check the\n> remaining payload, it might be enough.\n>\n> Jakub what do you think?  I suspect that scraping approach may turn out to\n> be too fragile for tests to be worth doing, but I am just throwing out a\n> thought.\n>\n\nThis issue comes out when chop_and_escape_str function is called with\na non-ascii string (like my name :)) without before calling to_utf8 on\nit. \"author_name\" and \"committer_name\" are two examples, and\n\"author_name\" shows up with bad encoding in HTML.\n\nExample from one of my repos (little piece from shortlog output):\nFüzesi Zoltán\nAfter applying the patch:\nFüzesi Zoltán\n\nThis is an \"old\" (seen in 1.5.6 version too) and (I think) minor issue.\nI haven't spent time on thinking how a test script could show this yet.\nWaiting for Jakub's reaction.\n"},{"id":"119855","messageId":"200908070241.07372.jnareb@gmail.com","threadId":"20327","inReplyTo":"7viqh43vz3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: parse_commit_text encoding fix","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-08-07T00:41:05Z","receivedAt":"2009-08-07T00:41:05Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 4 Aug 2009, Junio C Hamano wrote:\n> Zoltán Füzesi writes:\n> \n> > Call to_utf8 when parsing author and committer names, otherwise they will appear\n> > with bad encoding if they written by using chop_and_escape_str.\n> >\n> > Signed-off-by: Zoltán Füzesi \n> > ---\n> \n> Thanks, Zoltán.\n> \n> We should be able to set up a script that scrapes the output to test this\n> kind of thing. We may not want to have a test pattern that matches too\n> strictly for the current structure and appearance of the output\n> (e.g. counting nested
s, presentation styles and such), but if we can\n> robustly scrape off HTML tags (e.g. \"elinks -dump\") and check the\n> remaining payload, it might be enough.\n> \n> Jakub what do you think? I suspect that scraping approach may turn out to\n> be too fragile for tests to be worth doing, but I am just throwing out a\n> thought.\n\nFirst, I'd like to have existing t9500-gitweb-standalone-no-errors.sh\nbe about Perl errors and warning only, as it is now. Anything outside\nthis should IMVHO be put in separate test.\n\nSecond, for checking whether gitweb handles non US-ASCII input correctly\nwe don't need HTML scrapping or parsing. We can simply check if we have\ncorrect string in output... and (after Zoltán Füzesi example) that we\ndon't have incorrect one. For example if we have 'xxxóxxx' in input,\nthen there is 'xxxóxxx' in output, and that all match againts 'xxx.xxx'\nmatches 'xxxóxxx'.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"119928","messageId":"200908072231.35707.jnareb@gmail.com","threadId":"20327","inReplyTo":"9ab80d150908060115q4b56b2e5xb327e09cda7e2b7a@mail.gmail.com","subject":"Re: [PATCH] gitweb: parse_commit_text encoding fix","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-08-07T20:31:33Z","receivedAt":"2009-08-07T20:31:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 6 Aug 2009, Zoltán Füzesi wrote:\n> 2009/8/4 Junio C Hamano :\n> >\n> > Thanks, Zoltán.\n> >\n> > We should be able to set up a script that scrapes the output to test this\n> > kind of thing.  We may not want to have a test pattern that matches too\n> > strictly for the current structure and appearance of the output\n> > (e.g. counting nested
s, presentation styles and such), but if we can\n> > robustly scrape off HTML tags (e.g. \"elinks -dump\") and check the\n> > remaining payload, it might be enough.\n> >\n> > Jakub what do you think?  I suspect that scraping approach may turn out to\n> > be too fragile for tests to be worth doing, but I am just throwing out a\n> > thought.\n> >\n> \n> This issue comes out when chop_and_escape_str function is called with\n> a non-ascii string (like my name :)) without before calling to_utf8 on\n> it. \"author_name\" and \"committer_name\" are two examples, and\n> \"author_name\" shows up with bad encoding in HTML.\n> \n> Example from one of my repos (little piece from shortlog output):\n> Füzesi Zoltán\n> After applying the patch:\n> Füzesi Zoltán\n> \n> This is an \"old\" (seen in 1.5.6 version too) and (I think) minor issue.\n> I haven't spent time on thinking how a test script could show this yet.\n> Waiting for Jakub's reaction.\n\nOh, so the problem is not only to just have correct output (for example\n\"Füzesi Zoltán\" somewhere on HTML page produced by gitweb), but also do\nnot have incorrect output (for example \"Füzesi Zoltán\").\n\nI think it would be better to leave t9500-gitweb-standalone-no-errors.sh\nto be only about no Perl errors and no Perl warnings. So I'd rather\nhave test checking if gitweb handles non US-ASCII in output correctly\nin a separate test, e.g. t9501-gitweb-standalone-i18n.sh. That would\nmean extracting gitweb_init() and gitweb_run() (and perhaps also\ngitweb_check_prereq() or something) into common file t/lib-gitweb.sh\n\nWe would check e.g. if \"startáąend\" is present in output (correct output),\nand whether extracting \"start[^ ]*end\" produces only \"startáąend\" (no\nincorrect output).\n\n\nAs for gitweb, we should make sure that everything is stored in Perl\nvariables and Perl structures _after_ treating with to_utf8(). This\nwould require some cleanup of the code, and having such test would\nhelp to check if we didn't introduce any regressions.\n\n-- \nJakub Narebski\nPoland\n"}]}