From: Tomas Doran <bobtfish@bobtfish.net>
Date: Thu, 12 Nov 2009 00:22:06 +0000 (+0000)
Subject: Tests, fix args as well as captureargs and be less violent - only encode /
X-Git-Tag: 5.80014~18
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=36c67dc10cd5a928718d4e9122e03297a959a47e;hp=3c9a702f521c1635602320cf62f056d362a45c2d

Tests, fix args as well as captureargs and be less violent - only encode /
---

diff --git a/Makefile.PL b/Makefile.PL
index c87002e..aba193a 100644
--- 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')) {
diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index 9c342e8..8d5113e 100644
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -1247,7 +1247,7 @@ sub uri_for {
     }
 
     if ( blessed($path) ) { # action object
-        my $captures = [ map { URI::Escape::uri_escape($_) }
+        my $captures = [ map { s|/|%2F|; $_; }
                         ( scalar @args && ref $args[0] eq 'ARRAY'
                          ? @{ shift(@args) }
                          : ()) ];
@@ -1268,6 +1268,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..099c833 100644
--- 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,27 @@ 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 ;)
+    # FIXME - what about people who have code to hack around this and manually uri encode args and captures
+    #         themselves, erk! 
+    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
--- 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) = @_;