[Catalyst-dev] [Patch] Object exceptions with questionable overloading

[Daniel Westermann-Clark]

Hi,

Attached is a patch to fix a problem mentioned on the Catalyst mailing
list last week with object exceptions.

The issue was originally exposed by Catalyst::Plugin::StackTrace When
object exceptions have an overload on "" but not eq, the comparison to
$DETACH in execute can cause another exception to be thrown ("no
overloaded magic").

The attached includes a test which fails without the extra check.
Patch is against the current branch.

-- 
Daniel Westermann-Clark
-------------- next part --------------
Patching locally against mirror source http://dev.catalyst.perl.org/repos/Catalyst.
==== Patch <-> level 1
Source: [No source]
Target: 4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/current/Catalyst-Runtime:4725
        (http://dev.catalyst.perl.org/repos/Catalyst)
Log:
Check that errors are refs before comparing them to $DETACH, to avoid
"no overloaded magic" exceptions. This only seems to come up when an
exception object with an overload on "" but not eq is thrown.

=== Changes
==================================================================
--- Changes	(revision 4725)
+++ Changes	(patch - level 1)
@@ -1,5 +1,8 @@
 This file documents the revision history for Perl extension Catalyst.
 
+        - check that errors are refs before comparing them to $DETACH
+          to avoid "no overloaded magic" exceptions
+
 5.7001   2006-07-19 23:46:54
         - fix for component loading
         - uri_for and uri_with now behave as they used to with non-
=== lib/Catalyst.pm
==================================================================
--- lib/Catalyst.pm	(revision 4725)
+++ lib/Catalyst.pm	(patch - level 1)
@@ -1147,7 +1147,7 @@
     my $last = pop( @{ $c->stack } );
 
     if ( my $error = $@ ) {
-        if ( $error eq $DETACH ) { die $DETACH if $c->depth > 1 }
+        if ( !ref($error) and $error eq $DETACH ) { die $DETACH if $c->depth > 1 }
         else {
             unless ( ref $error ) {
                 no warnings 'uninitialized';
@@ -2280,6 +2280,8 @@
 
 Dan Sully
 
+Daniel Westermann-Clark
+
 Danijel Milicevic
 
 David Kamholz
=== t/lib/TestApp/Controller/Engine/Response/Errors.pm
==================================================================
--- t/lib/TestApp/Controller/Engine/Response/Errors.pm	(revision 4725)
+++ t/lib/TestApp/Controller/Engine/Response/Errors.pm	(patch - level 1)
@@ -20,4 +20,11 @@
     die("I'm going to die!\n");
 }
 
+sub four : Relative {
+    my ( $self, $c ) = @_;
+
+    require TestApp::Exception;
+    die(TestApp::Exception->new);
+}
+
 1;
=== t/lib/TestApp/Exception.pm
==================================================================
--- t/lib/TestApp/Exception.pm	(revision 4725)
+++ t/lib/TestApp/Exception.pm	(patch - level 1)
@@ -0,0 +1,24 @@
+package TestApp::Exception;
+
+use strict;
+use warnings;
+use overload
+    '""' => \&as_string;
+
+sub new {
+    my ($class) = @_;
+
+    my $self = {
+        message => 'aw crap',
+    };
+
+    bless $self, $class;
+}
+
+sub as_string {
+    my ($self) = @_;
+
+    return $self->{message};
+}
+
+1;
=== t/live_engine_response_errors.t
==================================================================
--- t/live_engine_response_errors.t	(revision 4725)
+++ t/live_engine_response_errors.t	(patch - level 1)
@@ -6,7 +6,7 @@
 use FindBin;
 use lib "$FindBin::Bin/lib";
 
-use Test::More tests => 18;
+use Test::More tests => 24;
 use Catalyst::Test 'TestApp';
 
 close STDERR;    # i'm naughty :)
@@ -58,3 +58,21 @@
         'Catalyst Error'
     );
 }
+
+{
+    ok( my $response = request('http://localhost/engine/response/errors/four'),
+        'Request' );
+    ok( $response->is_error, 'Response Server Error 5xx' );
+    is( $response->code,         500,         'Response Code' );
+    is( $response->content_type, 'text/html', 'Response Content-Type' );
+    is(
+        $response->header('X-Catalyst-Action'),
+        'engine/response/errors/four',
+        'Test Action'
+    );
+    unlike(
+        $response->header('X-Catalyst-Error'),
+        qr/no overloaded magic/,
+        'Catalyst Error'
+    );
+}

==== BEGIN SVK PATCH BLOCK ====
Version: svk 1.08 (linux)

eJydVktv20YQ5iF9QOceEjSHjctUNmBZ3OUuSdmwYMOWnSJN0bruA0UAYblcSqwpUl5SdpTQQGg1
bQ9NUaS3/oL+j/bcQ4v+mNw6pB6xG9kqSkCAdnce33w7OzN76nBjC2fNppHp2Mg+/fz++vrHPBXd
u7DSrUx6QRornWWhPJGhbmZh3NFpFvGehNOUq45Mm00Mys5YuVUqTI3QwobL0zhK9EZprJ0qKXWS
YXML02yLZFsmKGcYjuO+jNoqjlNwh7HBGJy2C1QijBPZLrS3QIUV8mQq7weh1O1sp8ujjkxKS1Pt
UtDUwU0p6QVKCsA2LKII3NdF2RxRnKWXBM0CEZsguiiGi1DY+JT3++GwncpHqSfDlJdwTZL50qUG
Ib7lYkJdy2+YzDElMS2TYsd2LHBPgcSPNC3/jee/P6wMtX3t6RBWt1to8tWQ6EpxhNIuT5FUKlYJ
4koiJf0EudKP4b+Ie32ugqgDUrKH0hjpu63D7Z17FTT7YJOfxIGHlqIYxSdShTH3pId6vBOIJSQf
CdlPA7g2YPzCHZRsT8LxDCEcamOHccJMj7sWFj4VviUoMyxzQghI00t3BfwA+/UdnvJwmKRr/V55
SwsJNq8lmI4R8YbnOBY1ie9LixLRwJybhDLBsWcJ3XLG/I7+/Gr014133tPyl3XtJy3/ezt/ufaz
Njoe/RHcASqX9ZLaFcQjcIC+kEkqVY9HUW0n5OpoLiclAkmpzVxuU9pwLQt7noMtn3qm5wjmSMOF
8MacWHNyjWVpfZyY7GJiFm/lEoMmLgVP4ElEnSCSbSWTPlwVrMuEWEsvmbiSUTa7IXte5ptjOPVD
iH673y9BL7RpzSJ05tgk9LLN+k4cpSoOQ6lKFFPzVhk10Z2Me944aGL9S7U1TdFxBtltUKnhq5HZ
s2gb80oCvhJZvVWyXIa0MH5n4gXgG3PcUGORm/rB5DJLoAv9NV75w5eShP13T/XWOG2AxxL2AqeF
zDVvsURSPAVfEMfFnuNxIX3PIJ60TZ87DjYEMxiFBGuMX+P5o1H85mPtfKA9fZwMXOTHA4XW0YEM
eQpZjp6Udas3RMtIT2ToryJdoBW0ibbaG5XyTMnjAeBDk1ihC02TY6M89wK5/PpZrRnJ05WNylml
gjcqcx71NBTOPIdaVDKocQ3Dhv/ENqTHLMPyObb59WwQyEzCxqFq5+9u5rd+bd3+fq/6y+6dH1s7
L+51vtuXre5o98bz1k6+nz3by9843+uP9h5+cPCh3M3fenDyLWyN9vObeSt/MOpzccQ782OtDBKJ
klQFIt045SqCHpBMi3tJRHVpqYo2m+jh+zxpRx3QKAgHGtCTgl9dhDxJptSWZKNNmSSgUeWnSCje
r66euSFsrSIgTsl0oIDHJ2dXEli2CsZsaTVMYvuezQxfYkIcISQ2PIot4brXE2gWJhzfE47VcF3D
a0jIKhvbWFiYusJhDdP3dGJMOH7ef3HzVr6s+dq59vztrnb/WeMbLffzofbJD9rTfInQIsGKX3V1
1hGrBZ1oWxQ8VsvdlXHqDKIwOJLLM0F9Wmxrza6ElqmWq1/Wps2sVr6k6soru8eqPqfB1i84nuqi
se7U99kVfJZkNKTDmWUTB+YI4QtpGI7r+tzElm0Ll5vlyEIMDGPR/x4WVmczQmXRjLCGDrtBguIo
HKJEyl5SqIJhiQZ9dNqVEfTRykwcxe7XUErQaZB24WBmGfTR0hJyBymKYsB7jMBm2lXxabRWgYhq
zSaBQkfGc+ZhOXqur38WQYlQCQ/vQtOgWZ+nXZjvlDyBxWAQeLpJs7qreARzU1IXA6VklM6mj9rB
IEoDmGVpRm3CdNPKKPdMW3ikxnwpaoaJjZpjMr/mmrbDhE24adN/ADpHnNE=
==== END SVK PATCH BLOCK ====