{"thread":{"id":"28195","subject":"[PATCH] gitweb: highlight: strip non-printable characters via col(1)","startedAt":"2011-08-22T22:58:43Z","lastAt":"2011-09-16T20:24:11Z","messageCount":10,"participants":["Christopher M. Fuhrman","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"174055","messageId":"1314053923-13122-1-git-send-email-cfuhrman@panix.com","threadId":"28195","inReplyTo":null,"subject":"[PATCH] gitweb: highlight: strip non-printable characters via col(1)","fromName":"Christopher M. Fuhrman","fromEmail":"cfuhrman@panix.com","sentAt":"2011-08-22T22:58:43Z","receivedAt":"2011-08-22T22:58:43Z","isPatch":true,"sender":{"key":"cfuhrman@panix.com","avatar":"https://gravatar.com/avatar/ec9fed21fc8fadbb9fab491480c80a464ac25a0632eedfa594ca6c2f39f04c2d?d=mp&s=160"},"body":"From: \"Christopher M. Fuhrman\" <cfuhrman@panix.com>\n\nThe current code, as is, passes control characters, such as form-feed\n(^L) to highlight which then passes it through to the browser.  This\nwill cause the browser to display one of the following warnings:\n\nSafari v5.1 (6534.50) & Google Chrome v13.0.782.112:\n\n  This page contains the following errors:\n\n  error on line 657 at column 38: PCDATA invalid Char value 12\n  Below is a rendering of the page up to the first error.\n\nMozilla Firefox 3.6.19 & Mozilla Firefox 5.0:\n\n   XML Parsing Error: not well-formed\n   Location:\n   http://path/to/git/repo/blah/blah\n\nBoth errors were generated by gitweb.perl v1.7.3.4 w/ highlight 2.7\nusing arch/ia64/kernel/unwind.c from the Linux kernel.\n\nStrip non-printable control-characters by piping the output produced\nby git-cat-file(1) to col(1) as follows:\n\n  git cat-file blob deadbeef314159 | col -bx | highlight <args>\n\nNote usage of the '-x' option which tells col(1) to output multiple\nspaces instead of tabs.\n\nTested under OpenSuSE 11.4 & NetBSD 5.1 using perl 5.12.3 and perl\n5.12.2 respectively using Safari, Firefox, and Google Chrome.\n\nSigned-off-by: Christopher M. Fuhrman <cfuhrman@panix.com>\n---\nHowdy,\n\nI haven't gotten any responses to my patch for a while, so I am now\nsubmitting this for general inclusion into git.  Please note that this\nis based off the \"maint\" branch per Documentation/SubmittingPatches\n\nFor an example of this bug in action, see:\n\n* http://git.fuhrbear.com/~cfuhrman/?p=linux/.git;a=blob;f=arch/alpha/kernel/core_titan.c;h=219bf271c0ba2e5f2d668af707df57fbbd00ccfd;hb=HEAD\n* http://git.fuhrbear.com/~cfuhrman/?p=linux/.git;a=blob;f=arch/ia64/kernel/unwind.c;h=fed6afa2e8a9014e65229e51e64fa4b1c13cc284;hb=HEAD\n\nWRT the col(1) command, I've verified that the binary is installed in\n/usr/bin on OpenSuSE, NetBSD, OpenBSD, Solaris 10, and AIX.  This\npatch assumes that /usr/bin is in $PATH.\n\nCheers!\n\n gitweb/gitweb.perl |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 50a835a..4c68165 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3656,6 +3656,7 @@ sub run_highlighter {\n \n \tclose $fd;\n \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n+\t          \"col -bx | \".\n \t          quote_command($highlight_bin).\n \t          \" --replace-tabs=8 --fragment --syntax $syntax |\"\n \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n-- \n1.7.5.4\n"},{"id":"174057","messageId":"7vmxf1t4l3.fsf@alter.siamese.dyndns.org","threadId":"28195","inReplyTo":"1314053923-13122-1-git-send-email-cfuhrman@panix.com","subject":"Re: [PATCH] gitweb: highlight: strip non-printable characters via col(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-22T23:21:44Z","receivedAt":"2011-08-22T23:21:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Christopher M. Fuhrman\" <cfuhrman@panix.com> writes:\n\n> Strip non-printable control-characters by piping the output produced\n> by git-cat-file(1) to col(1) as follows:\n>\n>   git cat-file blob deadbeef314159 | col -bx | highlight <args>\n>\n> Note usage of the '-x' option which tells col(1) to output multiple\n> spaces instead of tabs.\n\nAre all implementations of col known to correctly handle bytes with their\nhighest bit on, without mistaking them with unknown control sequences?\nHas the code updated by your patch been tested with non-ASCII payload, at\nleast with UTF-8 outside US-ASCII?\n\nIn what locale does the code updated by your patch run under, and would\nthe use of \"col\" affected by the choice of the locale in a negative way?\n\nFor example, here is what I get on my box:\n\n    $ LANG=C LC_ALL=C col -bx <t/t3902-quoted.sh ; echo $?\n    col: Invalid or incomplete multibyte or wide character\n    1\n\nthat makes me ask you these questions.\n\n> I haven't gotten any responses to my patch for a while, so I am now\n> submitting this for general inclusion into git.\n\nUnfortunately, no news is not good news around here, and that is why I\nam asking you the above questions.\n\nThanks.\n"},{"id":"174342","messageId":"201108262154.14493.jnareb@gmail.com","threadId":"28195","inReplyTo":"1314053923-13122-1-git-send-email-cfuhrman@panix.com","subject":"Re: [PATCH] gitweb: highlight: strip non-printable characters via col(1)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-08-26T19:54:13Z","receivedAt":"2011-08-26T19:54:13Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 23 Aug 2011, Christopher M. Fuhrman wrote:\n\n> The current code, as is, passes control characters, such as form-feed\n> (^L) to highlight which then passes it through to the browser.  This\n> will cause the browser to display one of the following warnings:\n> \n> Safari v5.1 (6534.50) & Google Chrome v13.0.782.112:\n> \n>   This page contains the following errors:\n> \n>   error on line 657 at column 38: PCDATA invalid Char value 12\n>   Below is a rendering of the page up to the first error.\n> \n> Mozilla Firefox 3.6.19 & Mozilla Firefox 5.0:\n> \n>    XML Parsing Error: not well-formed\n>    Location:\n>    http://path/to/git/repo/blah/blah\n> \n> Both errors were generated by gitweb.perl v1.7.3.4 w/ highlight 2.7\n> using arch/ia64/kernel/unwind.c from the Linux kernel.\n> \n> Strip non-printable control-characters by piping the output produced\n> by git-cat-file(1) to col(1) as follows:\n> \n>   git cat-file blob deadbeef314159 | col -bx | highlight <args>\n> \n> Note usage of the '-x' option which tells col(1) to output multiple\n> spaces instead of tabs.\n\nWhy use external program (which ming be not installed, or might not\nstrip control-characters), instead of making gitweb sanitize highlighter\noutput itself.  Something like the patch below (which additionally\nshows where there are control characters):\n\n-- >8 --\ndiff --git i/gitweb/gitweb.perl w/gitweb/gitweb.perl\nindex 7cf12af..192db2c 100755\n--- i/gitweb/gitweb.perl\n+++ w/gitweb/gitweb.perl\n@@ -1517,6 +1517,17 @@ sub esc_path {\n \treturn $str;\n }\n \n+# Sanitize for use in XHTML + application/xml+xhtml\n+sub sanitize {\n+\tmy $str = shift;\n+\n+\treturn undef unless defined $str;\n+\n+\t$str = to_utf8($str);\n+\t$str =~ s|([[:cntrl:]])|quot_cec($1)|eg;\n+\treturn $str;\n+}\n+\n # Make control characters \"printable\", using character escape codes (CEC)\n sub quot_cec {\n \tmy $cntrl = shift;\n@@ -6546,7 +6557,8 @@ sub git_blob {\n \t\t\t$nr++;\n \t\t\t$line = untabify($line);\n \t\t\tprintf qq!<div class=\"pre\"><a id=\"l%i\" href=\"%s#l%i\" class=\"linenr\">%4i</a> %s</div>\\n!,\n-\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr, $syntax ? to_utf8($line) : esc_html($line, -nbsp=>1);\n+\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n+\t\t\t       $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n \t\t}\n \t}\n \tclose $fd\n\n-- 8< --\n\n-- \nJakub Narebski\nPoland\n"},{"id":"174352","messageId":"7v8vqfdf0l.fsf@alter.siamese.dyndns.org","threadId":"28195","inReplyTo":"201108262154.14493.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: highlight: strip non-printable characters via col(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-26T21:44:26Z","receivedAt":"2011-08-26T21:44:26Z","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> Why use external program (which ming be not installed, or might not\n> strip control-characters), instead of making gitweb sanitize highlighter\n> output itself.  Something like the patch below (which additionally\n> shows where there are control characters):\n\nI agree that that would be a more sensible approach. What does your sample\ncode below do to a HT by the way?\n\n> -- >8 --\n> diff --git i/gitweb/gitweb.perl w/gitweb/gitweb.perl\n> index 7cf12af..192db2c 100755\n> --- i/gitweb/gitweb.perl\n> +++ w/gitweb/gitweb.perl\n> @@ -1517,6 +1517,17 @@ sub esc_path {\n>  \treturn $str;\n>  }\n>  \n> +# Sanitize for use in XHTML + application/xml+xhtml\n> +sub sanitize {\n> +\tmy $str = shift;\n> +\n> +\treturn undef unless defined $str;\n> +\n> +\t$str = to_utf8($str);\n> +\t$str =~ s|([[:cntrl:]])|quot_cec($1)|eg;\n> +\treturn $str;\n> +}\n> +\n>  # Make control characters \"printable\", using character escape codes (CEC)\n>  sub quot_cec {\n>  \tmy $cntrl = shift;\n> @@ -6546,7 +6557,8 @@ sub git_blob {\n>  \t\t\t$nr++;\n>  \t\t\t$line = untabify($line);\n>  \t\t\tprintf qq!<div class=\"pre\"><a id=\"l%i\" href=\"%s#l%i\" class=\"linenr\">%4i</a> %s</div>\\n!,\n> -\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr, $syntax ? to_utf8($line) : esc_html($line, -nbsp=>1);\n> +\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> +\t\t\t       $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n>  \t\t}\n>  \t}\n>  \tclose $fd\n>\n> -- 8< --\n"},{"id":"174354","messageId":"201108270006.19289.jnareb@gmail.com","threadId":"28195","inReplyTo":"7v8vqfdf0l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: highlight: strip non-printable characters via col(1)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-08-26T22:06:18Z","receivedAt":"2011-08-26T22:06:18Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 26 Aug 2011, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > Why use external program (which ming be not installed, or might not\n> > strip control-characters), instead of making gitweb sanitize highlighter\n> > output itself.  Something like the patch below (which additionally\n> > shows where there are control characters):\n> \n> I agree that that would be a more sensible approach. What does your sample\n> code below do to a HT by the way?\n\nActually the line earlier\n\n \t\t\t$line = untabify($line);\n\nreplaces HT (\"\\t\") with spaces.\n\n> > -- >8 --\n> > diff --git i/gitweb/gitweb.perl w/gitweb/gitweb.perl\n> > index 7cf12af..192db2c 100755\n> > --- i/gitweb/gitweb.perl\n> > +++ w/gitweb/gitweb.perl\n> > @@ -1517,6 +1517,17 @@ sub esc_path {\n> >  \treturn $str;\n> >  }\n> >  \n> > +# Sanitize for use in XHTML + application/xml+xhtml\n> > +sub sanitize {\n> > +\tmy $str = shift;\n> > +\n> > +\treturn undef unless defined $str;\n> > +\n> > +\t$str = to_utf8($str);\n> > +\t$str =~ s|([[:cntrl:]])|quot_cec($1)|eg;\n> > +\treturn $str;\n> > +}\n\nAnyway, it could well be\n\n+\t$str =~ s|([[:cntrl:]])|(($1 ne \"\\t\") ? quot_cec($1) : $1)|eg;\n+\treturn $str;\n\nlike in esc_html rather than like in esc_path.\n\n> > @@ -6546,7 +6557,8 @@ sub git_blob {\n> >  \t\t\t$nr++;\n> >  \t\t\t$line = untabify($line);\n                        ^^^^^^^^^^^^^^^^^^^^^^^^\n\n> >  \t\t\tprintf qq!<div class=\"pre\"><a id=\"l%i\" href=\"%s#l%i\" class=\"linenr\">%4i</a> %s</div>\\n!,\n> > -\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr, $syntax ? to_utf8($line) : esc_html($line, -nbsp=>1);\n> > +\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> > +\t\t\t       $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n> >  \t\t}\n> >  \t}\n> >  \tclose $fd\n\n-- \nJakub Narebski\nPoland\n"},{"id":"175642","messageId":"201109161441.58946.jnareb@gmail.com","threadId":"28195","inReplyTo":"201108270006.19289.jnareb@gmail.com","subject":"[PATCH] gitweb: Strip non-printable characters from syntax highlighter output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-09-16T12:41:57Z","receivedAt":"2011-09-16T12:41:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"The current code, as is, passes control characters, such as form-feed\n(^L) to highlight which then passes it through to the browser.  User\nagents (web browsers) that support 'application/xhtml+xml' usually\nrequire that web pages declared as XHTML and with this mimetype are\nwell-formed XML.  Unescaped control characters cannot appear within a\ncontents of a valid XML document.\n\nThis will cause the browser to display one of the following warnings:\n\n* Safari v5.1 (6534.50) & Google Chrome v13.0.782.112:\n\n   This page contains the following errors:\n\n   error on line 657 at column 38: PCDATA invalid Char value 12\n   Below is a rendering of the page up to the first error.\n\n* Mozilla Firefox 3.6.19 & Mozilla Firefox 5.0:\n\n   XML Parsing Error: not well-formed\n   Location:\n   http://path/to/git/repo/blah/blah\n\nBoth errors were generated by gitweb.perl v1.7.3.4 w/ highlight 2.7\nusing arch/ia64/kernel/unwind.c from the Linux kernel.\n\nWhen syntax highlighter is not used, control characters are replaced\nby esc_html(), but with syntax highlighter they were passed through to\nbrowser (to_utf8() doesn't remove control characters).\n\nIntroduce sanitize() subroutine which strips forbidden characters, but\ndoes not perform HTML escaping, and use it in git_blob() to sanitize\nsyntax highlighter output for XHTML.\n\nNote that excluding \"\\t\" (U+0009), \"\\n\" (U+000A) and \"\\r\" (U+000D) is\nnot strictly necessary, atleast for currently the only callsite: \"\\t\"\ntabs are replaced by spaces by untabify(), \"\\n\" is stripped from each\nline before processing it, and replacing \"\\r\" could be considered\nimprovement.\n\nOriginally-by: Christopher M. Fuhrman <cfuhrman@panix.com>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThe commit message is from Christopher, but I have replaced his solution\nof stripping non-printable characters via col(1) program by having gitweb\nstrip characters not allowed in XML.\n\nChristopher, could you check that it fixes your issue?\n\n gitweb/gitweb.perl |   14 +++++++++++++-\n 1 files changed, 13 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 70a576a..c28b847 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1517,6 +1517,17 @@ sub esc_path {\n \treturn $str;\n }\n \n+# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)\n+sub sanitize {\n+\tmy $str = shift;\n+\n+\treturn undef unless defined $str;\n+\n+\t$str = to_utf8($str);\n+\t$str =~ s|([[:cntrl:]])|($1 =~ /[\\t\\n\\r]/ ? $1 : quot_cec($1))|eg;\n+\treturn $str;\n+}\n+\n # Make control characters \"printable\", using character escape codes (CEC)\n sub quot_cec {\n \tmy $cntrl = shift;\n@@ -6484,7 +6495,8 @@ sub git_blob {\n \t\t\t$nr++;\n \t\t\t$line = untabify($line);\n \t\t\tprintf qq!<div class=\"pre\"><a id=\"l%i\" href=\"%s#l%i\" class=\"linenr\">%4i</a> %s</div>\\n!,\n-\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr, $syntax ? to_utf8($line) : esc_html($line, -nbsp=>1);\n+\t\t\t       $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n+\t\t\t       $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n \t\t}\n \t}\n \tclose $fd\n-- \n1.7.6\n"},{"id":"175649","messageId":"7vwrd8fnxr.fsf@alter.siamese.dyndns.org","threadId":"28195","inReplyTo":"201109161441.58946.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Strip non-printable characters from syntax highlighter output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-16T16:32:16Z","receivedAt":"2011-09-16T16:32:16Z","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> The commit message is from Christopher, but I have replaced his solution\n> of stripping non-printable characters via col(1) program by having gitweb\n> strip characters not allowed in XML.\n>\n> Christopher, could you check that it fixes your issue?\n\nThanks.\n\nMicronit:\n\n> +# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)\n> +sub sanitize {\n> +\tmy $str = shift;\n> +\n> +\treturn undef unless defined $str;\n\nGiven that the _whole_ point of this subroutine is to make $str safe for\nprinting, wouldn't you want to either (1) die, declaring that feeding an\nundef to this subroutine is a programming error, or (2) return an empty\nstring?\n\nGiven that the input to this function is from the result of feeding $line\nto untabify, which relies on $line being defined, and that $line comes\nfrom \"while (my $line = <$fd>)\" (and then chomp $line), it may be Ok for\nthis subroutine to make the same assumption as untabify makes.\n"},{"id":"175651","messageId":"alpine.NEB.2.01.1109161050080.2073@vc75.vc.panix.com","threadId":"28195","inReplyTo":"201109161441.58946.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Strip non-printable characters from syntax highlighter output","fromName":"Christopher M. Fuhrman","fromEmail":"cfuhrman@panix.com","sentAt":"2011-09-16T18:11:01Z","receivedAt":"2011-09-16T18:11:01Z","isPatch":true,"sender":{"key":"cfuhrman@panix.com","avatar":"https://gravatar.com/avatar/ec9fed21fc8fadbb9fab491480c80a464ac25a0632eedfa594ca6c2f39f04c2d?d=mp&s=160"},"body":"Howdy,\n\nOn Fri, 16 Sep 2011 at 5:41am, Jakub Narebski wrote:\n\n> The commit message is from Christopher, but I have replaced his solution\n> of stripping non-printable characters via col(1) program by having gitweb\n> strip characters not allowed in XML.\n>\n> Christopher, could you check that it fixes your issue?\n\nAfter applying the patch, I tested it successfully against the following\nfiles:\n\n * linux.git : arch/ia64/kernel/unwind.c\n * git.git   : t/t3902-quoted.sh\n\nFurthermore, I'm pleased to report that non en_US.UTF8 characters (e.g.,\nChinese hanzi) as found in t3902-quoted.sh are displayed properly when\nhighlight is enabled.\n\nTested Web Browsers:\n\n * Safari (5.1 (6534.50)\n * Firefox 6.0.2 under Mac OS X Snow Leopard\n * Google Chrome 13.0.782.220 under OpenSuSE 11.4\n\n>\n>  gitweb/gitweb.perl |   14 +++++++++++++-\n>  1 files changed, 13 insertions(+), 1 deletions(-)\n>\n\nCheers!\n\n-- \nChris Fuhrman\ncfuhrman@panix.com\n"},{"id":"175654","messageId":"201109162058.51132.jnareb@gmail.com","threadId":"28195","inReplyTo":"7vwrd8fnxr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: Strip non-printable characters from syntax highlighter output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-09-16T18:58:49Z","receivedAt":"2011-09-16T18:58:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 16 Sep 2011, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n\n> Micronit:\n> \n> > +# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)\n> > +sub sanitize {\n> > +\tmy $str = shift;\n> > +\n> > +\treturn undef unless defined $str;\n> \n> Given that the _whole_ point of this subroutine is to make $str safe for\n> printing, wouldn't you want to either (1) die, declaring that feeding an\n> undef to this subroutine is a programming error, or (2) return an empty\n> string?\n\nWell, that\n\n\treturn undef unless defined $str;\n\nline is copy'n'paste (as is most of sanitize() body) from esc_html().\nThis line was added in 1df4876 (gitweb: Protect escaping functions against\ncalling on undef, 2010-02-07) with the following explanation\n\n    This is a bit of future-proofing esc_html and friends: when called\n    with undefined value they would now would return undef... which would\n    probably mean that error would still occur, but closer to the source\n    of problem.\n    \n    This means that we can safely use\n      esc_html(shift) || \"Internal Server Error\"\n    in die_error() instead of\n      esc_html(shift || \"Internal Server Error\")\n\nSo actually now I see that while this line is good to have in esc_html(),\nit is not really necessary in sanitize().\n\nBut anyway we don't want to replace undef with an empty string; undef is\n(usually) an error, and we want to catch it, not to hide it.\n \n> Given that the input to this function is from the result of feeding $line\n> to untabify, which relies on $line being defined, and that $line comes\n> from \"while (my $line = <$fd>)\" (and then chomp $line), it may be Ok for\n> this subroutine to make the same assumption as untabify makes.\n\nRight.\n\nPassing undef to sanitize() is usually an error, and we don't want to hide\nit.  We want for gitweb test to detect it.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"175661","messageId":"7v62ksfd78.fsf@alter.siamese.dyndns.org","threadId":"28195","inReplyTo":"201109162058.51132.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Strip non-printable characters from syntax highlighter output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-16T20:24:11Z","receivedAt":"2011-09-16T20:24:11Z","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> So actually now I see that while this line is good to have in esc_html(),\n> it is not really necessary in sanitize().\n>\n> But anyway we don't want to replace undef with an empty string; undef is\n> (usually) an error, and we want to catch it, not to hide it.\n\nHeh, get off your high horse---whoever wrote such a caller that calls the\nsubroutine and uses its result without checking it against undef is not\nqualified to make such a statement. I do not think letting \"perl -w\"\nnotice and complain about an attempt to concatenate undef with string\ncounts as \"catching\" it.\n"}]}