Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML()

From: Junio C Hamano <junkio@cox.net>
Date: Mar 6, 2007, 10:45 UTC
Message-ID: <7vzm6qps51.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <989B956029373F45A0B8AF02970818902DAA12@zch01exm26.fsl.freescale.net>

"Li Yang-r58472" <LeoLi@freescale.com> writes:

Show 9 quoted lines

>> -----Original Message-----
>> From: Jeff King [mailto:peff@peff.net]
>> ...
>> I think this should do the same, and is perhaps less magic (or maybe
>> more, depending on your perspective).
>
> Yes, it also fixed the problem.  I'm not very familiar with perl.  Will
> CGI::autoEscape(1) change CGI action for other users of CGI module on
> the system?  If so, maybe it will break other CGIs.

By "other CGIs" if you mean other independent CGI scripts that do not have anything to do with gitweb, then I do think there is no need to worry.

What I'd be worried about more, however, is if all the callers of esc_html and esc_path are really expecting the full quoting done by CGI::autoEscape(1). I think we had some discussion on the path quoting when we introduced quot_cec and quot_upr, but do not recall the details. For example, many places esc_html() is used as the body of <a ...>$here</a> but some places it is used as

    $cgi->a({ ... -title =>esc_html($fullname) }, esc_path($dir))

which would be the same as:

    print '<a title="' . esc_html($fullname) . '">' . esc_path($dir) . '</a>';

which may or may not be right (I do not know offhand).

Speaking of -title, I see "sub git_project_list_body" does this:

    $cgi->a({ ... -title => $pr->{'descr_long'}}, esc_html($pr->{'descr'}));
	
which seems inconsistent with the earlier quoted $fullname
handling (unless $pr->{'descr_long'} is already quoted and $pr->{'descr'}
is not, which I find highly unlikely).