git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jakub Narebski <jnareb@gmail.com>
Date
Mar 7, 2007, 00:37 UTC
Message-ID
<200703070137.07477.jnareb@gmail.com>
In-Reply-To
<7vzm6qm07l.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
Show 18 quoted lines
> Jakub Narebski <jnareb@gmail.com> 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.
 
Show 10 quoted lines
> 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 <a ...>$here</a> 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
  <a ... attribute="attribute_value">tag_contents</a>

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

  <a href="URL">some <b>bold</b> text</a>
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
Previous: Junio C HamanoNext: Junio C Hamano
Message 18 of 21 in “gitweb: Change to use explicitly function call cgi->escapHTML()”
  1. gitweb: Change to use explicitly function call cgi->escapHTML()Li Yang, Mar 6, 2007
  2. Junio C HamanoMar 6, 2007
  3. Jakub NarebskiMar 6, 2007
  4. Jeff KingMar 6, 2007
  5. Junio C HamanoMar 6, 2007
  6. Li Yang-r58472Mar 6, 2007
  7. Jeff KingMar 6, 2007
  8. Junio C HamanoMar 6, 2007
  9. Jeff KingMar 6, 2007
  10. Junio C HamanoMar 6, 2007
  11. Jeff KingMar 6, 2007
  12. Junio C HamanoMar 6, 2007
  13. Jeff KingMar 6, 2007
  14. Li Yang-r58472Mar 6, 2007
  15. Junio C HamanoMar 6, 2007
  16. Jakub NarebskiMar 6, 2007
  17. Junio C HamanoMar 6, 2007
  18. Jakub NarebskiMar 7, 2007
  19. Junio C HamanoMar 7, 2007
  20. gitweb: Don't escape attributes in CGI.pm HTML methodsJakub Narebski, Mar 7, 2007
  21. Junio C HamanoMar 7, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.