Fix issue with array refs and actions from config. RT#65463
Tomas Doran [Wed, 5 Oct 2011 00:14:35 +0000 (01:14 +0100)]
Changes
lib/Catalyst/Controller.pm
t/aggregate/unit_core_controller_actions_config.t [new file with mode: 0644]
t/lib/TestApp/Controller/Action/ConfigSmashArrayRefs.pm [new file with mode: 0644]

diff --git a/Changes b/Changes
index cc25e0b..a72c75f 100644 (file)
--- a/Changes
+++ b/Changes
@@ -4,6 +4,13 @@
 
    - Make default body reponses for 302s W3C compliant. RT#71237
 
+   - Fix issue where groups of attributes to override controller actions
+     in config would be (incorrectly) overwritten, if the parser for that
+     attribute mangled the contents of the attribute. This was found
+     with Catalyst::Controller::ActionRole, where Does => [ '+Foo' ]
+     would be transformed to Does => [ 'Foo' ] and written back to config,
+     whereas Does => '+Foo' would not be changed in config. RT#65463
+
   Enhancements:
 
    - Set a matching Content-type for the redirect if Catalyst sets the
index 16b5a38..26e7e01 100644 (file)
@@ -321,7 +321,9 @@ sub _parse_attrs {
 
     %raw_attributes = (
         %raw_attributes,
-        exists $actions_config->{$name} ? %{ $actions_config->{$name } } : (),
+        # Note we deep copy array refs here to stop crapping on config
+        # when attributes are parsed. RT#65463
+        exists $actions_config->{$name} ? map { ref($_) eq 'ARRAY' ? [ @$_ ] : $_ } %{ $actions_config->{$name } } : (),
     );
 
     # Private actions with additional attributes will raise a warning and then
diff --git a/t/aggregate/unit_core_controller_actions_config.t b/t/aggregate/unit_core_controller_actions_config.t
new file mode 100644 (file)
index 0000000..06a0656
--- /dev/null
@@ -0,0 +1,10 @@
+use strict;
+use warnings;
+use Test::More;
+
+use TestApp;
+
+is(TestApp->controller("Action::ConfigSmashArrayRefs")->config->{action}{foo}{CustomAttr}[0], 'Bar', 'Config un-mangled. RT#65463');
+
+done_testing;
+
diff --git a/t/lib/TestApp/Controller/Action/ConfigSmashArrayRefs.pm b/t/lib/TestApp/Controller/Action/ConfigSmashArrayRefs.pm
new file mode 100644 (file)
index 0000000..c226dde
--- /dev/null
@@ -0,0 +1,23 @@
+package TestApp::Controller::Action::ConfigSmashArrayRefs;
+
+use strict;
+use base 'Catalyst::Controller';
+
+ sub foo : Action {}
+
+# check configuration for an inherited action
+__PACKAGE__->config(
+    action => {
+        foo => { CustomAttr => [ 'Bar' ] }
+    }
+);
+
+sub _parse_CustomAttr_attr {
+    my ($self, $app, $name, $value) = @_;
+    warn $value;
+    return CustomAttr => "PoopInYourShoes";
+}
+
+
+1;
+