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

Re: [PATCHv2 GSOC 01/11] gitweb: fix esc_url

From
Jakub Narebski <jnareb@gmail.com>
Date
Jul 15, 2010, 19:32 UTC
Message-ID
<201007152132.39441.jnareb@gmail.com>
In-Reply-To
<7vfwzkvahf.fsf@alter.siamese.dyndns.org>
Dnia czwartek 15. lipca 2010 20:57, Junio C Hamano napisał:
Show 16 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> On Thu, 15 Jul 2010, Pavan Kumar Sunkara wrote:
>>> The custom CGI escaping done in esc_url failed to escape UTF-8
>>> properly. Fix by using CGI::escape on each sequence of matched
>>> characters instead of sprintf()ing a custom escaping for each byte.
>>> 
>>> Additionally, the space -> + escape was being escaped due to greedy
>>> matching on the first substitution. Fix by adding space to the
>>> list of characters not handled on the first substitution.
>>> 
>>> Finally, remove an unnecessary escaping of the + sign.
>>> 
>>> commit 452e225 has missed fixing esc_url.
>>> 
>>> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
[...]
Show 10 quoted lines
>> Second, I would probably write commit message differently, to emphasize
>> that it is just finishing work of commit 452e225 (gitweb: fix esc_param,
>> 2009-10-13) by fixing esc_url like it fixed esc_params.  But it is not
>> something very important.
> 
> I tentatively rewrote the message like so:
> 
>     Earlier, 452e225 (gitweb: fix esc_param, 2009-10-13) fixed CGI
>     escaping rules used in esc_url.  A very similar logic exists in
>     esc_param and needs to be fixed the same way.
Thanks.
> It makes one wonder why they have to be separate functions, doesn't it,
> though?

They need to be separate because you have to escape params-related special characters ('?', ';', '=') when quoting params, but you shouldn't when escaping (external) URL as a whole.

-- 
Jakub Narebski
Poland
Previous: Junio C HamanoNext: Pavan Kumar Sunkara
Message 5 of 27 in “[PATCHv2 00/11] Splitting gitweb”
  1. Pavan Kumar SunkaraJul 15, 2010
  2. 01/11 gitweb: fix esc_urlPavan Kumar Sunkara, Jul 15, 2010
  3. Jakub NarebskiJul 15, 2010
  4. Junio C HamanoJul 15, 2010
  5. Jakub NarebskiJul 15, 2010
  6. 02/11 gitweb: Prepare for splitting gitwebPavan Kumar Sunkara, Jul 15, 2010
  7. Jakub NarebskiJul 15, 2010
  8. 03/11 gitweb: Create Gitweb::Git modulePavan Kumar Sunkara, Jul 15, 2010
  9. Jakub NarebskiJul 15, 2010
  10. 04/11 gitweb: Create Gitweb::Config modulePavan Kumar Sunkara, Jul 15, 2010
  11. Jakub NarebskiJul 15, 2010
  12. 05/11 gitweb: Create Gitweb::Request modulePavan Kumar Sunkara, Jul 15, 2010
  13. Jakub NarebskiJul 16, 2010
  14. 06/11 gitweb: Create Gitweb::Escape modulePavan Kumar Sunkara, Jul 15, 2010
  15. Jakub NarebskiJul 16, 2010
  16. 07/11 gitweb: Create Gitweb::RepoConfig modulePavan Kumar Sunkara, Jul 15, 2010
  17. Jakub NarebskiJul 16, 2010
  18. 08/11 gitweb: Create Gitweb::View modulePavan Kumar Sunkara, Jul 15, 2010
  19. Jakub NarebskiJul 18, 2010
  20. 09/11 gitweb: Create Gitweb::Util modulePavan Kumar Sunkara, Jul 15, 2010
  21. Jakub NarebskiJul 18, 2010
  22. 10/11 gitweb: Create Gitweb::Format modulePavan Kumar Sunkara, Jul 15, 2010
  23. Jakub NarebskiJul 18, 2010
  24. 11/11 gitweb: Create Gitweb::Parse modulePavan Kumar Sunkara, Jul 15, 2010
  25. Jakub NarebskiJul 19, 2010
  26. Sverre RabbelierAug 1, 2010
  27. Jakub NarebskiAug 2, 2010

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.