From: Tomas Doran <bobtfish@bobtfish.net>
Date: Mon, 12 Jan 2009 03:53:01 +0000 (+0000)
Subject: Fix Plugin::Authorization::ACL + tests for the weird behavior it needs
X-Git-Tag: 5.8000_05~27
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=12f0342e49c1b2af2e71c30aa4dcfbac0b7735c2

Fix Plugin::Authorization::ACL + tests for the weird behavior it needs
---

diff --git a/Changes b/Changes
index 594ac2d..266cb92 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,8 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+        - Fix request argumentss getting corrupted if you override the 
+          dispatcher and call an action which detaches (for 
+          Catalyst::Plugin::Authorization::ACL) (t0m)
         - Fix calling use Catalyst::Test 'MyApp' 'foo' which used to work,
           but stopped as the 2nd parameter can be an options hash now (t0m)
         - Bump Moose dependency to fix make_immutable bug (t0m)
diff --git a/TODO b/TODO
index a3d0cae..c251a73 100644
--- a/TODO
+++ b/TODO
@@ -13,10 +13,6 @@ Known issues:
      - CatalystX-CRUD and CatalystX-CRUD-ModelAdapter-DBIC
        fail tests against 5.80 (karpet)
 
-     - Catalyst-Plugin-Authorization-ACL fails as 
-       Catalyst::Dispatcher::_do_forward does not fix arguments if you throw
-       an exception. Needs a test case (Caelum)
-     
      - Waiting on new releases:
        - Catalyst::Plugin::Authentication - 0.100092
        - Catalyst::Action::RenderView     - 0.08
diff --git a/lib/Catalyst/Dispatcher.pm b/lib/Catalyst/Dispatcher.pm
index d5bb455..3cb06ea 100644
--- a/lib/Catalyst/Dispatcher.pm
+++ b/lib/Catalyst/Dispatcher.pm
@@ -251,10 +251,8 @@ sub _do_forward {
 
     no warnings 'recursion';
 
-    my $orig_args = $c->request->arguments();
-    $c->request->arguments($args);
+    local $c->request->{arguments} = $args;
     $action->dispatch( $c );
-    $c->request->arguments($orig_args);
 
     return $c->state;
 }
diff --git a/t/unit_dispatcher_requestargs_restore.t b/t/unit_dispatcher_requestargs_restore.t
new file mode 100644
index 0000000..731c4da
--- /dev/null
+++ b/t/unit_dispatcher_requestargs_restore.t
@@ -0,0 +1,57 @@
+# Insane test case for the behavior needed by Plugin::Auhorization::ACL
+
+# We have to localise $c->request->{arguments} in 
+# Catalyst::Dispatcher::_do_forward, rather than using save and restore,
+# as otherwise, the calling $c->detach on an action which says
+# die $Catalyst:DETACH causes the request arguments to not get restored,
+# and therefore sub gorch gets the wrong string $frozjob parameter.
+
+# Please feel free to break this behavior once a sane hook for safely
+# executing another action from the dispatcher (i.e. wrapping actions)
+# is present, so that the Authorization::ACL plugin can be re-written
+# to not be full of such crazy shit.
+{
+    package ACLTestApp;
+    use Test::More;
+
+    use strict;
+    use warnings;
+    use MRO::Compat;
+    use Scalar::Util ();
+
+    use base qw/Catalyst Catalyst::Controller/;
+    use Catalyst qw//;
+
+    sub execute {
+        my $c = shift;
+        my ( $class, $action ) = @_;
+
+        if ( Scalar::Util::blessed($action)
+            and $action->name ne "foobar" ) {
+              eval { $c->detach( 'foobar', [$action, 'foo'] ) };
+        }
+
+        $c->next::method( @_ );
+    }
+
+    sub foobar : Private {
+        die $Catalyst::DETACH;
+    }
+
+    sub gorch : Local {
+        my ( $self, $c, $frozjob ) = @_;
+        is $frozjob, 'wozzle';
+        $c->res->body("gorch");
+    }
+
+    __PACKAGE__->setup;
+}
+
+use strict;
+use warnings;
+use FindBin qw/$Bin/;
+use lib "$Bin/lib";
+use Catalyst::Test 'ACLTestApp';
+use Test::More tests => 1;
+
+request('http://localhost/gorch/wozzle');