threads / discuss / 24601

gitweb not friendly to firefox revived

Subject: gitweb not friendly to firefox revived

## tl;dr

12 messages between Aug 1, 2010 and Sep 7, 2010.

replies: 11people: 4as markdown or json

Uwe Kleine-König· Aug 1, 2010, 19:51 UTC · lore
Hello,
gitweb (at least) doesn't quote author names enough.
Firefox barfs for me at looking at
	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
with an error:
	XML Parsing Error: not well-formed Location:
http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
Line Number 112, Column 81:
<td class="author"><a title="Search for commits authored by YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B" class="list" href="/?p=ukl/linux-2.6.git;a=search;h=v2.6.16.10;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a></td><td><a class="list subject" title="[PATCH] IPV6: XFRM: Fix decoding session with preceding extension header(s)." href="/?p=ukl/linux-2.6.git;a=commit;h=fa39df2ff7f6102f1f37d3cf1f68243534d56253">[PATCH] IPV6: XFRM: Fix decoding session with preceding... </a></td>
--------------------------------------------------------------------------------^
This is with git 1.7.1 and Iceweasel (aka. Firefox) 3.5.10.
Making
	title=>"Search for commits $performed by $author"

in line 1694 of Debian's /usr/lib/cgi-bin/gitweb.cgi from the git 1.7.1 package read

	title=>esc_html("Search for commits $performed by $author")
this problem goes away.  (Still my browser barfs when clicking at the name.)

I'm not sure if this is the right way to fix this and I'm too tired now to do a complete patch, so I let this for someone else.

Best regards Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Ævar Arnfjörð Bjarmason· Aug 1, 2010, 20:15 UTC · re: Uwe Kleine-König · lore

Re: gitweb not friendly to firefox revived

2010/8/1 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
Show 12 quoted lines
> This is with git 1.7.1 and Iceweasel (aka. Firefox) 3.5.10.
>
> Making
>
>        title=>"Search for commits $performed by $author"
>
> in line 1694 of Debian's /usr/lib/cgi-bin/gitweb.cgi from the git 1.7.1
> package read
>
>        title=>esc_html("Search for commits $performed by $author")
>
> this problem goes away.  (Still my browser barfs when clicking at the name.)

That fix seems correct, the author appears to have control characters in his name. Probably as a result of some encoding corruption.

What's the URL of the Git repository itself? It'd be interesting to see how that went wrong in the first place.

Uwe Kleine-König· Aug 2, 2010, 05:31 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: gitweb not friendly to firefox revived

Hi,
> What's the URL of the Git repository itself? It'd be interesting to
> see how that went wrong in the first place.

it's just a copy of 2.6.16.y that is available at kernel.org: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.16.y.git

The commit happend before 2.6.16.10.

Best regards Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Jakub Narebski· Aug 1, 2010, 20:26 UTC · re: Uwe Kleine-König · lore

Re: gitweb not friendly to firefox revived

Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:
Show 31 quoted lines
> Hello,
> 
> gitweb (at least) doesn't quote author names enough.
> 
> Firefox barfs for me at looking at
> 
> 	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> 
> with an error:
> 
> 	XML Parsing Error: not well-formed Location:
> http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> Line Number 112, Column 81:
> <td class="author"><a title="Search for commits authored by YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B" class="list" href="/?p=ukl/linux-2.6.git;a=search;h=v2.6.16.10;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a></td><td><a class="list subject" title="[PATCH] IPV6: XFRM: Fix decoding session with preceding extension header(s)." href="/?p=ukl/linux-2.6.git;a=commit;h=fa39df2ff7f6102f1f37d3cf1f68243534d56253">[PATCH] IPV6: XFRM: Fix decoding session with preceding... </a></td>
> --------------------------------------------------------------------------------^
> 
> This is with git 1.7.1 and Iceweasel (aka. Firefox) 3.5.10.
> 
> Making
> 
> 	title=>"Search for commits $performed by $author"
> 
> in line 1694 of Debian's /usr/lib/cgi-bin/gitweb.cgi from the git 1.7.1
> package read
> 
> 	title=>esc_html("Search for commits $performed by $author")
> 
> this problem goes away.  (Still my browser barfs when clicking at the name.)
> 
> I'm not sure if this is the right way to fix this and I'm too tired now
> to do a complete patch, so I let this for someone else.
Actually gitweb leaves quoting of tag attributes to CGI module:
  return $cgi->a({-href => href(action=>"search", hash=>$hash,
                                searchtext=>$author, searchtype=>$searchtype),
                  -class => "list",
                  -title => "Search for commits $performed by $author"},
                 $displaytext);

