fixed bug in Mojolicious::Routes::Pattern where optional placeholders…

… in nested routes would sometimes not work correctly (closes #709)
mojolicious · Nov 17, 2014 · 36f64ab · 36f64ab
1 parent 060ebcc
commit 36f64ab
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 4 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,7 @@
 
-5.62  2014-11-15
+5.62  2014-11-17
+  - Fixed bug in Mojolicious::Routes::Pattern where optional placeholders in
+    nested routes would sometimes not work correctly.
 
 5.61  2014-11-14
   - Moved entities.txt into the DATA section of Mojo::Util to avoid

diff --git a/lib/Mojolicious/Routes/Pattern.pm b/lib/Mojolicious/Routes/Pattern.pm
@@ -195,17 +195,20 @@ sub _tokenize {
     # Quote end
     elsif ($char eq $quote_end) { ($inside, $quoted) = (0, 0) }
 
-    # Slash (first slash is text for optimizations)
+    # Slash
     elsif ($char eq '/') {
-      push @tree, @tree ? ['slash'] : ['text', '/'];
+      push @tree, ['slash'];
       $inside = 0;
     }
 
     # Placeholder, relaxed or wildcard
     elsif ($inside) { $tree[-1][-1] .= $char }
 
-    # Text (optimize text followed by slash followed by text)
+    # Text (optimize slash-text and *-text-slash-text)
     elsif ($tree[-1][0] eq 'text') { $tree[-1][-1] .= $char }
+    elsif (!$tree[-2] && $tree[-1][0] eq 'slash') {
+      @tree = (['text', "/$char"]);
+    }
     elsif ($tree[-2] && $tree[-2][0] eq 'text' && $tree[-1][0] eq 'slash') {
       pop @tree && ($tree[-1][-1] .= "/$char");
     }

diff --git a/t/mojolicious/routes.t b/t/mojolicious/routes.t
@@ -38,6 +38,12 @@ $r->route('/alternatives4/:foo', foo => [qw(foo foo.bar)]);
 # /optional/*/*
 $r->route('/optional/:foo/:bar')->to(bar => 'test');
 
+# /optional2
+# /optional2/*
+# /optional2/*/*
+$r->route('/optional2/:foo')->to(foo => 'one')->route('/:bar')
+  ->to(bar => 'two');
+
 # /*/test
 my $test = $r->route('/:controller/test')->to(action => 'test');
 
@@ -372,6 +378,20 @@ is $m->path_for('optionalfoobar')->{path}, '/optional/23/24', 'right path';
 is $m->path_for('optionalfoobar', foo => 0)->{path}, '/optional/0/24',
   'right path';
 
+# Optional placeholders in nested routes
+$m = Mojolicious::Routes::Match->new(root => $r);
+$m->match($c => {method => 'GET', path => '/optional2'});
+is_deeply $m->stack, [{foo => 'one', bar => 'two'}], 'right structure';
+is $m->path_for->{path}, '/optional2', 'right path';
+$m = Mojolicious::Routes::Match->new(root => $r);
+$m->match($c => {method => 'GET', path => '/optional2/three'});
+is_deeply $m->stack, [{foo => 'three', bar => 'two'}], 'right structure';
+is $m->path_for->{path}, '/optional2/three', 'right path';
+$m = Mojolicious::Routes::Match->new(root => $r);
+$m->match($c => {method => 'GET', path => '/optional2/three/four'});
+is_deeply $m->stack, [{foo => 'three', bar => 'four'}], 'right structure';
+is $m->path_for->{path}, '/optional2/three/four', 'right path';
+
 # Real world example using most features at once
 $m = Mojolicious::Routes::Match->new(root => $r);
 $m->match($c => {method => 'GET', path => '/articles/1/edit'});