Re: [PATCHv2 GSOC 01/11] gitweb: fix esc_url
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:49:08
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 <redacted>
First, as this patch is independent and unrelated to either splitting gitweb, or write support, it would be better if this patch was sent individually to git mailing list, and not only as a part of a large patch series. It would likely to be applied, as it is pure bugfix. 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. For what it is worth, ACK from me for this patch.
quoted hunk ↗ jump to hunk
--- gitweb/gitweb.perl | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 9446376..518328f 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl@@ -1322,8 +1322,7 @@ sub esc_param { sub esc_url { my $str = shift; return undef unless defined $str; - $str =~ s/([^A-Za-z0-9\-_.~();\/;?:@&=])/sprintf("%%%02X", ord($1))/eg; - $str =~ s/\+/%2B/g; + $str =~ s/([^A-Za-z0-9\-_.~();\/;?:@&= ]+)/CGI::escape($1)/eg; $str =~ s/ /\+/g; return $str; }-- 1.7.1.455.g8f441
-- Jakub Narebski Poland