From: Jakub Narebski Date: Thu, 15 Jul 2010 19:32:38 GMT Subject: Re: [PATCHv2 GSOC 01/11] gitweb: fix esc_url 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ł: > Jakub Narebski 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 [...] >> 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