[PATCH] gitweb: protect blob and diff output lines from controls.

This reuses the quot_cec to protect blob and text diff output
from leaking control characters.

Signed-off-by: Junio C Hamano <redacted>
---
 * requesting extra sets of eyeballs.

 gitweb/gitweb.perl |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f46d678..b5b1011 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -597,11 +597,10 @@ sub esc_html ($;%) {
 
 	$str = to_utf8($str);
 	$str = escapeHTML($str);
-	$str =~ s/\014/^L/g; # escape FORM FEED (FF) character (e.g. in COPYING file)
-	$str =~ s/\033/^[/g; # "escape" ESCAPE (\e) character (e.g. commit 20a3847d8a5032ce41f90dcc68abfb36e6fee9b1)
 	if ($opts{'-nbsp'}) {
 		$str =~ s/ / /g;
 	}
+	$str =~ s|([[:cntrl:]])|(($1 ne "\t") ? quot_cec($1) : $1)|eg;
 	return $str;
 }
 

@@ -1900,17 +1899,17 @@ sub git_print_page_path {
 			$fullname .= ($fullname ? '/' : '') . $dir;
 			print $cgi->a({-href => href(action=>"tree", file_name=>$fullname,
 			                             hash_base=>$hb),
-			              -title => $fullname}, esc_path($dir));
+			              -title => esc_html($fullname)}, esc_path($dir));
 			print " / ";
 		}
 		if (defined $type && $type eq 'blob') {
 			print $cgi->a({-href => href(action=>"blob_plain", file_name=>$file_name,
 			                             hash_base=>$hb),
-			              -title => $name}, esc_path($basename));
+			              -title => esc_html($name)}, esc_path($basename));
 		} elsif (defined $type && $type eq 'tree') {
 			print $cgi->a({-href => href(action=>"tree", file_name=>$file_name,
 			                             hash_base=>$hb),
-			              -title => $name}, esc_path($basename));
+			              -title => esc_html($name)}, esc_path($basename));
 			print " / ";
 		} else {
 			print esc_path($basename);

-- 
1.4.4.rc1.g659d

Jakub Narebski wrote:

I'm not sure what quoting to choose for esc_attr, but there we could
use even --no-control-chars quoting (replacing any control character
by '?');  but perhaps in some cases like git_print_page_path
subroutine CEC is better.

I'm rambling. esc_attr is special case, because CGI does escapeHTML
(and I hope also to_utf8) for us. Using <span class="cntrl">...</span>
has also no sense. So there should be separate esc_attr_path subroutine
I think.

Even if we decide that esc_html and esc_path should give identical
output (the difference that _might_ be here is that in esc_html we
don't need to escape whitespace control characters valid in HTML,
like tab (HT, TAB) or newline (LF); on the other hand thanks to
line-by-line processing we should never get newline in "blob", and
thanks to untabify we should never get tab in "blob") I think it would
be prudent to have esc_path, even as thin wrapper just caling esc_html.

We might decide to use different style for control characters in
different views, but that I think can be done using pure CSS.
-- 
Jakub Narebski

Junio C Hamano wrote:

This reuses the quot_cec to protect blob and text diff output
from leaking control characters.

Signed-off-by: Junio C Hamano <redacted>
---
 * requesting extra sets of eyeballs.

This changes the "blob" and "text diff" output somewhat, as earlier it 
used Control key Sequence (CS) representation for some non-whitespace 
control characters (not "\t' not '\n'), namely replacing form feed (FF) 
('\f', '\014') with ^L and escape (ESC) ('\e', '\033') with ^[.

And (what is not said in the commit message) it additionally esc_html 
some title elements (the subroutine should be I think named esc_attr).

The problems are:
1. First, esc_path should _not_ use subroutine which does it's own 
contol characters escaping. That was also a mistake I made in my patch.
Perhaps we should have some quot_html or to_html subroutine which does 
_only_ to_utf8 (decode from Encode module), escapeHTML and optionally 
s/ / /g conversion.

2. In my opinion CS is better than CEC for quoting/escaping control 
characters in the "bulk" output, namely "blob" output and "text 
diff" (patchset body) output. CEC is better for pathnames (which must 
fit in one line), and perhaps other one-liners; perhaps not. I'm not 
sure what quoting to choose for esc_attr, but there we could use even 
--no-control-chars quoting (replacing any control character by '?'); 
but perhaps in some cases like git_print_page_path subroutine CEC is 
better.

BTW. what had happened with to_qtext post?
-- 
Jakub Narebski