Re: [PATCH] gitweb: filter escapes from longer commit titles that break firefox

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jakub Narebski wrote:
Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx> writes:

If there is a commit that ends in ^X and is longer in length than
what will fit in title_short, then it doesn't get fed through
esc_html() and so the ^X will appear as-is in the page source.

When Firefox comes across this, it will fail to display the page,
and only display a couple lines of error messages that read like:

   XML Parsing Error: not well-formed
   Location: http://git ....

Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>

This is an issue for when project doesn't follow sanity (control
characters in commit message) nor commit message conventions of git
(limiting length of first line of commit message to 60-70 characters).

I agree - the situation should be that it doesn't happen, but it can happen (and it did happen) that a novice, or a simple mistake ends up with such a commit.
But I do not think that the solution presented here is good solution
for this problem.  chop_and_escape_str is meant as _output_ filter,
because it generates (can generate) fragment of HTML.  It is not a
good solution to use it for shortening in intermediate representation
of %co{'title'}.

And I think that issue might be a bug elsewhere in gitweb if we have
text output which is not passed through esc_html... or bug in CGI.pm
if the error is in not escaping of -title _attribute_ (attribute
escaping has slightly different rules than escaping HTML, and should
be done automatically by CGI.pm).


So thanks for noticing the issue, but NAK on the solution.

Fair enough -- I wasn't familiar with the code in there, and there wasn't really any indication that it was for output only. I can easily believe that there is a better place for it -- I just didn't see where any global esc_html filtering was taking place...

Paul.

---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 33ef190..e686e82 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2470,7 +2470,7 @@ sub parse_commit_text {
 	foreach my $title (@commit_lines) {
 		$title =~ s/^    //;
 		if ($title ne "") {
-			$co{'title'} = chop_str($title, 80, 5);
+			$co{'title'} = chop_and_escape_str($title, 80, 5);
 			# remove leading stuff of merges to make the interesting part visible
 			if (length($title) > 50) {
 				$title =~ s/^Automatic //;
--
1.6.2.3



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]