I am worrying (perhaps unnecessary) that using esc_html would result in double escaping. But it looks like the problem is with Unicode, so perhaps using

  	title => to_utf8("Search for commits $performed by $author")
in place of
  	title=>esc_html("Search for commits $performed by $author")
would be a better fix?  Does this fix work for you?

Cc-ed Stephen Boyd, who is author of commit e133d65 (gitweb: linkify author/committer names with search, 2009-10-15), introducing the code you found this bug in.

-- 
Jakub Narebski
Poland
ShadeHawk on #git
Uwe Kleine-König· Aug 3, 2010, 21:07 UTC · re: Jakub Narebski · lore

Re: gitweb not friendly to firefox revived

On Sun, Aug 01, 2010 at 01:26:16PM -0700, Jakub Narebski wrote:
Show 53 quoted lines
> Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:
> 
> > Hello,
> > 
> > gitweb (at least) doesn't quote author names enough.
> > 
> > Firefox barfs for me at looking at
> > 
> > 	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> > 
> > with an error:
> > 
> > 	XML Parsing Error: not well-formed Location:
> > http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> > Line Number 112, Column 81:
> > <td class="author"><a title="Search for commits authored by YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B" class="list" href="/?p=ukl/linux-2.6.git;a=search;h=v2.6.16.10;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a></td><td><a class="list subject" title="[PATCH] IPV6: XFRM: Fix decoding session with preceding extension header(s)." href="/?p=ukl/linux-2.6.git;a=commit;h=fa39df2ff7f6102f1f37d3cf1f68243534d56253">[PATCH] IPV6: XFRM: Fix decoding session with preceding... </a></td>
> > --------------------------------------------------------------------------------^
> > 
> > This is with git 1.7.1 and Iceweasel (aka. Firefox) 3.5.10.
> > 
> > Making
> > 
> > 	title=>"Search for commits $performed by $author"
> > 
> > in line 1694 of Debian's /usr/lib/cgi-bin/gitweb.cgi from the git 1.7.1
> > package read
> > 
> > 	title=>esc_html("Search for commits $performed by $author")
> > 
> > this problem goes away.  (Still my browser barfs when clicking at the name.)
> > 
> > I'm not sure if this is the right way to fix this and I'm too tired now
> > to do a complete patch, so I let this for someone else.
> 
> Actually gitweb leaves quoting of tag attributes to CGI module:
> 
>   return $cgi->a({-href => href(action=>"search", hash=>$hash,
>                                 searchtext=>$author, searchtype=>$searchtype),
>                   -class => "list",
>                   -title => "Search for commits $performed by $author"},
>                  $displaytext);
> 
> I am worrying (perhaps unnecessary) that using esc_html would result
> in double escaping.  But it looks like the problem is with Unicode,
> so perhaps using
> 
>   	title => to_utf8("Search for commits $performed by $author")
> 
> in place of
> 
>   	title=>esc_html("Search for commits $performed by $author")
> 
> would be a better fix?  Does this fix work for you?
No, this doesn't help.  Firefox still barfs with to_utf8.
With esc_html the code generated is:
<a title="Search for commits authored by YOSHIFUJI Hideaki / <span class="cntrl">\e</span>$B5HF#1QL@<span class="cntrl">\e</span>(B" class="list" href="/?p=.git;a=search;h=f66ab685594d49e570b2176cfa20b03360e9a6e9;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a>

Thanks Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Jakub Narebski· Aug 3, 2010, 21:50 UTC · re: Uwe Kleine-König · lore

Re: gitweb not friendly to firefox revived

