# [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() 21 messages from 2007-03-06 to 2007-03-07. Participants: Li Yang, Junio C Hamano, Jakub Narebski, Jeff King, Li Yang-r58472. Thread: https://gitlist.dev/t/7101 ## Li Yang, 2007-03-06 03:58 Subject: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <45ECE700.8090205@freescale.com> URL: https://gitlist.dev/e/45ECE700.8090205%40freescale.com ``` Change to use explicitly function call cgi->escapHTML(). This fix the problem on some systems that escapeHTML() is not functioning, as default CGI is not setting 'escape' parameter. Signed-off-by: Li Yang --- diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 653ca3c..3a564d1 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -591,7 +591,7 @@ sub esc_html ($;%) { my %opts = @_; $str = to_utf8($str); - $str = escapeHTML($str); + $str = $cgi->escapeHTML($str); if ($opts{'-nbsp'}) { $str =~ s/ / /g; } @@ -605,7 +605,7 @@ sub esc_path { my %opts = @_; $str = to_utf8($str); - $str = escapeHTML($str); + $str = $cgi->escapeHTML($str); if ($opts{'-nbsp'}) { $str =~ s/ / /g; } ``` ## Junio C Hamano, 2007-03-06 06:55 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7v649euai8.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v649euai8.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <45ECE700.8090205@freescale.com> ``` Li Yang writes: > Change to use explicitly function call cgi->escapHTML(). > This fix the problem on some systems that escapeHTML() is not > functioning, as default CGI is not setting 'escape' parameter. > > Signed-off-by: Li Yang Regardless of the recent xhtml+html vs html discussion, I think this is probably a sane change. Comments? ``` ## Jakub Narebski, 2007-03-06 09:34 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <8fe92b430703060134l14fffcc4rbece3c2071c56422@mail.gmail.com> URL: https://gitlist.dev/e/8fe92b430703060134l14fffcc4rbece3c2071c56422%40mail.gmail.com In-Reply-To: <7v649euai8.fsf@assigned-by-dhcp.cox.net> ``` On 3/6/07, Junio C Hamano wrote: > Li Yang writes: > > > Change to use explicitly function call cgi->escapHTML(). > > This fix the problem on some systems that escapeHTML() is not > > functioning, as default CGI is not setting 'escape' parameter. > > > > Signed-off-by: Li Yang > > Regardless of the recent xhtml+html vs html discussion, I think > this is probably a sane change. Comments? Good (although a bit magic) solution. Ack, FWIW. -- Jakub Narebski ``` ## Jeff King, 2007-03-06 09:39 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <20070306093917.GA1761@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070306093917.GA1761%40coredump.intra.peff.net In-Reply-To: <8fe92b430703060134l14fffcc4rbece3c2071c56422@mail.gmail.com> ``` On Tue, Mar 06, 2007 at 10:34:32AM +0100, Jakub Narebski wrote: > >Regardless of the recent xhtml+html vs html discussion, I think > >this is probably a sane change. Comments? > Good (although a bit magic) solution. Ack, FWIW. I think this should do the same, and is perhaps less magic (or maybe more, depending on your perspective). -Peff -- >8 -- diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 653ca3c..5d1d8cf 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -17,6 +17,7 @@ use Fcntl ':mode'; use File::Find qw(); use File::Basename qw(basename); binmode STDOUT, ':utf8'; +CGI::autoEscape(1); BEGIN { CGI->compile() if $ENV{MOD_PERL}; ``` ## Junio C Hamano, 2007-03-06 09:46 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7vwt1ur9fs.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwt1ur9fs.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20070306093917.GA1761@coredump.intra.peff.net> ``` Jeff King writes: > On Tue, Mar 06, 2007 at 10:34:32AM +0100, Jakub Narebski wrote: > >> >Regardless of the recent xhtml+html vs html discussion, I think >> >this is probably a sane change. Comments? >> Good (although a bit magic) solution. Ack, FWIW. > > I think this should do the same, and is perhaps less magic (or maybe > more, depending on your perspective). > > -Peff Thanks. I tend to agree, as it does not depend on the reader knowing what magic $cgi default behaviour is by being expliicit. > > -- >8 -- > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index 653ca3c..5d1d8cf 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -17,6 +17,7 @@ use Fcntl ':mode'; > use File::Find qw(); > use File::Basename qw(basename); > binmode STDOUT, ':utf8'; > +CGI::autoEscape(1); > > BEGIN { > CGI->compile() if $ENV{MOD_PERL}; ``` ## Li Yang-r58472, 2007-03-06 10:31 Subject: RE: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <989B956029373F45A0B8AF02970818902DAA12@zch01exm26.fsl.freescale.net> URL: https://gitlist.dev/e/989B956029373F45A0B8AF02970818902DAA12%40zch01exm26.fsl.freescale.net In-Reply-To: <20070306093917.GA1761@coredump.intra.peff.net> ``` > -----Original Message----- > From: Jeff King [mailto:peff@peff.net] > Sent: Tuesday, March 06, 2007 5:39 PM > To: Jakub Narebski > Cc: Junio C Hamano; Li Yang-r58472; git@vger.kernel.org > Subject: Re: [PATCH] gitweb: Change to use explicitly function call > cgi->escapHTML() > > On Tue, Mar 06, 2007 at 10:34:32AM +0100, Jakub Narebski wrote: > > > >Regardless of the recent xhtml+html vs html discussion, I think > > >this is probably a sane change. Comments? > > Good (although a bit magic) solution. Ack, FWIW. > > 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. - Leo ``` ## Jeff King, 2007-03-06 10:41 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <20070306104127.GA13096@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070306104127.GA13096%40coredump.intra.peff.net In-Reply-To: <989B956029373F45A0B8AF02970818902DAA12@zch01exm26.fsl.freescale.net> ``` On Tue, Mar 06, 2007 at 06:31:23PM +0800, Li Yang-r58472 wrote: > 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. I don't know enough about mod_perl to say, but if all scripts share the package globals from CGI, then yes, you're affecting all other scripts. Without mod_perl, obviously you have no impact. If it is the case, then your original fix is probably better. -Peff ``` ## Junio C Hamano, 2007-03-06 10:45 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7vzm6qps51.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vzm6qps51.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <989B956029373F45A0B8AF02970818902DAA12@zch01exm26.fsl.freescale.net> ``` "Li Yang-r58472" writes: >> -----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 $here but some places it is used as $cgi->a({ ... -title =>esc_html($fullname) }, esc_path($dir)) which would be the same as: print '' . esc_path($dir) . ''; 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). ``` ## Junio C Hamano, 2007-03-06 10:53 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7vveheprsc.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vveheprsc.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20070306104127.GA13096@coredump.intra.peff.net> ``` Jeff King writes: > On Tue, Mar 06, 2007 at 06:31:23PM +0800, Li Yang-r58472 wrote: > >> 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. > > I don't know enough about mod_perl to say, but if all scripts share the > package globals from CGI, then yes, you're affecting all other scripts. > Without mod_perl, obviously you have no impact. > > If it is the case, then your original fix is probably better. But then you are letting _other_ mod_perl users to affect your behaviour, aren't you? "sub autoEscape" does this: sub autoEscape { my($self,$escape) = self_or_default(@_); my $d = $self->{'escape'}; $self->{'escape'} = $escape; $d; } If we worry about mod_perl (provided if $CGI::Q is shared across mod_perl users), I suspect we would need to be a bit more paranoid, perhaps like this, woudln't we? --- diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 653ca3c..9c4e060 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -26,6 +26,7 @@ our $cgi = new CGI; our $version = "++GIT_VERSION++"; our $my_url = $cgi->url(); our $my_uri = $cgi->url(-absolute => 1); +$cgi->autoEscape(1); # core git executable to use # this can just be "git" if your webserver has a sensible PATH ``` ## Jeff King, 2007-03-06 10:56 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <20070306105629.GA13285@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070306105629.GA13285%40coredump.intra.peff.net In-Reply-To: <7vveheprsc.fsf@assigned-by-dhcp.cox.net> ``` On Tue, Mar 06, 2007 at 02:53:07AM -0800, Junio C Hamano wrote: > But then you are letting _other_ mod_perl users to affect your > behaviour, aren't you? "sub autoEscape" does this: Yes (but I don't know how mod_perl works, and I haven't been able to find a simple answer by skimming the docs). > If we worry about mod_perl (provided if $CGI::Q is shared across > mod_perl users), I suspect we would need to be a bit more > paranoid, perhaps like this, woudln't we? > [...] > +$cgi->autoEscape(1); That rebreaks the original problem, though. Calling escapeHTML doesn't look at $cgi, it looks at $Q (the "default" CGI object). I believe escape is _already_ set to 1 for $cgi (which is why the $cgi->escapeHTML patch worked). -Peff ``` ## Junio C Hamano, 2007-03-06 10:58 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7vps7mprj6.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vps7mprj6.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20070306105629.GA13285@coredump.intra.peff.net> ``` Jeff King writes: > On Tue, Mar 06, 2007 at 02:53:07AM -0800, Junio C Hamano wrote: > >> But then you are letting _other_ mod_perl users to affect your >> behaviour, aren't you? "sub autoEscape" does this: > > Yes (but I don't know how mod_perl works, and I haven't been able to > find a simple answer by skimming the docs). > >> If we worry about mod_perl (provided if $CGI::Q is shared across >> mod_perl users), I suspect we would need to be a bit more >> paranoid, perhaps like this, woudln't we? >> [...] >> +$cgi->autoEscape(1); > > That rebreaks the original problem, though. Sorry, what I meant to say was on top of Li's patch. ``` ## Jeff King, 2007-03-06 11:01 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <20070306110156.GA13380@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070306110156.GA13380%40coredump.intra.peff.net In-Reply-To: <7vps7mprj6.fsf@assigned-by-dhcp.cox.net> ``` On Tue, Mar 06, 2007 at 02:58:37AM -0800, Junio C Hamano wrote: > Sorry, what I meant to say was on top of Li's patch. Ah, I understand your point now. Yes, if other mod_perl CGIs can impact the value, then we should definitely set it explicitly, as per your patch (and we should use Li's patch for safety, then, not mine). -Peff ``` ## Junio C Hamano, 2007-03-06 11:05 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7vlkiapr70.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vlkiapr70.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20070306110156.GA13380@coredump.intra.peff.net> ``` Jeff King writes: > On Tue, Mar 06, 2007 at 02:58:37AM -0800, Junio C Hamano wrote: > >> Sorry, what I meant to say was on top of Li's patch. > > Ah, I understand your point now. Yes, if other mod_perl CGIs can impact > the value, then we should definitely set it explicitly, as per your > patch (and we should use Li's patch for safety, then, not mine). Reading "sub autoEscape", "sub escapeHTML" and "sub self_or_default" again, I think other people cannot affect the value of our $cgi->{'escape'} by calling autoEscape, so what I said is probably bogus. Let's use Li's original patch. ``` ## Li Yang-r58472, 2007-03-06 11:07 Subject: RE: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <989B956029373F45A0B8AF02970818902DAA14@zch01exm26.fsl.freescale.net> URL: https://gitlist.dev/e/989B956029373F45A0B8AF02970818902DAA14%40zch01exm26.fsl.freescale.net In-Reply-To: <20070306110156.GA13380@coredump.intra.peff.net> ``` > -----Original Message----- > From: Jeff King [mailto:peff@peff.net] > Sent: Tuesday, March 06, 2007 7:02 PM > To: Junio C Hamano > Cc: Li Yang-r58472; Jakub Narebski; git@vger.kernel.org > Subject: Re: [PATCH] gitweb: Change to use explicitly function call > cgi->escapHTML() > > On Tue, Mar 06, 2007 at 02:58:37AM -0800, Junio C Hamano wrote: > > > Sorry, what I meant to say was on top of Li's patch. > > Ah, I understand your point now. Yes, if other mod_perl CGIs can impact > the value, then we should definitely set it explicitly, as per your > patch (and we should use Li's patch for safety, then, not mine). I agree. -Leo ``` ## Jeff King, 2007-03-06 11:07 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <20070306110754.GA13470@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070306110754.GA13470%40coredump.intra.peff.net In-Reply-To: <7vlkiapr70.fsf@assigned-by-dhcp.cox.net> ``` On Tue, Mar 06, 2007 at 03:05:55AM -0800, Junio C Hamano wrote: > > Ah, I understand your point now. Yes, if other mod_perl CGIs can impact > > the value, then we should definitely set it explicitly, as per your > > patch (and we should use Li's patch for safety, then, not mine). > > Reading "sub autoEscape", "sub escapeHTML" and "sub > self_or_default" again, I think other people cannot affect the > value of our $cgi->{'escape'} by calling autoEscape, so what I > said is probably bogus. Let's use Li's original patch. Er, sorry, yes, I just accidentally agreed with your bogosity (while figuring out what you originally meant, I forgot which CGI we were talking about!). So I agree, Li's original is sufficient. Sorry for the noise. :) -Peff ``` ## Jakub Narebski, 2007-03-06 13:23 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <200703061423.18417.jnareb@gmail.com> URL: https://gitlist.dev/e/200703061423.18417.jnareb%40gmail.com In-Reply-To: <7vzm6qps51.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > 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). CGI::a() subroutine automatically quotes properly _attribute_ values, but it does not (and it should not) quote _contents_ of a tag. So the above code is correct. -- Jakub Narebski Poland ``` ## Junio C Hamano, 2007-03-06 23:17 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7vzm6qm07l.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vzm6qm07l.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <200703061423.18417.jnareb@gmail.com> ``` Jakub Narebski writes: > Junio C Hamano wrote: > >> 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). > > CGI::a() subroutine automatically quotes properly _attribute_ values, > but it does not (and it should not) quote _contents_ of a tag. > > So the above code is correct. Sorry, you lost me... I am wondering what you mean by "automatically". Do you mean 'always'? And if that is the case, shouldn't we drop esc_html() around $fullname here? ... For example, many places esc_html() is used as the body of $here but some places it is used as $cgi->a({ ... -title =>esc_html($fullname) }, esc_path($dir)) as we do not have it around $pr->{'descr_long'} in the above? ``` ## Jakub Narebski, 2007-03-07 00:37 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <200703070137.07477.jnareb@gmail.com> URL: https://gitlist.dev/e/200703070137.07477.jnareb%40gmail.com In-Reply-To: <7vzm6qm07l.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Jakub Narebski writes: >> Junio C Hamano wrote: >> >>> 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). >> >> CGI::a() subroutine automatically quotes properly _attribute_ values, >> but it does not (and it should not) quote _contents_ of a tag. >> >> So the above code is correct. > > Sorry, you lost me... I am wondering what you mean by > "automatically". Do you mean 'always'? Yes, I mean that CGI::a() does quoting _of attributes_, always. > And if that is the case, shouldn't we drop esc_html() around > $fullname here? > > ... For example, many places esc_html() > is used as the body of $here but some places it is > used as > > $cgi->a({ ... -title =>esc_html($fullname) }, esc_path($dir)) > > as we do not have it around $pr->{'descr_long'} in the above? The above is wrong, thrice. First, it should be esc_path($fullname). Second, rules for escaping attribute values are different from escaping HTML. Third, CGI::a() does escaping of attribute values. Explanation: $cgi->a({ ... -attribute => atribute_value }, tag_contents) is translated to tag_contents The rules for escaping attribute values (which are string contents) are different. For example you have to take care about escaping embedded '"' and "'" characters; CGI::a() does that for us automatically. CGI::a() cannot HTML escape tag contents automatically; we might want to write some bold text for example. Soe we have to esc_html (or esc_path) if needed. In short: escape tag contents if needed, do not escape attrbure values. -- Jakub Narebski Poland ``` ## Junio C Hamano, 2007-03-07 00:49 Subject: Re: [PATCH] gitweb: Change to use explicitly function call cgi->escapHTML() Message-ID: <7vvehdnaib.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vvehdnaib.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <200703070137.07477.jnareb@gmail.com> ``` Jakub Narebski writes: > In short: escape tag contents if needed, do not escape attrbure values. I trust a patch from you will follow shortly? ``` ## Jakub Narebski, 2007-03-07 01:21 Subject: [PATCH] gitweb: Don't escape attributes in CGI.pm HTML methods Message-ID: <200703070221.25519.jnareb@gmail.com> URL: https://gitlist.dev/e/200703070221.25519.jnareb%40gmail.com In-Reply-To: <7vvehdnaib.fsf@assigned-by-dhcp.cox.net> ``` There is no need to escape HTML tag's attributes in CGI.pm HTML methods (like CGI::a()), because CGI.pm does attribute escaping automatically. Explanation: $cgi->a({ ... -attribute => atribute_value }, tag_contents) is translated to tag_contents The rules for escaping attribute values (which are string contents) are different. For example you have to take care about escaping embedded '"' and "'" characters; CGI::a() does that for us automatically. CGI::a() cannot HTML escape tag contents automatically; we might want to write some bold text for example. So we have to esc_html (or esc_path) if needed. Signed-off-by: Jakub Narebski --- Junio C Hamano wrote: > Jakub Narebski writes: > >> In short: escape tag contents if needed, do not escape attrbure values. > > I trust a patch from you will follow shortly? Here it is. I hope I found everything. Commit message is bit long, so you can cut it to first sentence only (or even only to title/subject). gitweb/gitweb.perl | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 653ca3c..ea58946 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1974,17 +1974,17 @@ sub git_print_page_path { $fullname .= ($fullname ? '/' : '') . $dir; print $cgi->a({-href => href(action=>"tree", file_name=>$fullname, hash_base=>$hb), - -title => esc_html($fullname)}, esc_path($dir)); + -title => $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 => esc_html($name)}, esc_path($basename)); + -title => $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 => esc_html($name)}, esc_path($basename)); + -title => $name}, esc_path($basename)); print " / "; } else { print esc_path($basename); -- 1.5.0.2 ``` ## Junio C Hamano, 2007-03-07 01:40 Subject: Re: [PATCH] gitweb: Don't escape attributes in CGI.pm HTML methods Message-ID: <7vk5xtn84z.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vk5xtn84z.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <200703070221.25519.jnareb@gmail.com> ``` Jakub Narebski writes: > There is no need to escape HTML tag's attributes in CGI.pm > HTML methods (like CGI::a()), because CGI.pm does attribute > escaping automatically. > > Explanation: > $cgi->a({ ... -attribute => atribute_value }, tag_contents) > is translated to > tag_contents > The rules for escaping attribute values (which are string contents) are > different. For example you have to take care about escaping embedded '"' > and "'" characters; CGI::a() does that for us automatically. > > CGI::a() cannot HTML escape tag contents automatically; we might want to > write > some bold text > for example. So we have to esc_html (or esc_path) if needed. > > Signed-off-by: Jakub Narebski > --- > Junio C Hamano wrote: >> Jakub Narebski writes: >> >>> In short: escape tag contents if needed, do not escape attrbure values. >> >> I trust a patch from you will follow shortly? > > Here it is. I hope I found everything. > > Commit message is bit long, so you can cut it to first sentence only > (or even only to title/subject). Thanks. I think your explanation in the log message has the right amount of details and keeping it there would help people who would want to later touch the code. ```