[Catalyst-commits] r12746 - in Catalyst-Runtime/5.80/trunk: . lib t/aggregate

[t0m at dev.catalyst.perl.org]

Author: t0m
Date: 2010-01-27 21:04:17 +0000 (Wed, 27 Jan 2010)
New Revision: 12746

Modified:
   Catalyst-Runtime/5.80/trunk/Changes
   Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm
   Catalyst-Runtime/5.80/trunk/t/aggregate/live_component_controller_action_chained.t
   Catalyst-Runtime/5.80/trunk/t/aggregate/unit_core_uri_for.t
Log:
Fix / => %2F in uri_for so that it only works for action objects as previously advertised as this has been screwing people. Also fix multiple / => %2F conversion in the same arg/capture

Modified: Catalyst-Runtime/5.80/trunk/Changes
===================================================================
--- Catalyst-Runtime/5.80/trunk/Changes	2010-01-27 21:01:40 UTC (rev 12745)
+++ Catalyst-Runtime/5.80/trunk/Changes	2010-01-27 21:04:17 UTC (rev 12746)
@@ -1,5 +1,17 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+  Bug fixed:
+   - Calls to $c->uri_for with private paths as strings (e.g.
+     $c->uri_for('controller/action', 'arg1', 'arg2') ) no longer have
+     / encoded to %2F. This is due to $c->uri_for('static', 'css/foo', $bar)
+     which should not be encoded.
+     Calls with an action object (rather than a string), or uri_for action
+     will still encode / in args and captures to %2F
+
+   - The above noted / => %2F encoding in uri_for_action or uri_for with
+     an action object has been fixed to not just encode the first slash in
+     any set of args/captures.
+
   New features:
    - Allow passing additional arguments to action constructors.
 

Modified: Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm
===================================================================
--- Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm	2010-01-27 21:01:40 UTC (rev 12745)
+++ Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm	2010-01-27 21:04:17 UTC (rev 12746)
@@ -1259,8 +1259,19 @@
         $path .= '/';
     }
 
+    undef($path) if (defined $path && $path eq '');
+
+    my $params =
+      ( scalar @args && ref $args[$#args] eq 'HASH' ? pop @args : {} );
+
+    carp "uri_for called with undef argument" if grep { ! defined $_ } @args;
+    s/([^$URI::uric])/$URI::Escape::escapes{$1}/go for @args;
+    if (blessed $path) { # Action object only.
+        s|/|%2F|g for @args;
+    }
+
     if ( blessed($path) ) { # action object
-        my $captures = [ map { s|/|%2F|; $_; }
+        my $captures = [ map { s|/|%2F|g; $_; }
                         ( scalar @args && ref $args[0] eq 'ARRAY'
                          ? @{ shift(@args) }
                          : ()) ];
@@ -1274,15 +1285,6 @@
         $path = '/' if $path eq '';
     }
 
-    undef($path) if (defined $path && $path eq '');
-
-    my $params =
-      ( scalar @args && ref $args[$#args] eq 'HASH' ? pop @args : {} );
-
-    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);
 
     unless (defined $path && $path =~ s!^/!!) { # in-place strip

Modified: Catalyst-Runtime/5.80/trunk/t/aggregate/live_component_controller_action_chained.t
===================================================================
--- Catalyst-Runtime/5.80/trunk/t/aggregate/live_component_controller_action_chained.t	2010-01-27 21:01:40 UTC (rev 12745)
+++ Catalyst-Runtime/5.80/trunk/t/aggregate/live_component_controller_action_chained.t	2010-01-27 21:04:17 UTC (rev 12746)
@@ -1085,7 +1085,8 @@
             'request ' . $path . ' ok');
         # Just check that the path matches, as who the hell knows or cares
         # where the app is based (live tests etc)
-        ok( index($content, $path) > 1, 'uri can round trip through uri_for' );
+        ok( index($content, $path) > 1, 'uri can round trip through uri_for' )
+            or diag("Expected $path, got $content");
     }
 }
 

Modified: Catalyst-Runtime/5.80/trunk/t/aggregate/unit_core_uri_for.t
===================================================================
--- Catalyst-Runtime/5.80/trunk/t/aggregate/unit_core_uri_for.t	2010-01-27 21:01:40 UTC (rev 12745)
+++ Catalyst-Runtime/5.80/trunk/t/aggregate/unit_core_uri_for.t	2010-01-27 21:04:17 UTC (rev 12746)
@@ -1,16 +1,17 @@
 use strict;
 use warnings;
-
+use FindBin qw/$Bin/;
+use lib "$FindBin::Bin/../lib";
 use Test::More;
 use URI;
 
-use_ok('Catalyst');
+use_ok('TestApp');
 
 my $request = Catalyst::Request->new( {
                 base => URI->new('http://127.0.0.1/foo')
               } );
-
-my $context = Catalyst->new( {
+my $dispatcher = TestApp->dispatcher;
+my $context = TestApp->new( {
                 request => $request,
                 namespace => 'yada',
               } );
@@ -144,12 +145,20 @@
               "uri_for() doesn't mess up query parameter hash in the caller");
 }
 
-# 5.80018 is only encoding the first of the / in the arg. See line 1271.
-is(
-    Catalyst::uri_for( $context, 'controller/action', 'foo/bar/baz' )->as_string,
-    'http://127.0.0.1/controller/action/foo%2Fbar%2Fbaz',
-    'Escape both forward slashes in the arg as %2F'
-);
 
+{
+    my $path_action = $dispatcher->get_action_by_path(
+                       '/action/path/six'
+                     );
+
+    # 5.80018 is only encoding the first of the / in the arg.
+    is(
+        Catalyst::uri_for( $context, $path_action, 'foo/bar/baz' )->as_string,
+        'http://127.0.0.1/action/path/six/foo%2Fbar%2Fbaz',
+        'Escape all forward slashes in args as %2F'
+    );
+}
+
+
 done_testing;