{"thread":{"id":"43345","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","startedAt":"2006-11-08T23:34:13Z","lastAt":"2006-11-10T10:22:14Z","messageCount":12,"participants":["Jakub Narebski","Luben Tuikov","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"296079","messageId":"7vpsbxqzre.fsf@assigned-by-dhcp.cox.net","threadId":"43345","inReplyTo":null,"subject":"[PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-08T23:34:13Z","receivedAt":"2006-11-08T23:34:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This reuses the quot_cec to protect blob and text diff output\nfrom leaking control characters.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n * requesting extra sets of eyeballs.\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 f46d678..b5b1011 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -597,11 +597,10 @@ sub esc_html ($;%) {\n \n \t$str = to_utf8($str);\n \t$str = escapeHTML($str);\n-\t$str =~ s/\\014/^L/g; # escape FORM FEED (FF) character (e.g. in COPYING file)\n-\t$str =~ s/\\033/^[/g; # \"escape\" ESCAPE (\\e) character (e.g. commit 20a3847d8a5032ce41f90dcc68abfb36e6fee9b1)\n \tif ($opts{'-nbsp'}) {\n \t\t$str =~ s/ /&nbsp;/g;\n \t}\n+\t$str =~ s|([[:cntrl:]])|(($1 ne \"\\t\") ? quot_cec($1) : $1)|eg;\n \treturn $str;\n }\n \n@@ -1900,17 +1899,17 @@ sub git_print_page_path {\n \t\t\t$fullname .= ($fullname ? '/' : '') . $dir;\n \t\t\tprint $cgi->a({-href => href(action=>\"tree\", file_name=>$fullname,\n \t\t\t                             hash_base=>$hb),\n-\t\t\t              -title => $fullname}, esc_path($dir));\n+\t\t\t              -title => esc_html($fullname)}, esc_path($dir));\n \t\t\tprint \" / \";\n \t\t}\n \t\tif (defined $type && $type eq 'blob') {\n \t\t\tprint $cgi->a({-href => href(action=>\"blob_plain\", file_name=>$file_name,\n \t\t\t                             hash_base=>$hb),\n-\t\t\t              -title => $name}, esc_path($basename));\n+\t\t\t              -title => esc_html($name)}, esc_path($basename));\n \t\t} elsif (defined $type && $type eq 'tree') {\n \t\t\tprint $cgi->a({-href => href(action=>\"tree\", file_name=>$file_name,\n \t\t\t                             hash_base=>$hb),\n-\t\t\t              -title => $name}, esc_path($basename));\n+\t\t\t              -title => esc_html($name)}, esc_path($basename));\n \t\t\tprint \" / \";\n \t\t} else {\n \t\t\tprint esc_path($basename);\n-- \n1.4.4.rc1.g659d\n\n"},{"id":"296871","messageId":"200611090104.32247.jnareb@gmail.com","threadId":"43345","inReplyTo":"7vpsbxqzre.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-09T00:04:31Z","receivedAt":"2006-11-09T00:04:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> This reuses the quot_cec to protect blob and text diff output\n> from leaking control characters.\n> \n> Signed-off-by: Junio C Hamano <junkio@cox.net>\n> ---\n>  * requesting extra sets of eyeballs.\n\nThis changes the \"blob\" and \"text diff\" output somewhat, as earlier it \nused Control key Sequence (CS) representation for some non-whitespace \ncontrol characters (not \"\\t' not '\\n'), namely replacing form feed (FF) \n('\\f', '\\014') with ^L and escape (ESC) ('\\e', '\\033') with ^[.\n\nAnd (what is not said in the commit message) it additionally esc_html \nsome title elements (the subroutine should be I think named esc_attr).\n\nThe problems are:\n1. First, esc_path should _not_ use subroutine which does it's own \ncontol characters escaping. That was also a mistake I made in my patch.\nPerhaps we should have some quot_html or to_html subroutine which does \n_only_ to_utf8 (decode from Encode module), escapeHTML and optionally \ns/ /&nbsp;/g conversion.\n\n2. In my opinion CS is better than CEC for quoting/escaping control \ncharacters in the \"bulk\" output, namely \"blob\" output and \"text \ndiff\" (patchset body) output. CEC is better for pathnames (which must \nfit in one line), and perhaps other one-liners; perhaps not. I'm not \nsure what quoting to choose for esc_attr, but there we could use even \n--no-control-chars quoting (replacing any control character by '?'); \nbut perhaps in some cases like git_print_page_path subroutine CEC is \nbetter.\n\nBTW. what had happened with to_qtext post?\n-- \nJakub Narebski\n"},{"id":"295919","messageId":"7v1wodqxux.fsf@assigned-by-dhcp.cox.net","threadId":"43345","inReplyTo":"200611090104.32247.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-09T00:15:18Z","receivedAt":"2006-11-09T00:15:18Z","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> 1. First, esc_path should _not_ use subroutine which does it's own \n> contol characters escaping. That was also a mistake I made in my patch.\n> Perhaps we should have some quot_html or to_html subroutine which does \n> _only_ to_utf8 (decode from Encode module), escapeHTML and optionally \n> s/ /&nbsp;/g conversion.\n\nI hated that original arrangement, but I do not see anything\nobviously wrong in the output with the patch you are responding\nto.  Except that git_blame2 is missing a chomp() on \"my $data\"\nafter finishing the metainfo loop, that is.\n\n> 2. In my opinion CS is better than CEC for quoting/escaping control \n> characters in the \"bulk\" output, namely \"blob\" output and \"text \n> diff\" (patchset body) output. CEC is better for pathnames (which must \n> fit in one line), and perhaps other one-liners; perhaps not.\n\nI am more for code reuse and consistency.  If \"^L\" is more\nreadable then we should consistently use it for both contents\nand pathnames.  One of my tests were a symlink that points at a\nfunny filename ;-).\n\n> BTW. what had happened with to_qtext post?\n\nSorry, I don't recall.\n"},{"id":"295046","messageId":"200611090146.25306.jnareb@gmail.com","threadId":"43345","inReplyTo":"7v1wodqxux.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-09T00:46:25Z","receivedAt":"2006-11-09T00:46:25Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> 1. First, esc_path should _not_ use subroutine which does it's own \n>> contol characters escaping. That was also a mistake I made in my patch.\n>> Perhaps we should have some quot_html or to_html subroutine which does \n>> _only_ to_utf8 (decode from Encode module), escapeHTML and optionally \n>> s/ /&nbsp;/g conversion.\n> \n> I hated that original arrangement, \n\nWhat did you hate, again?\n\n>                                    but I do not see anything \n> obviously wrong in the output with the patch you are responding\n> to.  Except that git_blame2 is missing a chomp() on \"my $data\"\n> after finishing the metainfo loop, that is.\n\nThe original (mine) code for esc_path uses esc_html, which did it's\nown partial (very partial) special characters esaping, namely\n\\014 (\\f) => ^L, \\033 (\\e) => ^[. So if pathname had form feed character,\nit would be replaced by ^L, not '\\f'.\n\nYou have added quot_cec to esc_html subroutine directly. I don't know\nwhat is your version of esc_html after the changes you made, but\nthis makes escaping part (quot) in esc_path never invoked. \n\n>> 2. In my opinion CS is better than CEC for quoting/escaping control \n>> characters in the \"bulk\" output, namely \"blob\" output and \"text \n>> diff\" (patchset body) output. CEC is better for pathnames (which must \n>> fit in one line), and perhaps other one-liners; perhaps not.\n> \n> I am more for code reuse and consistency.  If \"^L\" is more\n> readable then we should consistently use it for both contents\n> and pathnames.  \n\nWell, the pathname has the limit that it must be in single line\nafter quoting. The \"blob\" output is multipage. IMHO CEC like \\n, \\f,\n\\t are better in pathnames because this is what ls uses, while CS\nfor \"blob\" output is better because editors (including one true\neditor being GNU Emacs ;-) uses CS like ^L (there is no end-of-line\nas we split on LF and chomp; there is no tab character because line\nis untabified first). But that is my opinion.\n\nI think that conrol characters in filenames (in esc_path) should\nbe encompassed with <span class=\"cntrl\">...</span> and styled.\nI'm not sure if in \"blob\" view they should be styled. For sure\nthere should be no <span>...</span> for escaped attributed (future\nesc_attr). Common to_html/quot_html would give us code reuse (as gives\nquot_cec), if not consistent.\n\n>                One of my tests were a symlink that points at a \n> funny filename ;-).\n\nThis should be IMHO solved rather by better \"tree\" view support\nfor symlinks, 'symlink' -> 'target' like in ls -l output.\n\n>> BTW. what had happened with to_qtext post?\n> \n> Sorry, I don't recall.\n\nThere was quite a bit of discussion about name of _suggested_\nfilename in blob_plain, blobdiff_plain view, namely the \n  -content_disposition => 'inline; filename=\"' ...\nHTTP header. The result (probably lost in the noise) was to\nadd to_qtext subroutine for that.\n\nTime to go to sleep...\n-- \nJakub Narebski\n"},{"id":"295388","messageId":"7vwt65pgqs.fsf@assigned-by-dhcp.cox.net","threadId":"43345","inReplyTo":"200611090146.25306.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-09T01:10:19Z","receivedAt":"2006-11-09T01:10:19Z","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> Junio C Hamano wrote:\n>> Jakub Narebski <jnareb@gmail.com> writes:\n>> \n>>> 1. First, esc_path should _not_ use subroutine which does it's own \n>>> contol characters escaping. That was also a mistake I made in my patch.\n>>> Perhaps we should have some quot_html or to_html subroutine which does \n>>> _only_ to_utf8 (decode from Encode module), escapeHTML and optionally \n>>> s/ /&nbsp;/g conversion.\n>> \n>> I hated that original arrangement, \n>\n> What did you hate, again?\n\nesc_path calling esc_html you mentioned, of course.\n\n\n>> obviously wrong in the output with the patch you are responding\n>> to.  Except that git_blame2 is missing a chomp() on \"my $data\"\n>> after finishing the metainfo loop, that is.\n>\n> The original (mine) code for esc_path uses esc_html, which did it's\n> own partial (very partial) special characters esaping, namely\n> \\014 (\\f) => ^L, \\033 (\\e) => ^[. So if pathname had form feed character,\n> it would be replaced by ^L, not '\\f'.\n\nI know -- that is what I meant by \"code reuse and consistency\".\n\n> You have added quot_cec to esc_html subroutine directly. I don't know\n> what is your version of esc_html after the changes you\n> made,...\n\nSee \"pu\".\n\n> Well, the pathname has the limit that it must be in single line\n> after quoting. The \"blob\" output is multipage.\n\nI honestly have _no_ idea what distincition you are seeing\nhere.  Both blob and diff output are processed one line at a\ntime and its result would be on a single line too.\n"},{"id":"296853","messageId":"200611091024.35019.jnareb@gmail.com","threadId":"43345","inReplyTo":"200611090104.32247.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-09T09:24:34Z","receivedAt":"2006-11-09T09:24:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n> I'm not sure what quoting to choose for esc_attr, but there we could\n> use even --no-control-chars quoting (replacing any control character\n> by '?');  but perhaps in some cases like git_print_page_path\n> subroutine CEC is better.\n\nI'm rambling. esc_attr is special case, because CGI does escapeHTML\n(and I hope also to_utf8) for us. Using <span class=\"cntrl\">...</span>\nhas also no sense. So there should be separate esc_attr_path subroutine\nI think.\n\nEven if we decide that esc_html and esc_path should give identical\noutput (the difference that _might_ be here is that in esc_html we\ndon't need to escape whitespace control characters valid in HTML,\nlike tab (HT, TAB) or newline (LF); on the other hand thanks to\nline-by-line processing we should never get newline in \"blob\", and\nthanks to untabify we should never get tab in \"blob\") I think it would\nbe prudent to have esc_path, even as thin wrapper just caling esc_html.\n\nWe might decide to use different style for control characters in\ndifferent views, but that I think can be done using pure CSS.\n-- \nJakub Narebski\n"},{"id":"298489","messageId":"200611091034.42190.jnareb@gmail.com","threadId":"43345","inReplyTo":"7vwt65pgqs.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-09T09:34:41Z","receivedAt":"2006-11-09T09:34:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia czwartek 9. listopada 2006 02:10, Junio C Hamano napisał:\n> Jakub Narebski <jnareb@gmail.com> writes:\n\n>> Well, the pathname has the limit that it must be in single line\n>> after quoting. The \"blob\" output is multipage.\n> \n> I honestly have _no_ idea what distincition you are seeing\n> here.  Both blob and diff output are processed one line at a\n> time and its result would be on a single line too.\n\nI was thinking about _conceptual_ difference, not technical.\n(Perhaps I should make it more clear.) \n\nPathname is item (or part of item in the case of page_path)\nwhich is contained (and must be contained) in single line.\nIt is also expected (although if we follow this expectation\nis up to us) that the pathname would quote special characters\nsimilarly to how shell/operating system quotes pathnames\n(e.g. in ls output).\n\n\"Blob\" output on the other hand (\"blob\" view and patch part of\n\"blobdiff\" and \"commitdiff\" views) is [a part of] larger, multiline\nwhole. One could also expect that special characters would be\nquoted like editor quotes special characters. (Of course question\nis: which editor?)\n\nThis of course is complicate by single line output like subject\nor authorship, or signoff, which is not pathname.\n\n\nAll this discussion shows that gitweb quoting is more complicated\nthat I thought.\n-- \nJakub Narebski\n"},{"id":"298203","messageId":"7vlkmlkkq8.fsf@assigned-by-dhcp.cox.net","threadId":"43345","inReplyTo":"200611091024.35019.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-09T09:55:27Z","receivedAt":"2006-11-09T09:55:27Z","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> Jakub Narebski wrote:\n>> I'm not sure what quoting to choose for esc_attr, but there we could\n>> use even --no-control-chars quoting (replacing any control character\n>> by '?');  but perhaps in some cases like git_print_page_path\n>> subroutine CEC is better.\n\nTo be honest, I do not have strong preference between the\nescaping style.  If the gitweb cabal feel it is more natural to\nsee \"^L\" in blobs and \"\\f\" in path, I will very happily accept\nsuch a patch.\n\n> I'm rambling. esc_attr is special case, because CGI does escapeHTML\n> (and I hope also to_utf8) for us. Using <span class=\"cntrl\">...</span>\n> has also no sense. So there should be separate esc_attr_path subroutine\n> I think.\n\nYes.  It is unfortunate that there needs different types of\nquoting.  I think the first step would be to stop calling\nesc_html in esc_path.  I think it was a mistake, and I did not\ncorrect it when I started touching it.\n\nSomehow I ended up spending sizeable part of my git day this\nweek on fixing up blob/blame/tag/commit message view regarding\nthis \"make controls visible and safe\" issues on the 'master'\nbranch, but I have been consciously staying out of gitweb/ part\nof the system, primarily because there are many other people who\nare more interested and qualified in it than myself.\n\nI'll step aside and try not to get in the way.  There is another\nthing I noticed while testing it with an artifitial test that I\nhaven't fixed, but I think you already know about it (when the\ncommitdiff is completely empty except mode changes, we end up\nwith unbalanced div).  My test's tip can be found at\n'gitweb-test-funny-char' branch temporarily in the git.git\nrepository.\n"},{"id":"294421","messageId":"200611091102.56565.jnareb@gmail.com","threadId":"43345","inReplyTo":"7vlkmlkkq8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-09T10:02:55Z","receivedAt":"2006-11-09T10:02:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> [...]  There is another\n> thing I noticed while testing it with an artifitial test that I\n> haven't fixed, but I think you already know about it (when the\n> commitdiff is completely empty except mode changes, we end up\n> with unbalanced div).  My test's tip can be found at\n> 'gitweb-test-funny-char' branch temporarily in the git.git\n> repository.\n\nDamn. I thought I corrected this on resend...\n\n\tif ($patch_found) {\n\t\t# close extended header for previous empty patch\n\t\tif ($in_header) {\n\t\t\tprint \"</div>\\n\" # class=\"diff extended_header\"\n\t\t}\n\t\t# close previous patch\n\t\tprint \"</div>\\n\"; # class=\"patch\"\n\t}\n-- \nJakub Narebski\n"},{"id":"297950","messageId":"7v8xiklxh1.fsf@assigned-by-dhcp.cox.net","threadId":"43345","inReplyTo":"200611091102.56565.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-09T10:34:50Z","receivedAt":"2006-11-09T10:34:50Z","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> Junio C Hamano wrote:\n>> [...]  There is another\n>> thing I noticed while testing it with an artifitial test that I\n>> haven't fixed, but I think you already know about it (when the\n>> commitdiff is completely empty except mode changes, we end up\n>> with unbalanced div).  My test's tip can be found at\n>> 'gitweb-test-funny-char' branch temporarily in the git.git\n>> repository.\n>\n> Damn. I thought I corrected this on resend...\n\nI think you need this, otherwise when the last filepair changes\nonly metainfo you fail to close the extended header div.\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1a757cc..e54a29e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2338,6 +2338,8 @@ sub git_patchset_body {\n \n \t\tprint format_diff_line($patch_line);\n \t}\n+\tprint \"</div>\\n\" if $in_header; # extended header\n+\n \tprint \"</div>\\n\" if $patch_found; # class=\"patch\"\n \n \tprint \"</div>\\n\"; # class=\"patchset\"\n"},{"id":"298153","messageId":"200611091141.42205.jnareb@gmail.com","threadId":"43345","inReplyTo":"7v8xiklxh1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-11-09T10:41:41Z","receivedAt":"2006-11-09T10:41:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n>> Junio C Hamano wrote:\n>>> [...]  There is another\n>>> thing I noticed while testing it with an artifitial test that I\n>>> haven't fixed, but I think you already know about it (when the\n>>> commitdiff is completely empty except mode changes, we end up\n>>> with unbalanced div).  My test's tip can be found at\n>>> 'gitweb-test-funny-char' branch temporarily in the git.git\n>>> repository.\n>>\n>> Damn. I thought I corrected this on resend...\n> \n> I think you need this, otherwise when the last filepair changes\n> only metainfo you fail to close the extended header div.\n\nTrue, I forgot about the situation where empty patch is _last_\nin the patchset (which includes the situation when it is _only_\npatch).\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 1a757cc..e54a29e 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2338,6 +2338,8 @@ sub git_patchset_body {\n>  \n>  \t\tprint format_diff_line($patch_line);\n>  \t}\n> +\tprint \"</div>\\n\" if $in_header; # extended header\nI would write '# class=\"diff extended_header\"' here instead.\n> +\n>  \tprint \"</div>\\n\" if $patch_found; # class=\"patch\"\n>  \n>  \tprint \"</div>\\n\"; # class=\"patchset\"\n\nLooks good. Ack.\n\n-- \nJakub Narebski\n"},{"id":"295084","messageId":"976905.27021.qm@web31807.mail.mud.yahoo.com","threadId":"43345","inReplyTo":"7vlkmlkkq8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: protect blob and diff output lines from controls.","fromName":"Luben Tuikov","fromEmail":"ltuikov@yahoo.com","sentAt":"2006-11-10T10:22:14Z","receivedAt":"2006-11-10T10:22:14Z","isPatch":true,"sender":{"key":"ltuikov@yahoo.com","avatar":null},"body":"--- Junio C Hamano <junkio@cox.net> wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > Jakub Narebski wrote:\n> >> I'm not sure what quoting to choose for esc_attr, but there we could\n> >> use even --no-control-chars quoting (replacing any control character\n> >> by '?');  but perhaps in some cases like git_print_page_path\n> >> subroutine CEC is better.\n> \n> To be honest, I do not have strong preference between the\n> escaping style.  If the gitweb cabal feel it is more natural to\n> see \"^L\" in blobs and \"\\f\" in path, I will very happily accept\n> such a patch.\n\nI've little preference either, as long as the intention\nof the original name is preserved across gitweb (to a user's\ngit-repo/download).\n\n> Yes.  It is unfortunate that there needs different types of\n> quoting.  I think the first step would be to stop calling\n> esc_html in esc_path.  I think it was a mistake, and I did not\n> correct it when I started touching it.\n\nWhen Jakub mentioned \"to_qtext\" he meant this patch:\nhttp://marc.theaimsgroup.com/?l=git&m=116016249121781&w=2\n\n   Luben\n\n\n> Somehow I ended up spending sizeable part of my git day this\n> week on fixing up blob/blame/tag/commit message view regarding\n> this \"make controls visible and safe\" issues on the 'master'\n> branch, but I have been consciously staying out of gitweb/ part\n> of the system, primarily because there are many other people who\n> are more interested and qualified in it than myself.\n> \n> I'll step aside and try not to get in the way.  There is another\n> thing I noticed while testing it with an artifitial test that I\n> haven't fixed, but I think you already know about it (when the\n> commitdiff is completely empty except mode changes, we end up\n> with unbalanced div).  My test's tip can be found at\n> 'gitweb-test-funny-char' branch temporarily in the git.git\n> repository.\n> \n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"}]}