Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix index matching with trailing slashes #16

Open
wants to merge 7 commits into
from
View
@@ -475,46 +475,53 @@ method _build_match_request_path ($interp:) {
my ( $request, $request_path ) = @_;
my $interp = $request->interp;
my $path_info = '';
+ my $trailing_slash = '';
my $declined_paths = $request->declined_paths;
my @index_subpaths = map { "/$_" } @index_names;
my $path = $request_path;
my @tried_paths;
- # Deal with trailing slash
- #
- $path_info = chop($path) if $path ne '/' && substr( $path, -1 ) eq '/';
+ $trailing_slash = chop($path) if $path ne '/' && substr($path, -1) eq '/';
while (1) {
- my @candidate_paths =
- ( $path_info eq '' && !@autoextensions ) ? ($path)
- : ( $path eq '/' ) ? ( @index_subpaths, @dhandler_subpaths )
- : (
- ( grep { !/$ignore_file_regex/ } map { $path . $_ } @autoextensions ),
- ( map { $path . $_ } ( @index_subpaths, @dhandler_subpaths ) )
- );
+ my @candidate_paths;
+ if ($path_info eq '' && !@autoextensions) {
+ @candidate_paths = ($path);
+ }
+ elsif ($path eq '/') {
+ @candidate_paths = (@index_subpaths, @dhandler_subpaths);
+ }
+ else {
+ my @idx_dhandler = map { $path . $_ } (@index_subpaths, @dhandler_subpaths);
+ my @filtered_autoext = grep { !/$ignore_file_regex/ } map { $path . $_ } @autoextensions;
+ if ($trailing_slash eq '/') {
+ @candidate_paths = (@idx_dhandler, @filtered_autoext);
+ }
+ else {
+ @candidate_paths = (@filtered_autoext, @idx_dhandler);
+ }
+ }
push( @tried_paths, @candidate_paths );
foreach my $candidate_path (@candidate_paths) {
next if $declined_paths->{$candidate_path};
if ( my $compc = $interp->load($candidate_path) ) {
+ my $path_info_ok = $compc->cmeta->is_dhandler || $compc->allow_path_info;
if (
$compc->cmeta->is_top_level
&& ( $path_info eq ''
- || $compc->cmeta->is_dhandler
- || $compc->allow_path_info )
+ || $path_info_ok )
)
{
+ $path_info .= $trailing_slash if $path_info_ok;
$request->{path_info} = $path_info;
return $compc->cmeta->path;
}
}
}
$interp->_top_level_not_found( $request_path, \@tried_paths ) if $path eq '/';
my $name = basename($path);
- $path_info =
- $path_info eq '/' ? "$name/"
- : length($path_info) ? "$name/$path_info"
- : $name;
- $path = dirname($path);
+ $path_info = length($path_info) ? "$name/$path_info" : $name;
+ $path = dirname($path);
@index_subpaths = (); # only match index file in same directory
}
};
@@ -73,29 +73,44 @@ dhandlers is to match partial paths.
=head2 Trailing slash
-If the request URL has a trailing slash (ends with C</>), we remove it before
-the match process begins and add it to the C<< $m->path_info >>. Components
-that should match must have C<allow_path_info> return true.
+If the request URL has a trailing slash (ends with C</>), we remove it
+before the match process begins. If your component has
+C<allow_path_info> return true or if it is a dhandler, the slash will be
+part of the C<path_info> information.
For example:
## request URL /news/
- /news/index.{mp,mc} # $m->path_info = / if index.{mp,mc} has
- # allow_path_info => true
- /news/dhandler.{mp,mc} # $m->path_info = /
- /news.{mp,mc} # $m->path_info = / if news.{mp,mc} has
+ /news/index.{mp,mc} # $m->path_info = '' if allow_path_info => false
+ /news/index.{mp,mc} # $m->path_info = '/' if allow_path_info => true
+
+ /news/dhandler.{mp,mc} # $m->path_info = '/'
+
+ /news.{mp,mc} # $m->path_info = '' if news.{mp,mc} has
+ # allow_path_info => false
+ /news.{mp,mc} # $m->path_info = '/' if news.{mp,mc} has
# allow_path_info => true
## request URL /news/sports/
- /news/sports/index.{mp,mc} # $m->path_info = / if index.{mp,mc} has
+ /news/sports/index.{mp,mc} # $m->path_info = '' if index.{mp,mc} has
+ # allow_path_info => false
+ /news/sports/index.{mp,mc} # $m->path_info = '/' if index.{mp,mc} has
# allow_path_info => true
- /news/sports/dhandler.{mp,mc} # $m->path_info = /
- /news/sports.{mp,mc} # $m->path_info = / if sports.{mp,mc}
+
+ /news/sports/dhandler.{mp,mc} # $m->path_info = '/'
+
+ /news/sports.{mp,mc} # $m->path_info = '' if sports.{mp,mc}
+ # has allow_path_info => false
+ /news/sports.{mp,mc} # $m->path_info = '/' if sports.{mp,mc}
# has allow_path_info => true
- /news/dhandler.{mp,mc} # $m->path_info = sports/
- /news.{mp,mc} # $m->path_info = /sports/ if news.{mp,mc}
+
+ /news/dhandler.{mp,mc} # $m->path_info = 'sports/'
+
+ /news.{mp,mc} # will not match if allow_path_info => false
+ /news.{mp,mc} # $m->path_info = '/sports/' if news.{mp,mc}
# has allow_path_info => true
+
=head2 Routes
It is possible to use route syntax to more elegantly parse C<< $m->path_info >>
View
@@ -9,8 +9,11 @@ sub test_resolve : Tests {
my ( $run_path, $existing_paths, $resolve_path, $path_info ) = @_;
$path_info ||= '';
+ my ($line) = (caller)[2];
+
$self->setup_dirs(@interp_params);
- foreach my $existing_path (@$existing_paths) {
+ foreach (@$existing_paths) {
+ my $existing_path = $_;
my $allow_path_info = 0;
if ( $existing_path =~ /=1$/ ) {
substr( $existing_path, -2, 2 ) = '';
@@ -26,12 +29,12 @@ sub test_resolve : Tests {
my $desc = sprintf( "run %s against %s", $run_path, join( ",", @$existing_paths ) );
if ( defined($resolve_path) ) {
my $good = "path: $resolve_path; path_info: $path_info";
- is( $self->interp->run($run_path)->output, $good, "$desc = matched $good" );
+ is( $self->interp->run($run_path)->output, $good, "$desc = matched $good ($line)" );
}
else {
throws_ok { $self->interp->run($run_path)->output }
qr/could not resolve request path/,
- "$desc = failed to match";
+ "$desc = failed to match ($line)";
}
};
@@ -86,23 +89,53 @@ sub test_resolve : Tests {
$try->( $run_path, ['/foo/bar/baz/index'], '/foo/bar/baz/index', '' );
$try->( $run_path, ['/foo/bar/baz/index2'], '/foo/bar/baz/index2', '' );
$try->( $run_path, [ '/foo/bar/baz/index2', '/foo/bar/baz/index' ], '/foo/bar/baz/index', '' );
+ $try->( '/foo', [ '/foo/index' ], '/foo/index', '' );
+ $try->( '/foo/', [ '/foo/index' ], '/foo/index', '' );
- # trailing slashes
+ # trailing slashes + path_info
$try->( '/foo', ['/foo.mc=1'], '/foo.mc', '' );
$try->( '/foo/', ['/foo.mc=1'], '/foo.mc', '/' );
$try->( '/foo/bar', ['/foo.mc=1'], '/foo.mc', 'bar' );
$try->( '/foo/bar/', ['/foo.mc=1'], '/foo.mc', 'bar/' );
- $try->( '/foo/', ['/foo.mc'], undef );
+ $try->( '/foo/', ['/foo.mc'], '/foo.mc', '' );
@interp_params = ( dhandler_names => ['dhandler'] );
$try->( '/foo/', ['/foo/dhandler'], '/foo/dhandler', '/' );
$try->( '/foo/bar', ['/foo/dhandler'], '/foo/dhandler', 'bar' );
$try->( '/foo/bar/', ['/foo/dhandler'], '/foo/dhandler', 'bar/' );
@interp_params = ( index_names => ['index'] );
- $try->( '/foo/', ['/foo/index'], undef );
$try->( '/foo/', ['/foo/index=1'], '/foo/index', '/' );
@interp_params = ( dhandler_names => ['dhandler'], index_names => ['index'] );
- $try->( '/foo/', [ '/foo/dhandler', '/foo/index' ], '/foo/dhandler', '/' );
- $try->( '/foo/', [ '/foo/dhandler', '/foo/index=1' ], '/foo/index', '/' );
+ $try->( '/foo/', ['/foo/dhandler', '/foo/index'], '/foo/index', '' );
+ $try->( '/foo/', ['/foo/dhandler', '/foo/index=1'], '/foo/index', '/' );
+ $try->( '/foo/more', ['/foo/dhandler', '/foo/index'], '/foo/dhandler', 'more' );
+ $try->( '/foo/more', ['/foo/dhandler', '/foo/index=1'], '/foo/dhandler', 'more' );
+
+ # trailing slashes: cases from Mason::Manual::RequestDispatch
+ @interp_params = ( dhandler_names => ['dhandler'], index_names => ['index'] );
+ $try->( '/news/', ['/news/index'], '/news/index', '');
+ $try->( '/news/', ['/news/index=1'], '/news/index', '/');
+ $try->( '/news/', ['/news/dhandler'], '/news/dhandler', '/');
+ $try->( '/news/', ['/news.mc'], '/news.mc', '');
+ $try->( '/news/', ['/news.mc=1'], '/news.mc', '/');
+ $try->( '/news/sports/', ['/news/sports/index'], '/news/sports/index', '');
+ $try->( '/news/sports/', ['/news/sports/index=1'], '/news/sports/index', '/');
+ $try->( '/news/sports/', ['/news/sports/dhandler'], '/news/sports/dhandler', '/');
+ $try->( '/news/sports/', ['/news/sports.mc'], '/news/sports.mc', '');
+ $try->( '/news/sports/', ['/news/sports.mc=1'], '/news/sports.mc', '/');
+ $try->( '/news/sports/', ['/news/dhandler'], '/news/dhandler', 'sports/');
+ $try->( '/news/sports/', ['/news.mc'], undef);
+ $try->( '/news/sports/', ['/news.mc=1'], '/news.mc', 'sports/');
+ $try->( '/news/sports', ['/news/sports/index','/news/sports.mc'], '/news/sports.mc', '');
+ $try->( '/news/sports/', ['/news/sports/index','/news/sports.mc'], '/news/sports/index', '');
+ $try->( '/news/sports/', ['/news/sports/index=1','/news/sports.mc'], '/news/sports/index', '/');
+ $try->( '/news/sports', ['/news/sports/dhandler','/news/sports.mc'], '/news/sports.mc', '');
+ $try->( '/news/sports/', ['/news/sports/dhandler','/news/sports.mc'], '/news/sports/dhandler', '/');
+ $try->( '/news/sports', ['/news/dhandler','/news/sports.mc'], '/news/sports.mc', '');
+ $try->( '/news/sports/', ['/news/dhandler','/news/sports.mc'], '/news/sports.mc', '');
+ $try->( '/news/sports/', ['/news/dhandler','/news/sports.mc=1'], '/news/sports.mc', '/');
+ $try->( '/news/sports', ['/dhandler','/news.mc'], '/dhandler', 'news/sports');
+ $try->( '/news/sports/', ['/dhandler','/news.mc'], '/dhandler', 'news/sports/');
+ $try->( '/news/sports/', ['/dhandler','/news.mc=1'], '/news.mc', 'sports/');
}
sub test_decline : Tests {