On Tue, Aug 03, 2010, Uwe Kleine-König wrote:
Show 60 quoted lines
> On Sun, Aug 01, 2010 at 01:26:16PM -0700, Jakub Narebski wrote:
> > Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:
> > 
> > > Hello,
> > > 
> > > gitweb (at least) doesn't quote author names enough.
> > > 
> > > Firefox barfs for me at looking at
> > > 
> > > 	http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> > > 
> > > with an error:
> > > 
> > > 	XML Parsing Error: not well-formed Location:
> > > http://git.pengutronix.de/?p=ukl/linux-2.6.git;a=shortlog;h=v2.6.16.10
> > > Line Number 112, Column 81:
> > > <td class="author"><a title="Search for commits authored by YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B" class="list" href="/?p=ukl/linux-2.6.git;a=search;h=v2.6.16.10;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a></td><td><a class="list subject" title="[PATCH] IPV6: XFRM: Fix decoding session with preceding extension header(s)." href="/?p=ukl/linux-2.6.git;a=commit;h=fa39df2ff7f6102f1f37d3cf1f68243534d56253">[PATCH] IPV6: XFRM: Fix decoding session with preceding... </a></td>
> > > --------------------------------------------------------------------------------^
> > > 
> > > This is with git 1.7.1 and Iceweasel (aka. Firefox) 3.5.10.
> > > 
> > > Making
> > > 
> > > 	title=>"Search for commits $performed by $author"
> > > 
> > > in line 1694 of Debian's /usr/lib/cgi-bin/gitweb.cgi from the git 1.7.1
> > > package read
> > > 
> > > 	title=>esc_html("Search for commits $performed by $author")
> > > 
> > > this problem goes away.  (Still my browser barfs when clicking at the name.)
> > > 
> > > I'm not sure if this is the right way to fix this and I'm too tired now
> > > to do a complete patch, so I let this for someone else.
> > 
> > Actually gitweb leaves quoting of tag attributes to CGI module:
> > 
> >   return $cgi->a({-href => href(action=>"search", hash=>$hash,
> >                                 searchtext=>$author, searchtype=>$searchtype),
> >                   -class => "list",
> >                   -title => "Search for commits $performed by $author"},
> >                  $displaytext);
> > 
> > I am worrying (perhaps unnecessary) that using esc_html would result
> > in double escaping.  But it looks like the problem is with Unicode,
> > so perhaps using
> > 
> >   	title => to_utf8("Search for commits $performed by $author")
> > 
> > in place of
> > 
> >   	title=>esc_html("Search for commits $performed by $author")
> > 
> > would be a better fix?  Does this fix work for you?
>
> No, this doesn't help.  Firefox still barfs with to_utf8.
> 
> With esc_html the code generated is:
> 
> <a title="Search for commits authored by YOSHIFUJI Hideaki / <span class="cntrl">\e</span>$B5HF#1QL@<span class="cntrl">\e</span>(B" class="list" href="/?p=.git;a=search;h=f66ab685594d49e570b2176cfa20b03360e9a6e9;s=YOSHIFUJI+Hideaki+/+%1B%24B5HF%231QL@%1B(B;st=author"><span title="YOSHIFUJI Hideaki / ?$B5HF#1QL@?(B">YOSHIFUJI Hideaki...  </span></a>

As you can see the HTML code generated with esc_html solution is way wrong because of embedded '<span class="cntrl">\e</span>' as you see _without_ '"' being escaped, so HTML is wrong.

Nevertheless it shows what's the problem. Somehow (perhaps wrong encoding, perhaps screw up with quoted-printable and git-am, perhaps copy'n' paste included ANSII color codes from terminal, perhaps something different altogether) you got control characters (\e = ESC) in $author. In strict XHTML mode (with 'application/xml

Please try the following patch
-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH] gitweb: Harden format_search_author()

