Re: [PATCH] gitweb: fix esc_param

From: Jakub Narebski <jnareb@gmail.com>
Date: Oct 14, 2009, 09:03 UTC
Message-ID: <200910141103.19295.jnareb@gmail.com>
In-Reply-To: <4AD56FD9.70602@gmail.com>

On Wed, 14 Oct 2009, Stephen Boyd wrote:

Show 14 quoted lines

> Giuseppe Bilotta wrote:
>> On Wed, Oct 14, 2009 at 3:13 AM, Stephen Boyd <bebarino@gmail.com> wrote:
>>
>>> This works great for my purposes. Thanks.
>>>
>> Can you also check if this fixes the branch name issue you mentioned
>> in the other branch? (And/or do you have a repository exposing the
>> problem if not?)
> 
> (We're jumping back and forth between threads haha)
> 
> Sorry, it doesn't. But this diff fixes the first part of the problem.
> There are still problems with the RSS/Atom feed names being mangled. The
> branch name I'm using is gitwéb, but I imagine any utf8 character will fail.

That it is (probably) not because of lack of esc_html, but because of lack of to_utf8. Grrr... we really should convert to utf8 (usually just mark as utf8) on reading input, to avoid such kind of errors.

Show 13 quoted lines

> 
> I see the title and the actual text being mangled without this patch.
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4b21ad2..910c370 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1500,9 +1500,10 @@ sub format_ref_marker {
>                                 -href => href(
>                                         action=>$dest_action,
>                                         hash=>$dest
> -                               )}, $name);
> +                               )}, esc_html($name));

Hmmm... to_utf8 would be enough here, but for the fact that git-check-ref-format doesn't prohibit '<', '>' in ref names, so it is possible that somebody somewhere is using such strange ref names (e.g. branch named '<b>' is valid branch name).

>  
> -                       $markers .= " <span class=\"$class\" title=\"$ref\">" .
> +                       my $title_ref = esc_html($ref);
> +                       $markers .= " <span class=\"$class\" title=\"$title_ref\">" .

Here it would be really useful to have esc_attr (which would do to_utf8 plus escaping of '"' and '%', quote and escape characters), although I guess that full HTML escaping done with esc_html wouldn't do anything bad.

>                                 $link . "</span>";
>                 }
>         }

-- 
Jakub Narebski
Poland