Back out 10498 and 10097
Tomas Doran [Wed, 17 Jun 2009 16:03:51 +0000 (16:03 +0000)]
Changes
lib/Catalyst.pm
lib/Catalyst/DispatchType/Chained.pm
lib/Catalyst/DispatchType/Regex.pm
t/aggregate/unit_core_uri_for_action.t
t/unit_core_uri_for.t

diff --git a/Changes b/Changes
index 71b8bca..ed00869 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,7 +1,9 @@
 # This file documents the revision history for Perl extension Catalyst.
 
   Bug fixes:
-        - Stop encoding plus signs in uri_for args
+        -  Revert change to URL encode things passed into $c->uri_for
+           Args and CaptureArgs as this causes breakage to pre-existing
+           applications.
 
 5.80005 2009-06-06 14:40:00
 
index 549af88..f9ab9c4 100644 (file)
@@ -1199,7 +1199,7 @@ sub uri_for {
       ( scalar @args && ref $args[$#args] eq 'HASH' ? pop @args : {} );
 
     carp "uri_for called with undef argument" if grep { ! defined $_ } @args;
-    s/([^A-Za-z0-9\-_.!~*'()+])/$URI::Escape::escapes{$1}/go for @args;
+    s/([^$URI::uric])/$URI::Escape::escapes{$1}/go for @args;
 
     unshift(@args, $path);
 
@@ -1233,7 +1233,7 @@ sub uri_for {
               $_ = "$_";
               utf8::encode( $_ ) if utf8::is_utf8($_);
               # using the URI::Escape pattern here so utf8 chars survive
-              s/([^A-Za-z0-9\-_.!~*'()])/$URI::Escape::escapes{$1}/go;
+              s/([^A-Za-z0-9\-_.!~*'() ])/$URI::Escape::escapes{$1}/go;
               s/ /+/g;
               "${key}=$_"; } ( ref $val eq 'ARRAY' ? @$val : $val ));
       } @keys);
index 94a8a4c..890961d 100644 (file)
@@ -353,9 +353,7 @@ sub uri_for_action {
         if (my $cap = $curr->attributes->{CaptureArgs}) {
             return undef unless @captures >= $cap->[0]; # not enough captures
             if ($cap->[0]) {
-                unshift(@parts,
-                    map { s/([^A-Za-z0-9\-_.!~*'()])/$URI::Escape::escapes{$1}/go; $_; }
-                    splice(@captures, -$cap->[0]));
+                unshift(@parts, splice(@captures, -$cap->[0]));
             }
         }
         if (my $pp = $curr->attributes->{PartPath}) {
index fb1dc73..11a9cbf 100644 (file)
@@ -148,7 +148,7 @@ sub uri_for_action {
             $re =~ s/^\^//;
             $re =~ s/\$$//;
             my $final = '/';
-            my @captures =  map { s/([^A-Za-z0-9\-_.!~*'()])/$URI::Escape::escapes{$1}/go; $_; } @$captures;
+            my @captures = @$captures;
             while (my ($front, $rest) = split(/\(/, $re, 2)) {
                 last unless defined $rest;
                 ($rest, $re) =
index 53d71df..dfe3666 100644 (file)
@@ -8,7 +8,7 @@ use lib "$FindBin::Bin/../lib";
 
 use Test::More;
 
-plan tests => 33;
+plan tests => 30;
 
 use_ok('TestApp');
 
@@ -54,10 +54,6 @@ is($dispatcher->uri_for_action($regex_action, [ 'foo', 123 ]),
    "/action/regexp/foo/123",
    "Regex action interpolates captures correctly");
 
-is($dispatcher->uri_for_action($regex_action, [ 'foo/bar', 123 ]),
-   "/action/regexp/foo%2Fbar/123",
-   "Regex action interpolates captures correctly and url encodes /");
-
 #
 #   Index Action
 #
@@ -109,10 +105,6 @@ is($context->uri_for($path_action, qw/one two/, { q => 1 }),
    "http://127.0.0.1/foo/action/relative/relative/one/two?q=1",
    "uri_for correct for path action with args and query");
 
-is($context->uri_for($path_action, qw|one/quux two|),
-   "http://127.0.0.1/foo/action/relative/relative/one%2Fquux/two",
-   "uri_for correctly url encoded for path action with args containing /");
-
 ok(!defined($context->uri_for($path_action, [ 'blah' ])),
    "no URI returned by uri_for for Path action with snippets");
 
@@ -169,12 +161,8 @@ is($context->uri_for($chained_action, [ 1 ], 2, { q => 1 }),
         'uri_for_action returns uri with empty arg on undef last argument' );
 
     is( $context->uri_for_action($action_needs_two, [ 'foo' , 'bar/baz' ], (3,4)),
-        'http://127.0.0.1/foo/chained/foo2/foo/bar%2Fbaz/end2/3/4',
-        'uri_for_action works correctly when CaptureArg contains /' );
-
-    is( $context->uri_for_action($action_needs_two, [ 'foo' , 'bar' ], ('3/baz',4)),
-        'http://127.0.0.1/foo/chained/foo2/foo/bar/end2/3%2Fbaz/4',
-        'uri_for_action works correctly when Args contains /' );
+        'http://127.0.0.1/foo/chained/foo2/foo/bar%2Fbaz/end2/3/',
+        'uri_for_action returns uri with empty arg on undef last argument' );
 
     my $complex_chained = '/action/chained/empty_chain_f';
     is( $context->uri_for_action( $complex_chained, [23], (13), {q => 3} ),
index f54123e..fb5ea1e 100644 (file)
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 17;
+use Test::More tests => 16;
 use URI;
 
 use_ok('Catalyst');
@@ -51,11 +51,6 @@ is( Catalyst::uri_for( $context, qw/bar wibble?/, 'with space' )->as_string,
     'http://127.0.0.1/foo/yada/bar/wibble%3F/with%20space', 'Space gets encoded'
 );
 
-is(
-    Catalyst::uri_for( $context, '/bar', 'with+plus', { 'also' => 'with+plus' })->as_string,
-    'http://127.0.0.1/foo/bar/with+plus?also=with%2Bplus',
-    'Plus is not encoded'
-);
 
 # test with utf-8
 is(