Protect format_search_author against control characters in $author. While at it simplify it a bit, and use spaces for align.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   29 ++++++++++++++---------------
 1 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8b02767..ea9c09c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1856,23 +1856,22 @@ sub format_search_author {
 	my ($author, $searchtype, $displaytext) = @_;
 	my $have_search = gitweb_check_feature('search');
 
-	if ($have_search) {
-		my $performed = "";
-		if ($searchtype eq 'author') {
-			$performed = "authored";
-		} elsif ($searchtype eq 'committer') {
-			$performed = "committed";
-		}
-
-		return $cgi->a({-href => href(action=>"search", hash=>$hash,
-				searchtext=>$author,
-				searchtype=>$searchtype), class=>"list",
-				title=>"Search for commits $performed by $author"},
-				$displaytext);
+	return $displaytext unless ($have_search);
 
-	} else {
-		return $displaytext;
+	my $performed = "";
+	if ($searchtype eq 'author') {
+		$performed = "authored";
+	} elsif ($searchtype eq 'committer') {
+		$performed = "committed";
 	}
+
+	my $title = to_utf8("Search for commits $performed by $author");
+	$title =~ s/[[:cntrl:]]/?/g;
+
+	return $cgi->a({-href => href(action=>"search", hash=>$hash,
+	                              searchtext=>$author, searchtype=>$searchtype),
+	                -class=>"list", -title=>$title},
+	               $displaytext);
 }
 
 # format the author name of the given commit with the given tag
-- 
1.7.2.1
Uwe Kleine-König· Aug 12, 2010, 09:23 UTC · re: Jakub Narebski · lore

Re: gitweb not friendly to firefox revived

Hello,
Show 60 quoted lines
> Nevertheless it shows what's the problem.  Somehow (perhaps wrong
> encoding, perhaps screw up with quoted-printable and git-am, perhaps
> copy'n' paste included ANSII color codes from terminal, perhaps something
> different altogether) you got control characters (\e = ESC) in $author.
> In strict XHTML mode (with 'application/xml
> 
> Please try the following patch
> 
> -- >8 --
> From: Jakub Narebski <jnareb@gmail.com>
> Subject: [PATCH] gitweb: Harden format_search_author()
> 
> Protect format_search_author against control characters in $author.
> While at it simplify it a bit, and use spaces for align.
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
>  gitweb/gitweb.perl |   29 ++++++++++++++---------------
>  1 files changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8b02767..ea9c09c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1856,23 +1856,22 @@ sub format_search_author {
>  	my ($author, $searchtype, $displaytext) = @_;
>  	my $have_search = gitweb_check_feature('search');
>  
> -	if ($have_search) {
> -		my $performed = "";
> -		if ($searchtype eq 'author') {
> -			$performed = "authored";
> -		} elsif ($searchtype eq 'committer') {
> -			$performed = "committed";
> -		}
> -
> -		return $cgi->a({-href => href(action=>"search", hash=>$hash,
> -				searchtext=>$author,
> -				searchtype=>$searchtype), class=>"list",
> -				title=>"Search for commits $performed by $author"},
> -				$displaytext);
> +	return $displaytext unless ($have_search);
>  
> -	} else {
> -		return $displaytext;
> +	my $performed = "";
> +	if ($searchtype eq 'author') {
> +		$performed = "authored";
> +	} elsif ($searchtype eq 'committer') {
> +		$performed = "committed";
>  	}
> +
> +	my $title = to_utf8("Search for commits $performed by $author");
> +	$title =~ s/[[:cntrl:]]/?/g;
> +
> +	return $cgi->a({-href => href(action=>"search", hash=>$hash,
> +	                              searchtext=>$author, searchtype=>$searchtype),
> +	                -class=>"list", -title=>$title},
> +	               $displaytext);
>  }

Seems to do the right thing. With Firefox I get a correct listing now instead of the xml parse error.

Thanks Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Stephen Boyd· Aug 14, 2010, 10:33 UTC · re: Jakub Narebski · lore

Re: gitweb not friendly to firefox revived

  On 08/03/2010 02:50 PM, Jakub Narebski wrote:
Show 5 quoted lines
> +
> +	my $title = to_utf8("Search for commits $performed by $author");
> +	$title =~ s/[[:cntrl:]]/?/g;
> +
>
Isn't it possible that other data coming from git could have escape characters in them such as the commit subject line? In which case this same bug would occur?
Therefore isn't it better to strip out control characters (that's what this patch is doing right?) in esc_html?
Ævar Arnfjörð Bjarmason· Aug 14, 2010, 10:48 UTC · re: Stephen Boyd · lore

Re: gitweb not friendly to firefox revived

2010/8/14 Stephen Boyd <bebarino@gmail.com>:
Show 14 quoted lines
>  On 08/03/2010 02:50 PM, Jakub Narebski wrote:
>>
>> +
>> +       my $title = to_utf8("Search for commits $performed by $author");
>> +       $title =~ s/[[:cntrl:]]/?/g;
>> +
>>
>
> Isn't it possible that other data coming from git could have escape
> characters in them such as the commit subject line? In which case this same
> bug would occur?
>
> Therefore isn't it better to strip out control characters (that's what this
> patch is doing right?) in esc_html?

I don't think stripping them out is the right thing either, hiding from you that something is Really Wrong (binary garbage in patches) isn't good.

Something like this would be better:
    s/([[:cntrl:]])/sprintf("\\%03x", ord $1)/ge
E.g.:
    $ perl -E 'my $s = join "", map { chr } 1 .. 40; $s =~
s/([[:cntrl:]])/sprintf("\\%03x", ord $1)/ge; say $s'
    \001\002\003\004\005\006\007\008\009\00a\00b\00c\00d\00e\00f\010\011\012\013\014\015\016\017\018\019\01a\01b\01c\01d\01e\01f
!"#$%&'(
Jakub Narebski· Aug 14, 2010, 12:33 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: gitweb not friendly to firefox revived

