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, 13:52 UTC
Message-ID
<201007151552.23324.jnareb@gmail.com>
In-Reply-To
<1279178951-23712-2-git-send-email-pavan.sss1991@gmail.com>
On Thu, 15 Jul 2010, Pavan Kumar Sunkara wrote:
Show 13 quoted lines
> 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>

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.
Show 22 quoted lines
> ---
>  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
Previous: Pavan Kumar SunkaraNext: Junio C Hamano
Message 3 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.