Merge 'trunk' into 'uri_encode_captures_andor_args_take2'

diff --git a/Makefile.PL b/Makefile.PL

index be08062..dc3cea6 100644 (file)
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -52,6 +52,7 @@ recommends 'B::Hooks::OP::Check::StashChange';
 
 test_requires 'Class::Data::Inheritable';
 test_requires 'Test::Exception';
+test_requires 'Test::More' => '0.88';
 
 # aggregate tests if AGGREGATE_TESTS is set and a recent Test::Aggregate and a Test::Simple it works with is available
 if ($ENV{AGGREGATE_TESTS} && can_use('Test::Simple', '0.88') && can_use('Test::Aggregate', '0.35_05')) {
@@ -64,6 +65,7 @@ else {
         map  { glob } qw[t/*.t t/aggregate/*.t];
 }
 author_requires 'CatalystX::LeakChecker', '0.03'; # Skipped if this isn't installed
+author_requires 'File::Copy::Recursive'; # For http server test
 
 author_tests 't/author';
 author_requires(map {; $_ => 0 } qw(
diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm

index d681c90..6e74e58 100644 (file)
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -1259,9 +1259,10 @@ sub uri_for {
     }
 
     if ( blessed($path) ) { # action object
-        my $captures = ( scalar @args && ref $args[0] eq 'ARRAY'
-                         ? shift(@args)
-                         : [] );
+        my $captures = [ map { s|/|%2F|; $_; }
+                        ( scalar @args && ref $args[0] eq 'ARRAY'
+                         ? @{ shift(@args) }
+                         : ()) ];
         my $action = $path;
         $path = $c->dispatcher->uri_for_action($action, $captures);
         if (not defined $path) {
@@ -1279,6 +1280,7 @@ sub uri_for {
 
     carp "uri_for called with undef argument" if grep { ! defined $_ } @args;
     s/([^$URI::uric])/$URI::Escape::escapes{$1}/go for @args;
+    s|/|%2F| for @args;
 
     unshift(@args, $path);
 
diff --git a/t/aggregate/live_component_controller_action_chained.t b/t/aggregate/live_component_controller_action_chained.t

index 3784fe6..157f016 100644 (file)
--- a/t/aggregate/live_component_controller_action_chained.t
+++ b/t/aggregate/live_component_controller_action_chained.t
@@ -10,7 +10,7 @@ our $iters;
 
 BEGIN { $iters = $ENV{CAT_BENCH_ITERS} || 1; }
 
-use Test::More tests => 148*$iters;
+use Test::More;
 use Catalyst::Test 'TestApp';
 
 if ( $ENV{CAT_BENCHMARK} ) {
@@ -1018,5 +1018,29 @@ sub run_tests {
             'request with URI-encoded arg' );
         like( $content, qr{foo/bar;\z}, 'args decoded' );
     }
+
+    # Test round tripping, specifically the / character %2F in uri_for:
+    # not being able to feed it back action + captureargs and args into uri for
+    # and result in the original request uri is a major piece of suck ;)
+    foreach my $thing (
+        ['foo', 'bar'],
+        ['foo%2Fbar', 'baz'],
+        ['foo', 'bar%2Fbaz'],
+        ['foo%2Fbar', 'baz%2Fquux'],
+        ['foo%2Fbar', 'baz%2Fquux', { foo => 'bar', 'baz' => 'quux%2Ffrood'}],
+        ['foo%2Fbar', 'baz%2Fquux', { foo => 'bar', 'baz%2Ffnoo' => 'quux%2Ffrood'}],
+    ) {
+        my $uri = 'http://localhost/chained/roundtrip_urifor/' .
+            $thing->[0] . '/' . $thing->[1];
+        $uri .= '?' . join('&',
+            map { $_ .'='. $thing->[2]->{$_}}
+            sort keys %{$thing->[2]}) if $thing->[2];
+        ok( my $content =
+            get($uri),
+            'request ' . $uri . ' ok');
+        is( $content, $uri, 'uri can round trip through uri_for' );
+    }
 }
 
+done_testing;
+
diff --git a/t/lib/TestApp/Controller/Action/Chained.pm b/t/lib/TestApp/Controller/Action/Chained.pm

index 64de556..cbba762 100644 (file)
--- a/t/lib/TestApp/Controller/Action/Chained.pm
+++ b/t/lib/TestApp/Controller/Action/Chained.pm
@@ -204,6 +204,13 @@ sub return_arg_decoded : Chained('/') PathPart('chained/return_arg_decoded') Arg
     $c->req->args([ map { decode_entities($_) } @{ $c->req->args }]);
 }
 
+sub roundtrip_urifor : Chained('/') PathPart('chained/roundtrip_urifor') CaptureArgs(1) {}
+sub roundtrip_urifor_end : Chained('roundtrip_urifor') PathPart('') Args(1) {
+    my ($self, $c) = @_;
+    # This should round-trip, always - i.e. the uri you put in should come back out.
+    $c->res->body($c->uri_for($c->action, $c->req->captures, @{$c->req->args}, $c->req->parameters));
+    $c->stash->{no_end} = 1;
+}
 
 sub end :Private {
   my ($self, $c) = @_;
Makefile.PL		patch \| blob \| blame \| history
lib/Catalyst.pm		patch \| blob \| blame \| history
t/aggregate/live_component_controller_action_chained.t		patch \| blob \| blame \| history
t/lib/TestApp/Controller/Action/Chained.pm		patch \| blob \| blame \| history