On Sat, 14 Aug 2010, Ævar Arnfjörð Bjarmason wrote:
Show 23 quoted lines
> 2010/8/14 Stephen Boyd <bebarino@gmail.com>:
>>  On 08/03/2010 02:50 PM, Jakub Narebski wrote:
>>>
>>> +
>>> +       my $title = to_utf8("Search for commits $performed by $author");
>>> +       $title =~ s/[[:cntrl:]]/?/g;
>>> +
>>>
>>
>> Isn't it possible that other data coming from git could have escape
>> characters in them such as the commit subject line? In which case this same
>> bug would occur?
>>
>> Therefore isn't it better to strip out control characters (that's what this
>> patch is doing right?) in esc_html?
> 
> I don't think stripping them out is the right thing either, hiding
> from you that something is Really Wrong (binary garbage in patches)
> isn't good.
> 
> Something like this would be better:
> 
>     s/([[:cntrl:]])/sprintf("\\%03x", ord $1)/ge
Or 
      s|([[:cntrl:]])|quot_cec($1)|eg;

But is it worth it? This is about _title_ attribute, shown only on mouseover (mouse hover).

But perhaps it would be worth it to add 'prep_attr' and 'esc_attr' functions, though esc_html can be used in those places where esc_attr would be needed...

-- 
Jakub Narebski
Poland
Uwe Kleine-König· Sep 7, 2010, 08:22 UTC · re: Jakub Narebski · lore

Re: gitweb not friendly to firefox revived

Hello,
just to stop this topic dying ...
On Sat, Aug 14, 2010 at 02:33:33PM +0200, Jakub Narebski wrote:
Show 35 quoted lines
> On Sat, 14 Aug 2010, Ævar Arnfjörð Bjarmason wrote:
> > 2010/8/14 Stephen Boyd <bebarino@gmail.com>:
> >>  On 08/03/2010 02:50 PM, Jakub Narebski wrote:
> >>>
> >>> +
> >>> +       my $title = to_utf8("Search for commits $performed by $author");
> >>> +       $title =~ s/[[:cntrl:]]/?/g;
> >>> +
> >>>
> >>
> >> Isn't it possible that other data coming from git could have escape
> >> characters in them such as the commit subject line? In which case this same
> >> bug would occur?
> >>
> >> Therefore isn't it better to strip out control characters (that's what this
> >> patch is doing right?) in esc_html?
> > 
> > I don't think stripping them out is the right thing either, hiding
> > from you that something is Really Wrong (binary garbage in patches)
> > isn't good.
> > 
> > Something like this would be better:
> > 
> >     s/([[:cntrl:]])/sprintf("\\%03x", ord $1)/ge
> 
> Or 
>       s|([[:cntrl:]])|quot_cec($1)|eg;
> 
> But is it worth it?  This is about _title_ attribute, shown only on 
> mouseover (mouse hover).
> 
> 
> But perhaps it would be worth it to add 'prep_attr' and 'esc_attr'
> functions, though esc_html can be used in those places where esc_attr
> would be needed...

Is there something I can do to bring this forward? (I assume this isn't fixed yet, at least it isn't in Debian's 1.7.2.3 package.)

Best regards Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Jakub Narebski· Aug 14, 2010, 12:29 UTC · re: Stephen Boyd · lore

Re: gitweb not friendly to firefox revived

On Sat, 14 Aug 2010, Stephen Boyd wrote:
Show 13 quoted lines
>   On 08/03/2010 02:50 PM, Jakub Narebski wrote:
> > +
> > +	my $title = to_utf8("Search for commits $performed by $author");
> > +	$title =~ s/[[:cntrl:]]/?/g;
> > +
> >
> 
> Isn't it possible that other data coming from git could have escape
> characters in them such as the commit subject line? In which case this
> same bug would occur?  
> 
> Therefore isn't it better to strip out control characters (that's what
> this patch is doing right?) in esc_html? 

First, esc_html and esc_path *do* escape control characters using either control escape characters (e.g. "\n" for LF), or escaped octal representation (e.g. "\001").

Second, it does not help with contents of *attributes* of HTML tag elements (like e.g. 'title' attribute) when those elements are generated using CGI (e.g. $cgi->a({..., -title => ...},esc_html(...))). Unfortunately (older?) CGI.pm does not escape control characters, and we cannot do escape ourselves because it would lead to double escaping.

The problem with Firefox is that in strict XHTML conformance model (XHTML DTD and application/xhtml+xml mimetype) it *enforces* that XML is well formed, which includes lack of control characters, instead of silently allowing it like in more loose HTML mode.

-- 
Jakub Narebski
Poland

← back to recent threads