{"thread":{"id":"13710","subject":"gitweb forgets to send utf8 header for raw blob views","startedAt":"2008-05-28T18:04:03Z","lastAt":"2008-06-03T14:47:10Z","messageCount":12,"participants":["Jan Engelhardt","Lea Wiemann","Jakub Narebski","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"77960","messageId":"alpine.LNX.1.10.0805282002510.19264@fbirervta.pbzchgretzou.qr","threadId":"13710","inReplyTo":null,"subject":"gitweb forgets to send utf8 header for raw blob views","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2008-05-28T18:04:03Z","receivedAt":"2008-05-28T18:04:03Z","isPatch":false,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"Hi,\n\n\nI have configured gitweb to use utf8, and that works for text blob views \nlike on\nhttp://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob;f=bin/git-forest;hb=HEAD\nbut it does not for raw blob views like\nhttp://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob_plain;f=bin/git-forest;hb=HEAD\n\n\nJan\n"},{"id":"78031","messageId":"483E9468.1090300@gmail.com","threadId":"13710","inReplyTo":"alpine.LNX.1.10.0805282002510.19264@fbirervta.pbzchgretzou.qr","subject":"Re: gitweb forgets to send utf8 header for raw blob views","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-05-29T11:32:56Z","receivedAt":"2008-05-29T11:32:56Z","isPatch":false,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jan Engelhardt wrote:\n> [utf8 does not work] for raw blob views like\n> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob_plain;f=bin/git-forest;hb=HEAD\n\nGitweb should probably not be recoding blobs, so the best I can think of \nis check for UTF-8 validity and add charset=utf-8 in that case (and in \nother cases leave the charset undeclared).\n\nThe drawback with that is that we cannot send plain blobs without \nreading them into memory (or reading them twice), since we have to check \nfor UTF-8 validity of the whole blob before sending it.  (Gitweb is \ncurrently reading the whole blob into memory, but that's unnecessary and \ncould be changed in the future.)\n\nAfter my next refactoring, there *might* be some chance to easily \nimplement something like \"if it's smaller than x KB (e.g. 512), read it \ninto memory, check for valid UTF-8 and optionally add charset=utf-8, \notherwise don't read it into memory and send it without charset=utf-8 \n[or perhaps check for BOM presence at the beginning].\"  I'll remember \nif/when it comes up in my refactoring and get back to the mailing list \nabout it.\n\n-- Lea\n"},{"id":"78119","messageId":"m3tzgg1a06.fsf@localhost.localdomain","threadId":"13710","inReplyTo":"alpine.LNX.1.10.0805282002510.19264@fbirervta.pbzchgretzou.qr","subject":"Re: gitweb forgets to send utf8 header for raw blob views","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-30T08:18:14Z","receivedAt":"2008-05-30T08:18:14Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jan Engelhardt <jengelh@medozas.de> writes:\n\n> I have configured gitweb to use utf8, and that works for text blob views \n> like on\n> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob;f=bin/git-forest;hb=HEAD\n> but it does not for raw blob views like\n> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob_plain;f=bin/git-forest;hb=HEAD\n\nThis can depend on configuration, both on gitweb configuration (you\ncan for example define $default_blob_plain_mimetype to 'text/plain;\ncharset=utf-8', and define $default_text_plain_charset to 'utf-8'),\nand on your /etc/mime.types; gitweb does not add charset info if\nmimetype is acquired from mime.types, which I guess is a mistake.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"78216","messageId":"20080531112513.30913.44393.stgit@localhost.localdomain","threadId":"13710","inReplyTo":"m3tzgg1a06.fsf@localhost.localdomain","subject":"[PATCH] gitweb: Add charset info to \"raw\" blob output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-31T11:27:01Z","receivedAt":"2008-05-31T11:27:01Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Always add charset info from $default_text_plain_charset (if it is\ndefined) to \"raw\" (a=blob_plain) output for 'text/plain' blobs.\nAdding charset info in a special case was removed from blob_mimetype().\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nPlease note that to have utf-8 for 'text/plain' blobs in blob_plain\nview (\"raw\" output) you still have to set $default_text_plain_charset\nto 'utf-8' (in gitweb configuration file).\n\n gitweb/gitweb.perl |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 57a1905..dd0f0ac 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2481,8 +2481,7 @@ sub blob_mimetype {\n \treturn $default_blob_plain_mimetype unless $fd;\n \n \tif (-T $fd) {\n-\t\treturn 'text/plain' .\n-\t\t       ($default_text_plain_charset ? '; charset='.$default_text_plain_charset : '');\n+\t\treturn 'text/plain';\n \t} elsif (! $filename) {\n \t\treturn 'application/octet-stream';\n \t} elsif ($filename =~ m/\\.png$/i) {\n@@ -4397,6 +4396,9 @@ sub git_blob_plain {\n \t\tor die_error(undef, \"Couldn't cat $file_name, $hash\");\n \n \t$type ||= blob_mimetype($fd, $file_name);\n+\tif ($type eq 'text/plain' && defined $default_text_plain_charset) {\n+\t\t$type .= \"; charset=$default_text_plain_charset\";\n+\t}\n \n \t# save as filename, even when no $file_name is given\n \tmy $save_as = \"$hash\";\n"},{"id":"78233","messageId":"alpine.LNX.1.10.0805311703420.4055@fbirervta.pbzchgretzou.qr","threadId":"13710","inReplyTo":"m3tzgg1a06.fsf@localhost.localdomain","subject":"Re: gitweb forgets to send utf8 header for raw blob views","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2008-05-31T15:04:53Z","receivedAt":"2008-05-31T15:04:53Z","isPatch":false,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Friday 2008-05-30 10:18, Jakub Narebski wrote:\n>Jan Engelhardt <jengelh@medozas.de> writes:\n>\n>> I have configured gitweb to use utf8, and that works for text blob views \n>> like on\n>> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob;f=bin/git-forest;hb=HEAD\n>> but it does not for raw blob views like\n>> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob_plain;f=bin/git-forest;hb=HEAD\n>\n>This can depend on configuration, both on gitweb configuration (you\n>can for example define $default_blob_plain_mimetype to 'text/plain;\n>charset=utf-8', and define $default_text_plain_charset to 'utf-8'),\n>and on your /etc/mime.types; gitweb does not add charset info if\n>mimetype is acquired from mime.types, which I guess is a mistake.\n\nThanks for the hint. Setting \n\tour $default_text_plain_charset  = \"utf-8\";\nwas all that was needed. I only had $fallback_encoding set to utf-8\nfor whatever reason...\n"},{"id":"78239","messageId":"7vprr2fi5z.fsf@gitster.siamese.dyndns.org","threadId":"13710","inReplyTo":"20080531112513.30913.44393.stgit@localhost.localdomain","subject":"Re: [PATCH] gitweb: Add charset info to \"raw\" blob output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-31T18:22:00Z","receivedAt":"2008-05-31T18:22:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Always add charset info from $default_text_plain_charset (if it is\n> defined) to \"raw\" (a=blob_plain) output for 'text/plain' blobs.\n> Adding charset info in a special case was removed from blob_mimetype().\n>\n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n> ---\n\nLooks Ok but it took a bit of digging on the list for me to figure out\nthat something like this was missing from the beginning of your commit log\nmessage:\n\n\tEarlier \"blob_plain\" view sent \"charset=utf-8\" only when gitweb\n\tguessed the content type to be text by reading from it, and not\n\twhen the MIME type was obtained from /etc/mime.types.\n\n\tThis fixes the bug by always adding....\n\nBut I wonder if moving of this to the calling site is the right thing to\ndo.  Wouldn't it become much more contained and robust if you did it this\nway?\n\n gitweb/gitweb.perl |   34 +++++++++++++++++++---------------\n 1 files changed, 19 insertions(+), 15 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 57a1905..f5338e1 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2471,29 +2471,33 @@ sub mimetype_guess {\n sub blob_mimetype {\n \tmy $fd = shift;\n \tmy $filename = shift;\n+\tmy $mime;\n \n \tif ($filename) {\n-\t\tmy $mime = mimetype_guess($filename);\n-\t\t$mime and return $mime;\n-\t}\n-\n-\t# just in case\n-\treturn $default_blob_plain_mimetype unless $fd;\n-\n-\tif (-T $fd) {\n-\t\treturn 'text/plain' .\n-\t\t       ($default_text_plain_charset ? '; charset='.$default_text_plain_charset : '');\n+\t\t$mime = mimetype_guess($filename);\n+\t} else if (!defined $fd) {\n+\t\t$mime = $default_blob_plain_mimetype;\n+\t} else if (-T $fd) {\n+\t\t$mime = 'text/plain';\n \t} elsif (! $filename) {\n-\t\treturn 'application/octet-stream';\n+\t\t$mime = 'application/octet-stream';\n \t} elsif ($filename =~ m/\\.png$/i) {\n-\t\treturn 'image/png';\n+\t\t$mime = 'image/png';\n \t} elsif ($filename =~ m/\\.gif$/i) {\n-\t\treturn 'image/gif';\n+\t\t$mime = 'image/gif';\n \t} elsif ($filename =~ m/\\.jpe?g$/i) {\n-\t\treturn 'image/jpeg';\n+\t\t$mime = 'image/jpeg';\n \t} else {\n-\t\treturn 'application/octet-stream';\n+\t\t$mime = 'application/octet-stream';\n \t}\n+\n+\t# Type specific postprocessing can be added as needed...\n+\tif ($mime =~ /^text\\//i &&\n+\t    $mime !~ /charset=/i && $default_text_plain_charset) {\n+\t\t$mime .=  '; charset='.$default_text_plain_charset;\n+\t}\n+\n+\treturn $mime;\n }\n \n ## ======================================================================\n"},{"id":"78252","messageId":"200806010039.14663.jnareb@gmail.com","threadId":"13710","inReplyTo":"alpine.LNX.1.10.0805311703420.4055@fbirervta.pbzchgretzou.qr","subject":"Re: gitweb forgets to send utf8 header for raw blob views","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-31T22:39:12Z","receivedAt":"2008-05-31T22:39:12Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 31 May 2008, Jan Engelhardt wrote:\n> On Friday 2008-05-30 10:18, Jakub Narebski wrote:\n>>Jan Engelhardt <jengelh@medozas.de> writes:\n>>\n>>> I have configured gitweb to use utf8, and that works for text blob views \n>>> like on\n>>> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob;f=bin/git-forest;hb=HEAD\n>>> but it does not for raw blob views like\n>>> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob_plain;f=bin/git-forest;hb=HEAD\n>>\n>> This can depend on configuration, both on gitweb configuration (you\n>> can for example define $default_blob_plain_mimetype to 'text/plain;\n>> charset=utf-8', and define $default_text_plain_charset to 'utf-8'),\n>> and on your /etc/mime.types; gitweb does not add charset info if\n>> mimetype is acquired from mime.types, which I guess is a mistake.\n> \n> Thanks for the hint. Setting \n> \tour $default_text_plain_charset  = \"utf-8\";\n> was all that was needed.\n\nBy the way, do you think that this should be the default for gitweb?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"78263","messageId":"alpine.LNX.1.10.0806010407460.4055@fbirervta.pbzchgretzou.qr","threadId":"13710","inReplyTo":"200806010039.14663.jnareb@gmail.com","subject":"Re: gitweb forgets to send utf8 header for raw blob views","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2008-06-01T02:08:30Z","receivedAt":"2008-06-01T02:08:30Z","isPatch":false,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Sunday 2008-06-01 00:39, Jakub Narebski wrote:\n\n>On Sat, 31 May 2008, Jan Engelhardt wrote:\n>> On Friday 2008-05-30 10:18, Jakub Narebski wrote:\n>>>Jan Engelhardt <jengelh@medozas.de> writes:\n>>>\n>>>> I have configured gitweb to use utf8, and that works for text blob views \n>>>> like on\n>>>> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob;f=bin/git-forest;hb=HEAD\n>>>> but it does not for raw blob views like\n>>>> http://dev.medozas.de/gitweb.cgi?p=hxtools;a=blob_plain;f=bin/git-forest;hb=HEAD\n>>>\n>>> This can depend on configuration, both on gitweb configuration (you\n>>> can for example define $default_blob_plain_mimetype to 'text/plain;\n>>> charset=utf-8', and define $default_text_plain_charset to 'utf-8'),\n>>> and on your /etc/mime.types; gitweb does not add charset info if\n>>> mimetype is acquired from mime.types, which I guess is a mistake.\n>> \n>> Thanks for the hint. Setting \n>> \tour $default_text_plain_charset  = \"utf-8\";\n>> was all that was needed.\n>\n>By the way, do you think that this should be the default for gitweb?\n\nDefinitely. I also made tbz2 the default for me over tgz, because that's\njust how it is.\n"},{"id":"78286","messageId":"200806011306.45945.jnareb@gmail.com","threadId":"13710","inReplyTo":"7vprr2fi5z.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: Add charset info to \"raw\" blob output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-01T11:06:45Z","receivedAt":"2008-06-01T11:06:45Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 31 May 2008, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> Always add charset info from $default_text_plain_charset (if it is\n>> defined) to \"raw\" (a=blob_plain) output for 'text/plain' blobs.\n>> Adding charset info in a special case was removed from blob_mimetype().\n>>\n>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n>> ---\n> \n> Looks Ok but it took a bit of digging on the list for me to figure out\n> that something like this was missing from the beginning of your commit log\n> message:\n> \n> \tEarlier \"blob_plain\" view sent \"charset=utf-8\" only when gitweb\n> \tguessed the content type to be text by reading from it, and not\n> \twhen the MIME type was obtained from /etc/mime.types.\n> \n> \tThis fixes the bug by always adding....\n\nI'm sorry that I have forgot to put the \"why\" in commit message.\nI'd add this when resending v2 of this patch.\n\nThanks for a comment.\n\n> But I wonder if moving of this to the calling site is the right thing to\n> do.  Wouldn't it become much more contained and robust if you did it this\n> way?\n[...]\n>  sub blob_mimetype {\n\nThis _might_ be better.  I didn't do this for the following two reasons:\n\nFirst, from purely theoretical point of view the name of subroutine is\nblob_mimetype(), and I think that charset info has place in Content-Type,\nbut is not part of MIME type info.  \n\nSecond, blob_mimetype() is used in two places: in git_blob_plain\n(in \"raw\" blob view) to generate correct Content-Type HTTP header, and\nin git_blob to decide whether a.) blame makes sense, b.) whether to\nredirect to \"raw\" (a=blob_plain) view.  I'd rather not muck with\ncharset info in second case, although I don't think that it matters\nat all, at least for now.\n\n\nSo perhaps best of those ways would be to create thin wrapper subroutine,\nblob_contenttype($fd, $file_name, $mimetype), where both $file_name and\n(especially) $mimetype are optional parameters, and ise it in\ngit_blob_plain() subroutine...\n\n> +\t# Type specific postprocessing can be added as needed...\n> +\tif ($mime =~ /^text\\//i &&\n> +\t    $mime !~ /charset=/i && $default_text_plain_charset) {\n> +\t\t$mime .=  '; charset='.$default_text_plain_charset;\n> +\t}\n> +\n> +\treturn $mime;\n\nI'm not sure about it.  I worry a bit about text/html, which can, and\nusually do, contain charset info inside the document.  I'm not sure\nwhat happens when charset information from HTTP headers contradict\ncharset information from presented file.  That's why I have limited\nadding charset info purely to 'text/plain', not 'text/*' without\ncharset info already present.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"78287","messageId":"alpine.LNX.1.10.0806011413110.4055@fbirervta.pbzchgretzou.qr","threadId":"13710","inReplyTo":"200806011306.45945.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Add charset info to \"raw\" blob output","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2008-06-01T12:15:12Z","receivedAt":"2008-06-01T12:15:12Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Sunday 2008-06-01 13:06, Jakub Narebski wrote:\n>> +\t# Type specific postprocessing can be added as needed...\n>> +\tif ($mime =~ /^text\\//i &&\n>> +\t    $mime !~ /charset=/i && $default_text_plain_charset) {\n>> +\t\t$mime .=  '; charset='.$default_text_plain_charset;\n>> +\t}\n>> +\n>> +\treturn $mime;\n>\n>I'm not sure about it.  I worry a bit about text/html, which can, and\n>usually do, contain charset info inside the document.  I'm not sure\n>what happens when charset information from HTTP headers contradict\n>charset information from presented file.\n\nThe HTTP header takes -- as stupid as it looks -- precedence\nover the HTML header. As such, a charset in the HTTP Response Header\nshould ONLY be sent if the file is guaranteed to be text/plain only.\n\n>That's why I have limited\n>adding charset info purely to 'text/plain', not 'text/*' without\n>charset info already present.\n"},{"id":"78288","messageId":"alpine.LNX.1.10.0806011415270.4055@fbirervta.pbzchgretzou.qr","threadId":"13710","inReplyTo":"alpine.LNX.1.10.0806011413110.4055@fbirervta.pbzchgretzou.qr","subject":"Re: [PATCH] gitweb: Add charset info to \"raw\" blob output","fromName":"Jan Engelhardt","fromEmail":"jengelh@medozas.de","sentAt":"2008-06-01T12:16:56Z","receivedAt":"2008-06-01T12:16:56Z","isPatch":true,"sender":{"key":"jengelh@medozas.de","avatar":null},"body":"\nOn Sunday 2008-06-01 14:15, Jan Engelhardt wrote:\n>On Sunday 2008-06-01 13:06, Jakub Narebski wrote:\n>>> +\t# Type specific postprocessing can be added as needed...\n>>> +\tif ($mime =~ /^text\\//i &&\n>>> +\t    $mime !~ /charset=/i && $default_text_plain_charset) {\n>>> +\t\t$mime .=  '; charset='.$default_text_plain_charset;\n>>> +\t}\n>>> +\n>>> +\treturn $mime;\n>>\n>>I'm not sure about it.  I worry a bit about text/html, which can, and\n>>usually do, contain charset info inside the document.  I'm not sure\n>>what happens when charset information from HTTP headers contradict\n>>charset information from presented file.\n>\n>The HTTP header takes -- as stupid as it looks -- precedence\n>over the HTML header. As such, a charset in the HTTP Response Header\n>should ONLY be sent if the file is guaranteed to be text/plain only.\n\nMinor correction; s/file/output/. gitweb normaly produces HTML for\nall its normal views, so no Charset header here; but when it\noutputs \"raw\", it should provide one.\n"},{"id":"78511","messageId":"200806031647.10982.jnareb@gmail.com","threadId":"13710","inReplyTo":"200806011306.45945.jnareb@gmail.com","subject":"[PATCH v2] gitweb: Add charset info to \"raw\" output of 'text/plain' blobs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-03T14:47:10Z","receivedAt":"2008-06-03T14:47:10Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Earlier \"blob_plain\" view sent \"charset=utf-8\" only when gitweb\nguessed the content type to be text by reading from it, and not when\nthe MIME type was obtained from /etc/mime.types, or when gitweb\ncouldn't guess mimetype and used $default_blob_plain_mimetype.\n\nThis fixes the bug by always add charset info from\n$default_text_plain_charset (if it is defined) to \"raw\" (a=blob_plain)\noutput for 'text/plain' blobs.\n\nGenerating information for Content-Type: header got separated into\nblob_contenttype() subroutine; adding charset info in a special case\nwas removed from blob_mimetype(), which now should return mimetype\nonly.\n\n\nWhile at it cleanup code a bit: put subroutine parameter\ninitialization first, make error message more robust (when $file_name\nis not defined) if more cryptic, remove unnecessary '\"' around\nvariable (\"$var\" -> $var).\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n\nOn Sun, 1 June 2008, Jakub Narebski wrote:\n> On Sat, 31 May 2008, Junio C Hamano wrote:\n>> Jakub Narebski <jnareb@gmail.com> writes:\n>> \n>>> Always add charset info from $default_text_plain_charset (if it is\n>>> defined) to \"raw\" (a=blob_plain) output for 'text/plain' blobs.\n>>> Adding charset info in a special case was removed from blob_mimetype().\n>> \n>> Looks Ok but it took a bit of digging on the list for me to figure out\n>> that something like this was missing from the beginning of your commit log\n>> message:\n>> \n>> \tEarlier \"blob_plain\" view sent \"charset=utf-8\" only when gitweb\n>> \tguessed the content type to be text by reading from it, and not\n>> \twhen the MIME type was obtained from /etc/mime.types.\n>> \n>> \tThis fixes the bug by always adding....\n> \n> I'm sorry that I have forgot to put the \"why\" in commit message.\n> I'd add this when resending v2 of this patch.\n\nAdded.\n\n>> But I wonder if moving of this to the calling site is the right thing to\n>> do.  Wouldn't it become much more contained and robust if you did it this\n>> way?\n> [...]\n>>  sub blob_mimetype {\n> \n> This _might_ be better.  I didn't do this for the following two reasons:\n[...]\n> So perhaps best of those ways would be to create thin wrapper subroutine,\n> blob_contenttype($fd, $file_name, $mimetype), where both $file_name and\n> (especially) $mimetype are optional parameters, and ise it in\n> git_blob_plain() subroutine...\n\nIt is now done this way.  IMHO it is a best solution.\n\n>> +\t# Type specific postprocessing can be added as needed...\n>> +\tif ($mime =~ /^text\\//i &&\n>> +\t    $mime !~ /charset=/i && $default_text_plain_charset) {\n>> +\t\t$mime .=  '; charset='.$default_text_plain_charset;\n>> +\t}\n>> +\n>> +\treturn $mime;\n> \n> I'm not sure about it.  I worry a bit about text/html, which can, and\n> usually do, contain charset info inside the document.  I'm not sure\n> what happens when charset information from HTTP headers contradict\n> charset information from presented file.  That's why I have limited\n> adding charset info purely to 'text/plain', not 'text/*' without\n> charset info already present.\n\nCurrently for the above reason gitweb adds charset info _only_\nfor 'text/plain' mimetype.\n\n gitweb/gitweb.perl |   29 ++++++++++++++++++++---------\n 1 files changed, 20 insertions(+), 9 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 57a1905..c6d43bf 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2481,8 +2481,7 @@ sub blob_mimetype {\n \treturn $default_blob_plain_mimetype unless $fd;\n \n \tif (-T $fd) {\n-\t\treturn 'text/plain' .\n-\t\t       ($default_text_plain_charset ? '; charset='.$default_text_plain_charset : '');\n+\t\treturn 'text/plain';\n \t} elsif (! $filename) {\n \t\treturn 'application/octet-stream';\n \t} elsif ($filename =~ m/\\.png$/i) {\n@@ -2496,6 +2495,17 @@ sub blob_mimetype {\n \t}\n }\n \n+sub blob_contenttype {\n+\tmy ($fd, $file_name, $type) = @_;\n+\n+\t$type ||= blob_mimetype($fd, $file_name);\n+\tif ($type eq 'text/plain' && defined $default_text_plain_charset) {\n+\t\t$type .= \"; charset=$default_text_plain_charset\";\n+\t}\n+\n+\treturn $type;\n+}\n+\n ## ======================================================================\n ## functions printing HTML: header, footer, error page\n \n@@ -4377,6 +4387,7 @@ sub git_heads {\n }\n \n sub git_blob_plain {\n+\tmy $type = shift;\n \tmy $expires;\n \n \tif (!defined $hash) {\n@@ -4392,13 +4403,13 @@ sub git_blob_plain {\n \t\t$expires = \"+1d\";\n \t}\n \n-\tmy $type = shift;\n \topen my $fd, \"-|\", git_cmd(), \"cat-file\", \"blob\", $hash\n-\t\tor die_error(undef, \"Couldn't cat $file_name, $hash\");\n+\t\tor die_error(undef, \"Open git-cat-file blob '$hash' failed\");\n \n-\t$type ||= blob_mimetype($fd, $file_name);\n+\t# content-type (can include charset)\n+\t$type = blob_contenttype($fd, $file_name, $type);\n \n-\t# save as filename, even when no $file_name is given\n+\t# \"save as\" filename, even when no $file_name is given\n \tmy $save_as = \"$hash\";\n \tif (defined $file_name) {\n \t\t$save_as = $file_name;\n@@ -4407,9 +4418,9 @@ sub git_blob_plain {\n \t}\n \n \tprint $cgi->header(\n-\t\t-type => \"$type\",\n-\t\t-expires=>$expires,\n-\t\t-content_disposition => 'inline; filename=\"' . \"$save_as\" . '\"');\n+\t\t-type => $type,\n+\t\t-expires => $expires,\n+\t\t-content_disposition => 'inline; filename=\"' . $save_as . '\"');\n \tundef $/;\n \tbinmode STDOUT, ':raw';\n \tprint <$fd>;\n-- \n1.5.5.3\n"}]}