On Thu, 16 Oct 2008, Giuseppe Bilotta wrote:
> If use_pathinfo is enabled, href now creates links that contain paths in
> the form $project/$action/oldhash:/oldname..newhash:/newname for actions
> that use hash_parent etc.
>
> If any of the filename contains two consecutive dots, it's kept as a CGI
> parameter since the resulting path would otherwise be ambiguous.
Note that this part is _required_ after previous patch, because
otherwise gitweb could generate path_info links which point to
different view than intended, when filename contains '..' in it,
like for example some test vectors in git test suite, for example
files in t/t5515
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx>
For what is worth:
Acked-by: Jakub Narebski <jnareb@xxxxxxxxx>
BTW. I wanted to test this using gitweb-Mechanize test from Lea Wiemann
GSoC 2008 work on gitweb caching, but I couldn't make it run as single
individual test (cd t && ./t9503-gitweb-Mechanize.sh); it requires to
run as "make test" and I didn't want that. I could have resurrected my
initial version of Test::WWW::Mechanize::CGI test...
> ---
> gitweb/gitweb.perl | 30 +++++++++++++++++++++++++-----
> 1 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1a7b0b9..f4642e7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -759,7 +759,8 @@ sub href (%) {
> # try to put as many parameters as possible in PATH_INFO:
> # - project name
> # - action
> - # - hash or hash_base:/filename
> + # - hash_parent or hash_parent_base:/file_parent
> + # - hash or hash_base:/file_name
Minor nit: this contain independent change 'filename' -> 'file_name',
but I think it is not worth separating...
>
> # When the script is the root DirectoryIndex for the domain,
> # $href here would be something like http://gitweb.example.com/
> @@ -778,17 +779,36 @@ sub href (%) {
> delete $params{'action'};
> }
>
> - # Finally, we put either hash_base:/file_name or hash
> + # Next, we put hash_parent_base:/file_parent..hash_base:/file_name,
> + # stripping nonexistent or useless pieces
> + $href .= "/" if ($params{'hash_base'} || $params{'hash_parent_base'}
> + || $params{'hash_parent'} || $params{'hash'});
Nice trick (and required change).
> if (defined $params{'hash_base'}) {
> - $href .= "/".esc_url($params{'hash_base'});
> - if (defined $params{'file_name'}) {
> + if (defined $params{'hash_parent_base'}) {
> + $href .= esc_url($params{'hash_parent_base'});
> + # skip the file_parent if it's the same as the file_name
> + delete $params{'file_parent'} if $params{'file_parent'} eq $params{'file_name'};
> + if (defined $params{'file_parent'} && $params{'file_parent'} !~ /\.\./) {
> + $href .= ":/".esc_url($params{'file_parent'});
> + delete $params{'file_parent'};
> + }
Side note: I wonder if we should use esc_url or esc_param here...
> + $href .= "..";
> + delete $params{'hash_parent'};
> + delete $params{'hash_parent_base'};
> + } elsif (defined $params{'hash_parent'}) {
> + $href .= esc_url($params{'hash_parent'}). "..";
> + delete $params{'hash_parent'};
> + }
> +
> + $href .= esc_url($params{'hash_base'});
> + if (defined $params{'file_name'} && $params{'file_name'} !~ /\.\./) {
> $href .= ":/".esc_url($params{'file_name'});
> delete $params{'file_name'};
> }
> delete $params{'hash'};
> delete $params{'hash_base'};
> } elsif (defined $params{'hash'}) {
> - $href .= "/".esc_url($params{'hash'});
> + $href .= esc_url($params{'hash'});
> delete $params{'hash'};
> }
> }
> --
> 1.5.6.5
>
>
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
