{"thread":{"id":"36657","subject":"[PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","startedAt":"2014-05-14T18:41:45Z","lastAt":"2014-06-04T20:47:46Z","messageCount":21,"participants":["Michael Wagner","Junio C Hamano","Jakub Narębski","Peter Krefting"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"241520","messageId":"20140514184145.GA25699@localhost.localdomain","threadId":"36657","inReplyTo":null,"subject":"[PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Michael Wagner","fromEmail":"accounts@mwagner.org","sentAt":"2014-05-14T18:41:45Z","receivedAt":"2014-05-14T18:41:45Z","isPatch":true,"sender":{"key":"accounts@mwagner.org","avatar":null},"body":"Perl has an internal encoding used to store text strings. Currently, trying to\nview files with UTF-8 encoded names results in an error (either \"404 - Cannot\nfind file\" [blob_plain] or \"XML Parsing Error\" [blob]). Converting these UTF-8\nencoded file names into Perl's internal format resolves these errors.\n\nSigned-off-by: Michael Wagner \n---\n gitweb/gitweb.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a9f57d6..6046977 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {\n \t\t}\n \t}\n \n-\tour $file_name = $input_params{'file_name'};\n+\tour $file_name = decode(\"utf-8\", $input_params{'file_name'});\n \tif (defined $file_name) {\n \t\tif (!is_valid_pathname($file_name)) {\n \t\t\tdie_error(400, \"Invalid file parameter\");\n-- \n1.9.0\n"},{"id":"241588","messageId":"xmqqd2fghvlf.fsf@gitster.dls.corp.google.com","threadId":"36657","inReplyTo":"20140514184145.GA25699@localhost.localdomain","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-14T21:57:32Z","receivedAt":"2014-05-14T21:57:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Wagner writes:\n\n> Perl has an internal encoding used to store text strings. Currently, trying to\n> view files with UTF-8 encoded names results in an error (either \"404 - Cannot\n> find file\" [blob_plain] or \"XML Parsing Error\" [blob]). Converting these UTF-8\n> encoded file names into Perl's internal format resolves these errors.\n>\n> Signed-off-by: Michael Wagner \n> ---\n\nCc'ing Jakub, who have been the area maintainer, for comments.\n\nOne thing I wonder is that, if there are some additional calls to\nencode() necessary before we embed $file_name (which are now decoded\nto the internal string form, not a byte-sequence that happens to be\nin utf-8) in the generated pages, if we were to do this change.\n\n> gitweb/gitweb.perl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index a9f57d6..6046977 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {\n> \t\t}\n> \t}\n> \n> -\tour $file_name = $input_params{'file_name'};\n> +\tour $file_name = decode(\"utf-8\", $input_params{'file_name'});\n> \tif (defined $file_name) {\n> \t\tif (!is_valid_pathname($file_name)) {\n> \t\t\tdie_error(400, \"Invalid file parameter\");\n"},{"id":"241626","messageId":"CANQwDwdh1qQkYi9sB=22wbNnb+g5qv5prCzj2aWhHBbTZhVhdg@mail.gmail.com","threadId":"36657","inReplyTo":"xmqqd2fghvlf.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-14T22:25:45Z","receivedAt":"2014-05-14T22:25:45Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, May 14, 2014 at 11:57 PM, Junio C Hamano wrote:\n> Michael Wagner writes:\n>\n>> Perl has an internal encoding used to store text strings. Currently, trying to\n>> view files with UTF-8 encoded names results in an error (either \"404 - Cannot\n>> find file\" [blob_plain] or \"XML Parsing Error\" [blob]). Converting these UTF-8\n>> encoded file names into Perl's internal format resolves these errors.\n\nCould you give us an example? What is important is whether filename\nis passed via path_info or via query string.\n\nBecause in evaluate_uri() there is\n\n our $path_info = decode_utf8($ENV{\"PATH_INFO\"});\n\nand in evaluate_query_params() there is\n\n $input_params{$name} = decode_utf8($cgi->param($symbol));\n\n>> Signed-off-by: Michael Wagner \n>> ---\n>\n> Cc'ing Jakub, who have been the area maintainer, for comments.\n>\n> One thing I wonder is that, if there are some additional calls to\n> encode() necessary before we embed $file_name (which are now decoded\n> to the internal string form, not a byte-sequence that happens to be\n> in utf-8) in the generated pages, if we were to do this change.\n\nThere should be no problem with output encoding. esc_path(), which\nshould be used for filenames, includes to_utf8, which in turn uses\ndecode($fallback_encoding, $str, Encode::FB_DEFAULT);\n\n>> gitweb/gitweb.perl | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index a9f57d6..6046977 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {\n>> }\n>> }\n>>\n>> - our $file_name = $input_params{'file_name'};\n>> + our $file_name = decode(\"utf-8\", $input_params{'file_name'});\n>> if (defined $file_name) {\n>> if (!is_valid_pathname($file_name)) {\n>> die_error(400, \"Invalid file parameter\");\n\nHmm... all %input_params should have been properly decoded\nalready, how it was missed?\n\nAlso, branchname (hash_base etc.), search query, filename in file_parent,\nproject name can be UTF-8 too, so it is at best partial fix.\n\n-- \nJakub Narębski\n"},{"id":"241642","messageId":"20140515050820.GA30785@localhost.localdomain","threadId":"36657","inReplyTo":"CANQwDwdh1qQkYi9sB=22wbNnb+g5qv5prCzj2aWhHBbTZhVhdg@mail.gmail.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Michael Wagner","fromEmail":"accounts@mwagner.org","sentAt":"2014-05-15T05:08:20Z","receivedAt":"2014-05-15T05:08:20Z","isPatch":true,"sender":{"key":"accounts@mwagner.org","avatar":null},"body":"On Thu, May 15, 2014 at 12:25:45AM +0200, Jakub Narębski wrote:\n> On Wed, May 14, 2014 at 11:57 PM, Junio C Hamano wrote:\n> > Michael Wagner writes:\n> >\n> >> Perl has an internal encoding used to store text strings. Currently, trying to\n> >> view files with UTF-8 encoded names results in an error (either \"404 - Cannot\n> >> find file\" [blob_plain] or \"XML Parsing Error\" [blob]). Converting these UTF-8\n> >> encoded file names into Perl's internal format resolves these errors.\n> \n> Could you give us an example? What is important is whether filename\n> is passed via path_info or via query string.\n> \n\nThere is a file named \"Gütekriterien.txt\" in my repository. Trying to\nview this file as \"blob_plain\" produces an 404 error (displaying the\nfile name with an additional print statement):\n\n$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi\n\nwork/Gütekriterien.txt\nStatus: 404 Not Found\n\nDecoding the UTF-8 encoded file name (again with an additional print\nstatement):\n\n$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi\n\nwork/Gütekriterien.txt\nContent-disposition: inline; filename=\"work/Gütekriterien.txt\"\n\n> Because in evaluate_uri() there is\n> \n> our $path_info = decode_utf8($ENV{\"PATH_INFO\"});\n> \n> and in evaluate_query_params() there is\n> \n> $input_params{$name} = decode_utf8($cgi->param($symbol));\n> \n> >> Signed-off-by: Michael Wagner \n> >> ---\n> >\n> > Cc'ing Jakub, who have been the area maintainer, for comments.\n> >\n> > One thing I wonder is that, if there are some additional calls to\n> > encode() necessary before we embed $file_name (which are now decoded\n> > to the internal string form, not a byte-sequence that happens to be\n> > in utf-8) in the generated pages, if we were to do this change.\n\nThe generated pages show the correct file names. \n\n> \n> There should be no problem with output encoding. esc_path(), which\n> should be used for filenames, includes to_utf8, which in turn uses\n> decode($fallback_encoding, $str, Encode::FB_DEFAULT);\n> \n> >> gitweb/gitweb.perl | 2 +-\n> >> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >>\n> >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> >> index a9f57d6..6046977 100755\n> >> --- a/gitweb/gitweb.perl\n> >> +++ b/gitweb/gitweb.perl\n> >> @@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {\n> >> }\n> >> }\n> >>\n> >> - our $file_name = $input_params{'file_name'};\n> >> + our $file_name = decode(\"utf-8\", $input_params{'file_name'});\n> >> if (defined $file_name) {\n> >> if (!is_valid_pathname($file_name)) {\n> >> die_error(400, \"Invalid file parameter\");\n> \n> Hmm... all %input_params should have been properly decoded\n> already, how it was missed?\n> \n> Also, branchname (hash_base etc.), search query, filename in file_parent,\n> project name can be UTF-8 too, so it is at best partial fix.\n> \n> -- \n> Jakub Narębski\n"},{"id":"241660","messageId":"alpine.DEB.2.00.1405150957520.10221@ds9.cixit.se","threadId":"36657","inReplyTo":"20140515050820.GA30785@localhost.localdomain","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2014-05-15T09:04:24Z","receivedAt":"2014-05-15T09:04:24Z","isPatch":true,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"Michael Wagner:\n\n> Decoding the UTF-8 encoded file name (again with an additional print\n> statement):\n>\n> $ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi\n>\n> work/Gütekriterien.txt\n> Content-disposition: inline; filename=\"work/Gütekriterien.txt\"\n\nYou should fix the code path that created that URI, though, as it is \nnot what you expected.\n\n%C3%83 decodes to U+00C3 Latin Capital Letter A With Tilde\n%C2%BC decodes to U+00BC Vulgar Graction One Quarter\n\nThe proper UTF-8 encoding for ü (U+00FC) is, as you can probably guess from \nlooking at which two characters the sequence above yielded, C3 BC, \nwhich in a URI is represented as %C3%BC.\n\nYour QUERY_STRING should thus be\n\n p=notes.git;a=blob_plain;f=work/G%C3%BCtekriterien.txt;hb=HEAD\n\nwhich probably works as expected.\n\nWhat is happening is that whatever is generating the URI us \nUTF-8-encoding the string twice (i.e., it generates a string with the \nproper C3 BC in it, and then interprets it as iso-8859-1 data and runs \nthat through a UTF-8 encoder again, yielding the C3 83 C2 BC sequence \nyou see above).\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"241662","messageId":"CANQwDwes_G-aNMo=UfGJk+Dk2YaokQsG_VLaVQyPevEDchWTVA@mail.gmail.com","threadId":"36657","inReplyTo":"20140515050820.GA30785@localhost.localdomain","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-15T12:32:33Z","receivedAt":"2014-05-15T12:32:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, May 15, 2014 at 7:08 AM, Michael Wagner wrote:\n> On Thu, May 15, 2014 at 12:25:45AM +0200, Jakub Narębski wrote:\n>> On Wed, May 14, 2014 at 11:57 PM, Junio C Hamano wrote:\n>>> Michael Wagner writes:\n>>>\n>>>> Perl has an internal encoding used to store text strings. Currently, trying to\n>>>> view files with UTF-8 encoded names results in an error (either \"404 - Cannot\n>>>> find file\" [blob_plain] or \"XML Parsing Error\" [blob]). Converting these UTF-8\n>>>> encoded file names into Perl's internal format resolves these errors.\n>>\n>> Could you give us an example? What is important is whether filename\n>> is passed via path_info or via query string.\n>>\n>\n> There is a file named \"Gütekriterien.txt\" in my repository. Trying to\n> view this file as \"blob_plain\" produces an 404 error (displaying the\n> file name with an additional print statement):\n>\n> $ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi\n>\n> work/Gütekriterien.txt\n> Status: 404 Not Found\n\nYou have URI encoding of \"ü\" wrong! \"ü\" encodes as %C3%BC, not\nas %C3%83%C2%BC (4 bytes?)\n\n http://www.url-encode-decode.com/\n\nYou tested with wrong input.\n\nBTW. there probably should be test for UTF-8 encoding, similar to\nthe one for XSS in t9502-gitweb-standalone-parse-output\n-- \nJakub Narębski\n"},{"id":"241703","messageId":"xmqq38gbgdkk.fsf@gitster.dls.corp.google.com","threadId":"36657","inReplyTo":"alpine.DEB.2.00.1405150957520.10221@ds9.cixit.se","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-15T17:24:27Z","receivedAt":"2014-05-15T17:24:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Krefting writes:\n\n> What is happening is that whatever is generating the URI us\n> UTF-8-encoding the string twice (i.e., it generates a string with the\n> proper C3 BC in it, and then interprets it as iso-8859-1 data and runs\n> that through a UTF-8 encoder again, yielding the C3 83 C2 BC sequence\n> you see above).\n\nThanks for a quick response. If the input was unnecessarily encoded\none extra time, it is no wonder it needed one unnecessary extra\ndecoding.\n"},{"id":"241761","messageId":"20140515184808.GA7964@localhost.localdomain","threadId":"36657","inReplyTo":"alpine.DEB.2.00.1405150957520.10221@ds9.cixit.se","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Michael Wagner","fromEmail":"accounts@mwagner.org","sentAt":"2014-05-15T18:48:09Z","receivedAt":"2014-05-15T18:48:09Z","isPatch":true,"sender":{"key":"accounts@mwagner.org","avatar":null},"body":"On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:\n> Michael Wagner:\n> \n> >Decoding the UTF-8 encoded file name (again with an additional print\n> >statement):\n> >\n> >$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi\n> >\n> >work/Gütekriterien.txt\n> >Content-disposition: inline; filename=\"work/Gütekriterien.txt\"\n> \n> You should fix the code path that created that URI, though, as it is not\n> what you expected.\n> \n> %C3%83 decodes to U+00C3 Latin Capital Letter A With Tilde\n> %C2%BC decodes to U+00BC Vulgar Graction One Quarter\n> \n> The proper UTF-8 encoding for ü (U+00FC) is, as you can probably guess from\n> looking at which two characters the sequence above yielded, C3 BC, which in\n> a URI is represented as %C3%BC.\n> \n> Your QUERY_STRING should thus be\n> \n> p=notes.git;a=blob_plain;f=work/G%C3%BCtekriterien.txt;hb=HEAD\n> \n> which probably works as expected.\n\nObviously, you are right, thanks.\n\n> \n> What is happening is that whatever is generating the URI us UTF-8-encoding\n> the string twice (i.e., it generates a string with the proper C3 BC in it,\n> and then interprets it as iso-8859-1 data and runs that through a UTF-8\n> encoder again, yielding the C3 83 C2 BC sequence you see above).\n> \n\nThe subroutine \"git tree\" generates the tree view. It stores the output\nof \"git ls-tree -z ...\" in an array named \"@entries\". Printing the content\nof this array yields the following result:\n\n00644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a 520 Gütekriterien.txt\n\nThis leads to the \"doubled\" encoding. Declaring the encoding in the call\nto open yields the following result:\n\n100644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a 520 Gütekriterien.txt\n\n---\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a9f57d6..f1414e1 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -7138,7 +7138,7 @@ sub git_tree {\n my @entries = ();\n {\n local $/ = \"\\0\";\n- open my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z',\n+ open my $fd, \"-|encoding(UTF-8)\", git_cmd(), \"ls-tree\", '-z',\n ($show_sizes ? '-l' : ()), @extra_options, $hash\n or die_error(500, \"Open git-ls-tree failed\");\n @entries = map { chomp; $_ } <$fd>;\n\n> -- \n> \\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"241772","messageId":"CANQwDwe+GJ+yAYWdVfMaHq97zGXBoepCfUdLiaQD9LFoz3SiOA@mail.gmail.com","threadId":"36657","inReplyTo":"20140515184808.GA7964@localhost.localdomain","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-15T19:28:02Z","receivedAt":"2014-05-15T19:28:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, May 15, 2014 at 8:48 PM, Michael Wagner wrote:\n> On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:\n>> Michael Wagner:\n>>\n>>>Decoding the UTF-8 encoded file name (again with an additional print\n>>>statement):\n>>>\n>>>$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi\n>>>\n>>>work/Gütekriterien.txt\n>>>Content-disposition: inline; filename=\"work/Gütekriterien.txt\"\n>>\n>> You should fix the code path that created that URI, though, as it is not\n>> what you expected.\n>>\n>> %C3%83 decodes to U+00C3 Latin Capital Letter A With Tilde\n>> %C2%BC decodes to U+00BC Vulgar Graction One Quarter\n>>\n>> The proper UTF-8 encoding for ü (U+00FC) is, as you can probably guess from\n>> looking at which two characters the sequence above yielded, C3 BC, which in\n>> a URI is represented as %C3%BC.\n>>\n>> Your QUERY_STRING should thus be\n>>\n>> p=notes.git;a=blob_plain;f=work/G%C3%BCtekriterien.txt;hb=HEAD\n>>\n>> which probably works as expected.\n>>\n>> What is happening is that whatever is generating the URI us UTF-8-encoding\n>> the string twice (i.e., it generates a string with the proper C3 BC in it,\n>> and then interprets it as iso-8859-1 data and runs that through a UTF-8\n>> encoder again, yielding the C3 83 C2 BC sequence you see above).\n>\n> The subroutine \"git tree\" generates the tree view. It stores the output\n> of \"git ls-tree -z ...\" in an array named \"@entries\". Printing the content\n> of this array yields the following result:\n>\n> 00644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a 520 Gütekriterien.txt\n>\n> This leads to the \"doubled\" encoding. Declaring the encoding in the call\n> to open yields the following result:\n>\n> 100644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a 520 Gütekriterien.txt\n\nGood catch.\n\nWriting test for this would not be easy, and require some HTML\nparser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,\n... or low level HTML::TreeBuilder, or other low level parser).\n\n> ---\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index a9f57d6..f1414e1 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -7138,7 +7138,7 @@ sub git_tree {\n> my @entries = ();\n> {\n> local $/ = \"\\0\";\n> - open my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z',\n> + open my $fd, \"-|encoding(UTF-8)\", git_cmd(), \"ls-tree\", '-z',\n> ($show_sizes ? '-l' : ()), @extra_options, $hash\n> or die_error(500, \"Open git-ls-tree failed\");\n\nOr put\n\n binmode $fd, ':utf8';\n\nlike in the rest of the code.\n\n> @entries = map { chomp; $_ } <$fd>;\n>\n\nEven better solution would be to use\n\n use open IN => ':encoding(utf-8)';\n\nat the beginning of gitweb.perl, once and for all.\n\nUnfortunately the output equivalent requires creating Perl\nmodule for gitweb, to be able to use\n\n use open OUT => ':encoding(utf-8-with-fallback)';\n\n-- \nJakub Narebski\n"},{"id":"241774","messageId":"CANQwDweFGwt5w2bJ1CesC50AAiJXBDT8uxuBEpFZ+nLbQaA_VQ@mail.gmail.com","threadId":"36657","inReplyTo":"CANQwDwe+GJ+yAYWdVfMaHq97zGXBoepCfUdLiaQD9LFoz3SiOA@mail.gmail.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-15T19:37:39Z","receivedAt":"2014-05-15T19:37:39Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, May 15, 2014 at 9:28 PM, Jakub Narębski wrote:\n> On Thu, May 15, 2014 at 8:48 PM, Michael Wagner wrote:\n[...]\n>> The subroutine \"git tree\" generates the tree view. It stores the output\n>> of \"git ls-tree -z ...\" in an array named \"@entries\". Printing the content\n>> of this array yields the following result:\n>>\n>> 00644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a 520 Gütekriterien.txt\n>>\n>> This leads to the \"doubled\" encoding. Declaring the encoding in the call\n>> to open yields the following result:\n>>\n>> 100644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a 520 Gütekriterien.txt\n\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index a9f57d6..f1414e1 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -7138,7 +7138,7 @@ sub git_tree {\n>> my @entries = ();\n>> {\n>> local $/ = \"\\0\";\n>> - open my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z',\n>> + open my $fd, \"-|encoding(UTF-8)\", git_cmd(), \"ls-tree\", '-z',\n>> ($show_sizes ? '-l' : ()), @extra_options, $hash\n>> or die_error(500, \"Open git-ls-tree failed\");\n>\n> Or put\n>\n> binmode $fd, ':utf8';\n>\n> like in the rest of the code.\n>\n>> @entries = map { chomp; $_ } <$fd>;\n\nThough to be exact there isn't any mechanism that ensures that\nfilenames in tree objects use utf-8 encoding, so perhaps a safer\nsolution would be to use\n\n to_utf8($file_name)\n\n(which respects $fallback_encoding) in appropriate places.\n\n-- \nJakub Narębski\n"},{"id":"241777","messageId":"xmqqmweiessl.fsf@gitster.dls.corp.google.com","threadId":"36657","inReplyTo":"CANQwDwe+GJ+yAYWdVfMaHq97zGXBoepCfUdLiaQD9LFoz3SiOA@mail.gmail.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-15T19:38:34Z","receivedAt":"2014-05-15T19:38:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n> Writing test for this would not be easy, and require some HTML\n> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,\n> ... or low level HTML::TreeBuilder, or other low level parser).\n\nHmph. Is it more than just looking for a specific run of %xx we\nwould expect to see in the output of the tree view for a repository\nin which there is one tree with non-ASCII name?\n\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index a9f57d6..f1414e1 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -7138,7 +7138,7 @@ sub git_tree {\n>> my @entries = ();\n>> {\n>> local $/ = \"\\0\";\n>> - open my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z',\n>> + open my $fd, \"-|encoding(UTF-8)\", git_cmd(), \"ls-tree\", '-z',\n>> ($show_sizes ? '-l' : ()), @extra_options, $hash\n>> or die_error(500, \"Open git-ls-tree failed\");\n>\n> Or put\n>\n> binmode $fd, ':utf8';\n>\n> like in the rest of the code.\n\nI expect a patch to do so and can forget about this thread myself,\nthen, OK?\n\nThanks all for digging this to the root.\n"},{"id":"241782","messageId":"CANQwDwffdbqD96OadyECFs=6WY_t+_0b63L5yAZVQ8aXrMvHHA@mail.gmail.com","threadId":"36657","inReplyTo":"xmqqmweiessl.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-15T20:45:22Z","receivedAt":"2014-05-15T20:45:22Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, May 15, 2014 at 9:38 PM, Junio C Hamano wrote:\n> Jakub Narębski writes:\n>\n>> Writing test for this would not be easy, and require some HTML\n>> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,\n>> ... or low level HTML::TreeBuilder, or other low level parser).\n>\n> Hmph. Is it more than just looking for a specific run of %xx we\n> would expect to see in the output of the tree view for a repository\n> in which there is one tree with non-ASCII name?\n\nThere is if we want to check (in non-fragile way) that said\nspecific run is in 'href' *attribute* of 'a' element (link target).\n\n-- \nJakub Narebski\n"},{"id":"241851","messageId":"xmqqmweibjjo.fsf@gitster.dls.corp.google.com","threadId":"36657","inReplyTo":"CANQwDwffdbqD96OadyECFs=6WY_t+_0b63L5yAZVQ8aXrMvHHA@mail.gmail.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T01:26:35Z","receivedAt":"2014-05-16T01:26:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n> On Thu, May 15, 2014 at 9:38 PM, Junio C Hamano wrote:\n>> Jakub Narębski writes:\n>>\n>>> Writing test for this would not be easy, and require some HTML\n>>> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,\n>>> ... or low level HTML::TreeBuilder, or other low level parser).\n>>\n>> Hmph. Is it more than just looking for a specific run of %xx we\n>> would expect to see in the output of the tree view for a repository\n>> in which there is one tree with non-ASCII name?\n>\n> There is if we want to check (in non-fragile way) that said\n> specific run is in 'href' *attribute* of 'a' element (link target).\n\nCorrect, but is \"where does it appear\" the question we are\nprimarily interested in, wrt this breakage and its fix?\n\nIf gitweb output has some volatile parts that do not depend on the\ncontents of the Git test repository (e.g. showing contents of\n/etc/motd, date/time of when the test was run, or the full pathname\nleading to the trash directory), then preparing a tree whose name is\näéìõû and making sure that the properly encoded version of äéìõû\nappears anywhere in the output may not be sufficient to validate\nthat we got the encoding right, as that string may appear in the\nparts that are totally unrelated to the contents being shown and not\nunder our control. But is that really the case?\n\nAlso we may introduce a bug and misspell the attr name and produce\nan anchor element with hpef attribute with the properly encoded URL\nin it, and your \"parse HTML properly\" approach would catch it, but\nis that the kind of breakage under discussion? You hinted at new\ntests for UTF-8 encoding in the other message in the thread earlier,\nand I've been assuming that we were talking about the encoding test,\nnot a test to catch s/href/hpef/ kind of breakage.\n"},{"id":"241855","messageId":"CANQwDwe8Eb+ORiRyuq3+kKw72Jath_DGySmws1Rvt8bmuHoXVw@mail.gmail.com","threadId":"36657","inReplyTo":"xmqqmweibjjo.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-16T07:54:58Z","receivedAt":"2014-05-16T07:54:58Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, May 16, 2014 at 3:26 AM, Junio C Hamano wrote:\n> Jakub Narębski writes:\n>> On Thu, May 15, 2014 at 9:38 PM, Junio C Hamano wrote:\n>>> Jakub Narębski writes:\n>>>\n>>>> Writing test for this would not be easy, and require some HTML\n>>>> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,\n>>>> ... or low level HTML::TreeBuilder, or other low level parser).\n>>>\n>>> Hmph. Is it more than just looking for a specific run of %xx we\n>>> would expect to see in the output of the tree view for a repository\n>>> in which there is one tree with non-ASCII name?\n>>\n>> There is if we want to check (in non-fragile way) that said\n>> specific run is in 'href' *attribute* of 'a' element (link target).\n>\n> Correct, but is \"where does it appear\" the question we are\n> primarily interested in, wrt this breakage and its fix?\n\nThat of course depends on how we want to test gitweb output.\nThe simplest solution, comparing with known output with perhaps\nfragile / variable elements masked out could be done quickly...\nbut changes in output (even if they don't change functionality,\nor don't change visible output) require regenerating test cases\n(expected output) to test against - which might be source of\nerrors in test suite.\n\nAnother simple solution, grepping for expected strings, also\neasy to create, has the disadvantage of being only positive\ntest - you cannot [easily] test that there are no *wrong* output,\nonly that right string exists somewhere.\n\n> If gitweb output has some volatile parts that do not depend on the\n> contents of the Git test repository (e.g. showing contents of\n> /etc/motd, date/time of when the test was run, or the full pathname\n> leading to the trash directory), then preparing a tree whose name is\n> äéìõû and making sure that the properly encoded version of äéìõû\n> appears anywhere in the output may not be sufficient to validate\n> that we got the encoding right, as that string may appear in the\n> parts that are totally unrelated to the contents being shown and not\n> under our control. But is that really the case?\n\nWell, I guess that any test is better than no test (though OTOH\nHeartbleed and \"goto fail\" bugs shows the importance of negative\ntests).\n\n> Also we may introduce a bug and misspell the attr name and produce\n> an anchor element with hpef attribute with the properly encoded URL\n> in it, and your \"parse HTML properly\" approach would catch it, but\n> is that the kind of breakage under discussion? You hinted at new\n> tests for UTF-8 encoding in the other message in the thread earlier,\n> and I've been assuming that we were talking about the encoding test,\n> not a test to catch s/href/hpef/ kind of breakage.\n\nOne of tests possible with HTML parser (e.g. WWW::Mechanize::CGI)\nis to check that all [internal] links leads to 200-OK pages, which\naccidentally would also be a test against this breakage.\n\n-- \nJakub Narebski\n"},{"id":"241990","messageId":"xmqq4n0pbqnc.fsf@gitster.dls.corp.google.com","threadId":"36657","inReplyTo":"CANQwDwe8Eb+ORiRyuq3+kKw72Jath_DGySmws1Rvt8bmuHoXVw@mail.gmail.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T17:05:26Z","receivedAt":"2014-05-16T17:05:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n>> Correct, but is \"where does it appear\" the question we are\n>> primarily interested in, wrt this breakage and its fix?\n>\n> That of course depends on how we want to test gitweb output.\n> The simplest solution, comparing with known output with perhaps\n> fragile / variable elements masked out could be done quickly...\n> but changes in output (even if they don't change functionality,\n> or don't change visible output) require regenerating test cases\n> (expected output) to test against - which might be source of\n> errors in test suite.\n\nI agree with your \"to test it fully, we need extra dependencies\",\nbut my point is that it does not have to be a full \"HTML-validating,\npicking the expected attribute via XPATH matching\" kind of test if\nwhat we want is only to add a new test to protect this particular\nfix from future breakages.\n\nFor example, I think it is sufficient to grep for 'href=\"...%xx%xx\"'\nin the output after preparing a sample tree with one entry to show.\nThe expected substring either exists (in which case we got it\nright), or it doesn't (in which case we are showing garbage). Of\ncourse that depends on the assumption that its output is not too\nheavily contaminated with volatile parts outside our control, as I\nalready mentioned in the message you are responding to.\n\nBut it all depends on \"if\" we wanted to add a new test ;-)\n"},{"id":"241984","messageId":"CAPc5daUzcQvPCY8UQE4-OuNOswKxTMBNTddGV9YWXwONXoV3Qg@mail.gmail.com","threadId":"36657","inReplyTo":"CANQwDwe8Eb+ORiRyuq3+kKw72Jath_DGySmws1Rvt8bmuHoXVw@mail.gmail.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T18:17:18Z","receivedAt":"2014-05-16T18:17:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"(sorry if you receive a dup; pobox.com seems to be constipated right now)\n\nJakub Narębski writes:\n\n>> Correct, but is \"where does it appear\" the question we are\n>> primarily interested in, wrt this breakage and its fix?\n>\n> That of course depends on how we want to test gitweb output.\n> The simplest solution, comparing with known output with perhaps\n> fragile / variable elements masked out could be done quickly...\n> but changes in output (even if they don't change functionality,\n> or don't change visible output) require regenerating test cases\n> (expected output) to test against - which might be source of\n> errors in test suite.\n\nI agree with your \"to test it fully, we need extra dependencies\",\nbut my point is that it does not have to be a full \"HTML-validating,\npicking the expected attribute via XPATH matching\" kind of test if\nwhat we want is only to add a new test to protect this particular\nfix from future breakages.\n\nFor example, I think it is sufficient to grep for 'href=\"...%xx%xx\"'\nin the output after preparing a sample tree with one entry to show.\nThe expected substring either exists (in which case we got it\nright), or it doesn't (in which case we are showing garbage). Of\ncourse that depends on the assumption that its output is not too\nheavily contaminated with volatile parts outside our control, as I\nalready mentioned in the message you are responding to.\n\nBut it all depends on \"if\" we wanted to add a new test ;-)\n"},{"id":"242747","messageId":"53849EC9.1010304@gmail.com","threadId":"36657","inReplyTo":"xmqq4n0pbqnc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-27T14:18:49Z","receivedAt":"2014-05-27T14:18:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 2014-05-16 19:05, Junio C Hamano pisze:\n> Jakub Narębski writes:\n> \n>>> Correct, but is \"where does it appear\" the question we are\n>>> primarily interested in, wrt this breakage and its fix?\n>>\n>> That of course depends on how we want to test gitweb output.\n>> The simplest solution, comparing with known output with perhaps\n>> fragile / variable elements masked out could be done quickly...\n>> but changes in output (even if they don't change functionality,\n>> or don't change visible output) require regenerating test cases\n>> (expected output) to test against - which might be source of\n>> errors in test suite.\n> \n> I agree with your \"to test it fully, we need extra dependencies\",\n> but my point is that it does not have to be a full \"HTML-validating,\n> picking the expected attribute via XPATH matching\" kind of test if\n> what we want is only to add a new test to protect this particular\n> fix from future breakages.\n> \n> For example, I think it is sufficient to grep for 'href=\"...%xx%xx\"'\n> in the output after preparing a sample tree with one entry to show.\n> The expected substring either exists (in which case we got it\n> right), or it doesn't (in which case we are showing garbage). Of\n> course that depends on the assumption that its output is not too\n> heavily contaminated with volatile parts outside our control, as I\n> already mentioned in the message you are responding to.\n> \n> But it all depends on \"if\" we wanted to add a new test ;-)\n\nI tried to add such simple test to t9502, but instead of tests\nfailing with current version, the test setup fails but succeeds\n(i.e. test library says that it failed, but manual examination\nshows that everything is O.K.).\n\n-- >8 --\nFrom: Jakub Narebski \nSubject: [PATCH/RFC] gitweb test: Test proper encoding of non US-ASCII filenames in output (WIP)\n\nThis t9502 test is intended to test for proper encoding of non\nUS-ASCII filenames (i.e. UTF-8 filenames) in generated links (which\nneed some form of URI encoding) and in generated HTML (which needs\nHTML encoding / escaping).\n\nFor now it tests only 'tree' view (though incidentally it also tests\nUTF-8 in commit subject), as this was the action where reportedly\nthere was bug in link encoding: $t{'name'} coming from the\n\"git ls-tree -z ...\" command via @ntries array was not marked as\nUTF-8, making Perl assume that it is in internal Perl format\ni.e. iso-8859-1 encoding and URI-escaping it as if it was in\niso-8859-1 encoding (e.g. \"Gütekriterien.txt\" in UTF-8 is\n\"Gütekriterien.txt\" if treated as iso-8859-1, and it then\nencodes to \"G%C3%83%C2%BCtekriterien.txt\" instead of correct\n\"G%C3%BCtekriterien.txt\").\n\nUNFORTUNATELY test does not fail as it should, even though the issue\nwas not fixed... OTOH it fails in setup though it is successful.\n\nReported-by: Michael Wagner \nSigned-off-by: Jakub Narębski \n---\n t/t9502-gitweb-standalone-parse-output.sh | 34 +++++++++++++++++++++++++++++\n 1 files changed, 34 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\nindex 86dfee2..37246a3 100755\n--- a/t/t9502-gitweb-standalone-parse-output.sh\n+++ b/t/t9502-gitweb-standalone-parse-output.sh\n@@ -201,4 +201,38 @@ test_expect_success 'xss checks' '\n \txss \"a=rss&p=foo.git&f=$TAG\"\n '\n \n+link_check () {\n+\tgrep -F \"%3C__%C2%A3%C3%A5%C3%AB%C3%AE%C3%B1%C3%B2%C3%BB%C3%BD%C2%B6\" \\\n+\t\tgitweb.body &&\n+\t! grep -F \"%3C__%A3%E5%EB%EE%F1%F2%FB%FD%B6\" \\\n+\t\tgitweb.body\n+}\n+\n+test_expect_success 'prepare UTF-8 output tests' '\n+\tFILENAME=\"<__£åëîñòûý¶ +;?&__>\" &&\n+\ttest_commit \"Adding $FILENAME\" \"$FILENAME\" \"$FILENAME contents\"\n+'\n+\n+test_expect_success 'check URI-escaped UTF-8 filename in query-params link' '\n+\tcat >>gitweb_config.perl <<-\\EOF &&\n+\t$feature{\"pathinfo\"}{\"default\"} = [0];\n+\tEOF\n+\tgitweb_run \"p=.git;a=tree\" &&\n+\tlink_check\n+'\n+\n+test_expect_success 'check URI-escaped UTF-8 filename in path_info link' '\n+\tcat >>gitweb_config.perl <<-\\EOF &&\n+\t$feature{\"pathinfo\"}{\"default\"} = [1];\n+\tEOF\n+\tgitweb_run \"\" \"/.git/tree\" &&\n+\tlink_check\n+'\n+\n+test_expect_success 'check HTML-escaped UTF-8 filename in body' '\n+\tgitweb_run \"p=.git;a=tree\" &&\n+\tgrep -F \"<__£åëîñòûý¶ +;?&__>\" gitweb.body &&\n+\t! grep -F \"<__£åëîñòûý¶ +;?&__>\" gitweb.body\n+'\n+\n test_done\n-- \n1.7.1\n\n\n \n"},{"id":"242748","messageId":"53849FB2.7000701@gmail.com","threadId":"36657","inReplyTo":"CANQwDwe+GJ+yAYWdVfMaHq97zGXBoepCfUdLiaQD9LFoz3SiOA@mail.gmail.com","subject":"[PATCH] gitweb: Harden UTF-8 handling in generated links","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-05-27T14:22:42Z","receivedAt":"2014-05-27T14:22:42Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 2014-05-15 21:28, Jakub Narębski pisze:\n> On Thu, May 15, 2014 at 8:48 PM, Michael Wagner wrote:\n>> On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:\n>>> Michael Wagner:\n\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index a9f57d6..f1414e1 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -7138,7 +7138,7 @@ sub git_tree {\n>> my @entries = ();\n>> {\n>> local $/ = \"\\0\";\n>> - open my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z',\n>> + open my $fd, \"-|encoding(UTF-8)\", git_cmd(), \"ls-tree\", '-z',\n>> ($show_sizes ? '-l' : ()), @extra_options, $hash\n>> or die_error(500, \"Open git-ls-tree failed\");\n> \n> Or put\n> \n> binmode $fd, ':utf8';\n> \n> like in the rest of the code.\n> \n>> @entries = map { chomp; $_ } <$fd>;\n>>\n> \n> Even better solution would be to use\n> \n> use open IN => ':encoding(utf-8)';\n> \n> at the beginning of gitweb.perl, once and for all.\n\nOr harden esc_param / esc_path_info the same way esc_html\nis hardened against missing ':utf8' flag.\n\n-- >8 -- \nSubject: [PATCH] gitweb: Harden UTF-8 handling in generated links\n\nesc_html() ensures that its input is properly UTF-8 encoded and marked\nas UTF-8 with to_utf8(). Make esc_param() (used for query parameters\nin generated URLs), esc_path_info() (for escaping path_info\ncomponents) and esc_url() use it too.\n\nThis hardens gitweb against errors in UTF-8 handling; because\nto_utf8() is idempotent it won't change correct output.\n\nReported-by: Michael Wagner \nSigned-off-by: Jakub Narębski \n---\n gitweb/gitweb.perl | 7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a9f57d6..77e1312 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1548,8 +1548,11 @@ sub to_utf8 {\n sub esc_param {\n \tmy $str = shift;\n \treturn undef unless defined $str;\n+\n+\t$str = to_utf8($str);\n \t$str =~ s/([^A-Za-z0-9\\-_.~()\\/:@ ]+)/CGI::escape($1)/eg;\n \t$str =~ s/ /\\+/g;\n+\n \treturn $str;\n }\n \n@@ -1558,6 +1561,7 @@ sub esc_path_info {\n \tmy $str = shift;\n \treturn undef unless defined $str;\n \n+\t$str = to_utf8($str);\n \t# path_info doesn't treat '+' as space (specially), but '?' must be escaped\n \t$str =~ s/([^A-Za-z0-9\\-_.~();\\/;:@&= +]+)/CGI::escape($1)/eg;\n \n@@ -1568,8 +1572,11 @@ sub esc_path_info {\n sub esc_url {\n \tmy $str = shift;\n \treturn undef unless defined $str;\n+\n+\t$str = to_utf8($str);\n \t$str =~ s/([^A-Za-z0-9\\-_.~();\\/;?:@&= ]+)/CGI::escape($1)/eg;\n \t$str =~ s/ /\\+/g;\n+\n \treturn $str;\n }\n \n-- \n1.7.1\n"},{"id":"243306","messageId":"20140604154128.GA28549@localhost.localdomain","threadId":"36657","inReplyTo":"53849FB2.7000701@gmail.com","subject":"Re: [PATCH] gitweb: Harden UTF-8 handling in generated links","fromName":"Michael Wagner","fromEmail":"mail@mwagner.org","sentAt":"2014-06-04T15:41:29Z","receivedAt":"2014-06-04T15:41:29Z","isPatch":true,"sender":{"key":"mail@mwagner.org","avatar":null},"body":"On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote:\n> W dniu 2014-05-15 21:28, Jakub Narębski pisze:\n> > On Thu, May 15, 2014 at 8:48 PM, Michael Wagner wrote:\n> >> On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:\n> >>> Michael Wagner:\n> \n> >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> >> index a9f57d6..f1414e1 100755\n> >> --- a/gitweb/gitweb.perl\n> >> +++ b/gitweb/gitweb.perl\n> >> @@ -7138,7 +7138,7 @@ sub git_tree {\n> >> my @entries = ();\n> >> {\n> >> local $/ = \"\\0\";\n> >> - open my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z',\n> >> + open my $fd, \"-|encoding(UTF-8)\", git_cmd(), \"ls-tree\", '-z',\n> >> ($show_sizes ? '-l' : ()), @extra_options, $hash\n> >> or die_error(500, \"Open git-ls-tree failed\");\n> > \n> > Or put\n> > \n> > binmode $fd, ':utf8';\n> > \n> > like in the rest of the code.\n> > \n> >> @entries = map { chomp; $_ } <$fd>;\n> >>\n> > \n> > Even better solution would be to use\n> > \n> > use open IN => ':encoding(utf-8)';\n> > \n> > at the beginning of gitweb.perl, once and for all.\n> \n> Or harden esc_param / esc_path_info the same way esc_html\n> is hardened against missing ':utf8' flag.\n> \n> -- >8 -- \n> Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links\n> \n> esc_html() ensures that its input is properly UTF-8 encoded and marked\n> as UTF-8 with to_utf8(). Make esc_param() (used for query parameters\n> in generated URLs), esc_path_info() (for escaping path_info\n> components) and esc_url() use it too.\n> \n> This hardens gitweb against errors in UTF-8 handling; because\n> to_utf8() is idempotent it won't change correct output.\n> \n> Reported-by: Michael Wagner \n> Signed-off-by: Jakub Narębski \n> ---\n> gitweb/gitweb.perl | 7 +++++++\n> 1 files changed, 7 insertions(+), 0 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index a9f57d6..77e1312 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1548,8 +1548,11 @@ sub to_utf8 {\n> sub esc_param {\n> \tmy $str = shift;\n> \treturn undef unless defined $str;\n> +\n> +\t$str = to_utf8($str);\n> \t$str =~ s/([^A-Za-z0-9\\-_.~()\\/:@ ]+)/CGI::escape($1)/eg;\n> \t$str =~ s/ /\\+/g;\n> +\n> \treturn $str;\n> }\n> \n> @@ -1558,6 +1561,7 @@ sub esc_path_info {\n> \tmy $str = shift;\n> \treturn undef unless defined $str;\n> \n> +\t$str = to_utf8($str);\n> \t# path_info doesn't treat '+' as space (specially), but '?' must be escaped\n> \t$str =~ s/([^A-Za-z0-9\\-_.~();\\/;:@&= +]+)/CGI::escape($1)/eg;\n> \n> @@ -1568,8 +1572,11 @@ sub esc_path_info {\n> sub esc_url {\n> \tmy $str = shift;\n> \treturn undef unless defined $str;\n> +\n> +\t$str = to_utf8($str);\n> \t$str =~ s/([^A-Za-z0-9\\-_.~();\\/;?:@&= ]+)/CGI::escape($1)/eg;\n> \t$str =~ s/ /\\+/g;\n> +\n> \treturn $str;\n> }\n> \n> -- \n> 1.7.1\n> \n> \n\nWhile trying to view a \"blob_plain\" of \"Gütekritierien.txt\", a 404 error\noccured. \"git_get_hash_by_path\" tries to resolve the hash with the wrong\nfilename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails.\n\nThe filename needs the correct encoding. Something like this is probably\nneeded for all filenames and should be done at a prior stage:\n---\n gitweb/gitweb.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 77e1312..e4a50e7 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -4725,7 +4725,7 @@ sub git_print_tree_entry {\n }\n print \" | \" .\n $cgi->a({-href => href(action=>\"blob_plain\", hash_base=>$hash_base,\n- file_name=>\"$basedir$t->{'name'}\")},\n+ file_name=>\"$basedir\" . to_utf8($t->{'name'}))}, \n \"raw\");\n print \"\\n\";\n\n-- \n1.7.1\n"},{"id":"243319","messageId":"538F69DA.9010201@gmail.com","threadId":"36657","inReplyTo":"20140604154128.GA28549@localhost.localdomain","subject":"Re: [PATCH] gitweb: Harden UTF-8 handling in generated links","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-06-04T18:47:54Z","receivedAt":"2014-06-04T18:47:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Michael Wagner wrote:\n> On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote:\n\n>> Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links\n>>\n>> esc_html() ensures that its input is properly UTF-8 encoded and marked\n>> as UTF-8 with to_utf8(). Make esc_param() (used for query parameters\n>> in generated URLs), esc_path_info() (for escaping path_info\n>> components) and esc_url() use it too.\n>>\n>> This hardens gitweb against errors in UTF-8 handling; because\n>> to_utf8() is idempotent it won't change correct output.\n[...]\n>> sub esc_param {\n>> \tmy $str = shift;\n>> \treturn undef unless defined $str;\n>> +\n>> +\t$str = to_utf8($str);\n>> \t$str =~ s/([^A-Za-z0-9\\-_.~()\\/:@ ]+)/CGI::escape($1)/eg;\n>> \t$str =~ s/ /\\+/g;\n>> +\n>> \treturn $str;\n>> } \n \n> While trying to view a \"blob_plain\" of \"Gütekritierien.txt\", a 404 error\n> occured. \"git_get_hash_by_path\" tries to resolve the hash with the wrong\n> filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails.\n> \n> The filename needs the correct encoding. Something like this is probably\n> needed for all filenames and should be done at a prior stage:\n\nTrue.\n\nFirst, I wonder why the tests I did for this situation didn't\nshow any errors even before the \"harden href()\" patch. What\nis different in your config that you see those errors?\n\n> ---\n> gitweb/gitweb.perl | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 77e1312..e4a50e7 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -4725,7 +4725,7 @@ sub git_print_tree_entry {\n> }\n> print \" | \" .\n> $cgi->a({-href => href(action=>\"blob_plain\", hash_base=>$hash_base,\n> - file_name=>\"$basedir$t->{'name'}\")},\n> + file_name=>\"$basedir\" . to_utf8($t->{'name'}))},\n\nSecond, my \"harder href()\" patch does not work for this because\nconcatenation of non-UFT8 with UTF8 string screws up Perl\nknowledge what is and isn't UTF8. So to_utf8() after concat\ndoesn't help.\n\n\n> \"raw\");\n> print \"\\n\";\n> \n"},{"id":"243339","messageId":"20140604204746.GA1855@localhost.localdomain","threadId":"36657","inReplyTo":"538F69DA.9010201@gmail.com","subject":"Re: [PATCH] gitweb: Harden UTF-8 handling in generated links","fromName":"Michael Wagner","fromEmail":"mail@mwagner.org","sentAt":"2014-06-04T20:47:46Z","receivedAt":"2014-06-04T20:47:46Z","isPatch":true,"sender":{"key":"mail@mwagner.org","avatar":null},"body":"On Wed, Jun 04, 2014 at 08:47:54PM +0200, Jakub Narębski wrote:\n> Michael Wagner wrote:\n> > On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote:\n> \n> >> Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links\n> >>\n> >> esc_html() ensures that its input is properly UTF-8 encoded and marked\n> >> as UTF-8 with to_utf8(). Make esc_param() (used for query parameters\n> >> in generated URLs), esc_path_info() (for escaping path_info\n> >> components) and esc_url() use it too.\n> >>\n> >> This hardens gitweb against errors in UTF-8 handling; because\n> >> to_utf8() is idempotent it won't change correct output.\n> [...]\n> >> sub esc_param {\n> >> \tmy $str = shift;\n> >> \treturn undef unless defined $str;\n> >> +\n> >> +\t$str = to_utf8($str);\n> >> \t$str =~ s/([^A-Za-z0-9\\-_.~()\\/:@ ]+)/CGI::escape($1)/eg;\n> >> \t$str =~ s/ /\\+/g;\n> >> +\n> >> \treturn $str;\n> >> } \n> \n> > While trying to view a \"blob_plain\" of \"Gütekritierien.txt\", a 404 error\n> > occured. \"git_get_hash_by_path\" tries to resolve the hash with the wrong\n> > filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails.\n> > \n> > The filename needs the correct encoding. Something like this is probably\n> > needed for all filenames and should be done at a prior stage:\n> \n> True.\n> \n> First, I wonder why the tests I did for this situation didn't\n> show any errors even before the \"harden href()\" patch. What\n> is different in your config that you see those errors?\n> \n\nNothing special. It is reproducible with git 1.9.3 (Fedora 20), git\ninstaweb (lighttpd) and LANG=de_DE.UTF-8. \n \n"}]}