Use Try::Tiny rather than eval
Tomas Doran [Sat, 30 Jun 2012 10:46:30 +0000 (11:46 +0100)]
Makefile.PL
lib/Catalyst/Authentication/Credential/Remote.pm
lib/Catalyst/Authentication/Realm.pm
lib/Catalyst/Plugin/Authentication.pm
lib/Catalyst/Plugin/Authentication/Internals.pod

index b8cd666..865a31c 100644 (file)
@@ -24,6 +24,7 @@ requires 'Moose';
 requires 'MooseX::Emulate::Class::Accessor::Fast';
 requires 'namespace::clean';
 requires 'String::RewritePrefix';
+requires 'Try::Tiny';
 
 test_requires 'Test::More' => '0.88';
 test_requires 'Test::Exception';
index 211e89d..e7bfb56 100644 (file)
@@ -2,6 +2,7 @@ package Catalyst::Authentication::Credential::Remote;
 
 use strict;
 use warnings;
+use Try::Tiny qw/ try catch /;
 
 use base 'Class::Accessor::Fast';
 
@@ -17,21 +18,27 @@ sub new {
     bless $self, $class;
 
     # we are gonna compile regular expresions defined in config parameters
-    # and explicitly throw an exception saying what parameter was invalid 
-    if (defined($config->{allow_regexp}) && ($config->{allow_regexp} ne "")) { 
-        eval { $self->allow_re( qr/$config->{allow_regexp}/ ) };
-        Catalyst::Exception->throw( "Invalid regular expression in ".
-        "'allow_regexp' configuration parameter") if $@;
+    # and explicitly throw an exception saying what parameter was invalid
+    if (defined($config->{allow_regexp}) && ($config->{allow_regexp} ne "")) {
+        try { $self->allow_re( qr/$config->{allow_regexp}/ ) }
+        catch {
+            Catalyst::Exception->throw( "Invalid regular expression in ".
+                "'allow_regexp' configuration parameter");
+        };
     }
-    if (defined($config->{deny_regexp}) && ($config->{deny_regexp} ne "")) { 
-        eval { $self->deny_re( qr/$config->{deny_regexp}/ ) };     
-        Catalyst::Exception->throw( "Invalid regular expression in ".
-             "'deny_regexp' configuration parameter") if $@;
+    if (defined($config->{deny_regexp}) && ($config->{deny_regexp} ne "")) {
+        try { $self->deny_re( qr/$config->{deny_regexp}/ ) }
+        catch {
+            Catalyst::Exception->throw( "Invalid regular expression in ".
+                 "'deny_regexp' configuration parameter");
+        };
     }
-    if (defined($config->{cutname_regexp}) && ($config->{cutname_regexp} ne "")) { 
-        eval { $self->cutname_re( qr/$config->{cutname_regexp}/ ) };
-        Catalyst::Exception->throw( "Invalid regular expression in ".
-             "'cutname_regexp' configuration parameter") if $@;
+    if (defined($config->{cutname_regexp}) && ($config->{cutname_regexp} ne "")) {
+        try { $self->cutname_re( qr/$config->{cutname_regexp}/ ) }
+        catch {
+            Catalyst::Exception->throw( "Invalid regular expression in ".
+                "'cutname_regexp' configuration parameter");
+        };
     }
     $self->source($config->{source} || 'REMOTE_USER');
     $self->realm($realm);
index 28ce7ea..c469f88 100644 (file)
@@ -3,6 +3,7 @@ package Catalyst::Authentication::Realm;
 use strict;
 use warnings;
 use String::RewritePrefix;
+use Try::Tiny qw/ try catch /;
 
 use base qw/Class::Accessor::Fast/;
 
@@ -68,48 +69,46 @@ sub new {
 
     ## Note to self - catch second exception and bitch in detail?
 
-    eval {
+    try {
         Catalyst::Utils::ensure_class_loaded( $credentialclass );
-    };
-
-    if ($@) {
+    }
+    catch {
         # If the file is missing, then try the old-style fallback,
         # but re-throw anything else for the user to deal with.
-        die unless $@ =~ /^Can't locate/;
+        die $_ unless /^Can't locate/;
         $app->log->warn( qq(Credential class "$credentialclass" not found, trying deprecated ::Plugin:: style naming. ) );
         my $origcredentialclass = $credentialclass;
         $credentialclass =~ s/Catalyst::Authentication/Catalyst::Plugin::Authentication/;
 
-        eval { Catalyst::Utils::ensure_class_loaded( $credentialclass ); };
-        if ($@) {
+        try { Catalyst::Utils::ensure_class_loaded( $credentialclass ); }
+        catch {
             # Likewise this croak is useful if the second exception is also "not found",
             # but would be confusing if it's anything else.
-            die unless $@ =~ /^Can't locate/;
+            die $_ unless /^Can't locate/;
             Carp::croak "Unable to load credential class, " . $origcredentialclass . " OR " . $credentialclass .
                         " in realm " . $self->name;
-        }
-    }
-
-    eval {
-        Catalyst::Utils::ensure_class_loaded( $storeclass );
+        };
     };
 
-    if ($@) {
+    try {
+        Catalyst::Utils::ensure_class_loaded( $storeclass );
+    }
+    catch {
         # If the file is missing, then try the old-style fallback,
         # but re-throw anything else for the user to deal with.
-        die unless $@ =~ /^Can't locate/;
+        die $_ unless /^Can't locate/;
         $app->log->warn( qq(Store class "$storeclass" not found, trying deprecated ::Plugin:: style naming. ) );
         my $origstoreclass = $storeclass;
         $storeclass =~ s/Catalyst::Authentication/Catalyst::Plugin::Authentication/;
-        eval { Catalyst::Utils::ensure_class_loaded( $storeclass ); };
-        if ($@) {
+        try { Catalyst::Utils::ensure_class_loaded( $storeclass ); }
+        catch {
             # Likewise this croak is useful if the second exception is also "not found",
             # but would be confusing if it's anything else.
-            die unless $@ =~ /^Can't locate/;
+            die $_ unless /^Can't locate/;
             Carp::croak "Unable to load store class, " . $origstoreclass . " OR " . $storeclass .
                         " in realm " . $self->name;
-        }
-    }
+        };
+    };
 
     # BACKWARDS COMPATIBILITY - if the store class does not define find_user, we define it in terms
     # of get_user and add it to the class.  this is because the auth routines use find_user,
@@ -127,13 +126,15 @@ sub new {
     ## we'll remove this soon.
     if ($storeclass->can('new')) {
         $self->store($storeclass->new($config->{'store'}, $app, $self));
-    } else {
+    }
+    else {
         $app->log->error("THIS IS DEPRECATED: $storeclass has no new() method - Attempting to use uninstantiated");
         $self->store($storeclass);
     }
     if ($credentialclass->can('new')) {
         $self->credential($credentialclass->new($config->{'credential'}, $app, $self));
-    } else {
+    }
+    else {
         $app->log->error("THIS IS DEPRECATED: $credentialclass has no new() method - Attempting to use uninstantiated");
         $self->credential($credentialclass);
     }
index 3408b75..46fbb3b 100644 (file)
@@ -863,8 +863,8 @@ default realm is checked.
 
 Returns the currently logged in user, or undef if there is none.
 Normally the user is re-retrieved from the store.
-For L<Catalyst::Authentication::Store::DBIx::Class> the user is re-restored 
-using the primary key of the user table. 
+For L<Catalyst::Authentication::Store::DBIx::Class> the user is re-restored
+using the primary key of the user table.
 Thus B<user> can throw an error even though B<user_exists>
 returned true.
 
index 9649cd8..fdbbc64 100644 (file)
@@ -32,7 +32,7 @@ is required to implement a credential or a store.
 
 There are two main entry points you need to be aware of when writing a store
 or credential module. The first is initialization and the second is during the
-actual call to the Catalyst application's authenticate method.  
+actual call to the Catalyst application's authenticate method.
 
 A simplified description of the authentication process follows:
 
@@ -70,7 +70,7 @@ C<< $c->authenticate( $userinfo, $realm ) >> called
 
 2) Credential's authenticate() method called with authinfo and realm object for current realm
 
-=over 4 
+=over 4
 
 The realm object and the authinfo hash are provided to the credential object's
 authenticate call. In most cases the credential object will attempt to
@@ -107,14 +107,14 @@ B<Sessions> - Per-Request operations
 When any user-related activity occurs, and $c->authenticate has not
 yet been called, the Catalyst::Plugin::Authentication module will
 attempt to restore the persisted user (normally from the session if one is available).
-There is only one step in this process: 
+There is only one step in this process:
 
 =over 4
 
 1) Store object's from_session() is called
 
-=back 
-    
+=back
+
 The serialized data previously returned by the store's for_session()
 method is provided to the from_session() method. The from_session()
 method should return a valid user object.
@@ -138,12 +138,12 @@ L<Catalyst::Plugin::Authentication|Catalyst::Plugin::Authentication>
 instantiates both a new credential object and a new store object. See below
 for the details of how credentials and stores are instantiated.
 
-B<NOTE>: The instances created will remain active throughout the entire 
-lifetime of the application, and so should be relatively lightweight. 
-Care should be taken to ensure that they do not grow, or retain 
-information per request, because they will be involved in each 
+B<NOTE>: The instances created will remain active throughout the entire
+lifetime of the application, and so should be relatively lightweight.
+Care should be taken to ensure that they do not grow, or retain
+information per request, because they will be involved in each
 authentication request and could therefore substantially
-hurt memory consumption over time.  
+hurt memory consumption over time.
 
 =head2 AUTHENTICATION
 
@@ -153,7 +153,7 @@ C<$c-E<gt>authenticate()> takes two arguments. The first is a hash reference
 containing all the information available about the user. This will be used to
 locate the user in the store and verify the user's credentials. The second
 argument is the realm to authenticate against. If the second argument is
-omitted, the default realm is assumed. 
+omitted, the default realm is assumed.
 
 The main authentication module then locates the credential and store objects
 for the realm specified and calls the credential object's C<authenticate()>
@@ -201,16 +201,16 @@ first argument, C<$config>, is a hash reference containing the configuration
 information for the store module. The second argument is a reference to the
 Catalyst application.
 
-Note that when new() is called, Catalyst has not yet loaded 
+Note that when new() is called, Catalyst has not yet loaded
 the various controller and model classes, nor is it definite
-that other plugins have been loaded, so your new() method 
-must not rely on any of those being present.  If any of 
+that other plugins have been loaded, so your new() method
+must not rely on any of those being present.  If any of
 this is required for your store to function, you should
-defer that part of initialization until the first method call. 
+defer that part of initialization until the first method call.
 
 The C<new()> method should return a blessed reference to your store object.
 
-=item find_user( $authinfo, $c ) 
+=item find_user( $authinfo, $c )
 
 This is the workhorse of any authentication store. It's job is to take the
 information provided to it via the C<$authinfo> hashref and locate the user
@@ -219,10 +219,10 @@ of anything else is considered to mean no user was found that matched the
 information provided.
 
 How C<find_user()> accomplishes it's job is entirely up to you, the author, as
-is what $authinfo is required to contain.  Many stores will simply use a 
+is what $authinfo is required to contain.  Many stores will simply use a
 username element in $authinfo to locate the user, but more advanced functionality
 is possible and you may bend the $authinfo to your needs.  Be aware, however, that
-both Credentials and Stores usually work with the same $authinfo hash, so take 
+both Credentials and Stores usually work with the same $authinfo hash, so take
 care to avoid overlapping element names.
 
 Please note that this routine may be called numerous times in various
@@ -234,22 +234,22 @@ store object.
 =item for_session( $c, $user )
 
 This method is responsible for preparing a user object for storage in the session.
-It should return information that can be placed in the session and later used to 
+It should return information that can be placed in the session and later used to
 restore a user object (using the C<from_session()> method).  It should therefore
 ensure that whatever information provided can be used by the C<from_session()>
 method to locate the unique user being saved.  Note that there is no guarantee
-that the same Catalyst instance will receive both the C<for_session()> and 
+that the same Catalyst instance will receive both the C<for_session()> and
 C<from_session()> calls.  You should take care to provide information that can
-be used to restore a user, regardless of the current state of the application. 
+be used to restore a user, regardless of the current state of the application.
 A good rule of thumb is that if C<from_session()> can revive the user with the
-given information even if the Catalyst application has just started up, you are 
+given information even if the Catalyst application has just started up, you are
 in good shape.
 
 =item from_session( $c, $frozenuser )
 
-This method is called whenever a user is being restored from the session.  
+This method is called whenever a user is being restored from the session.
 C<$frozenuser> contains the information that was stored in the session for the user.
-This will under normal circumstances be the exact data your store returned from 
+This will under normal circumstances be the exact data your store returned from
 the previous call to C<for_session()>.  C<from_session()> should return a valid
 user object.
 
@@ -260,9 +260,9 @@ underlying user object is capable of. This is pretty-well free-form and the
 main purpose is to allow graceful integration with credentials and
 applications that may provide advanced functionality based on whether the
 underlying user object can do certain things. In most cases you will want to
-pass this directly to the underlying user class' C<supports> method. Note that 
-this is used as a B<class> method against the user class and therefore must 
-be able to function without an instantiated user object. 
+pass this directly to the underlying user class' C<supports> method. Note that
+this is used as a B<class> method against the user class and therefore must
+be able to function without an instantiated user object.
 
 =back
 
@@ -307,19 +307,19 @@ should experience no problems.
 
 =item id( )
 
-The C<id()> method should return a unique id (scalar) that can be used to 
+The C<id()> method should return a unique id (scalar) that can be used to
 retreive this user from the store.  Often this will be provided to the store's
-C<find_user()> routine as C<id =E<gt> $user-E<gt>id> so you should ensure that your 
-store's C<find_user()> can cope with that. 
+C<find_user()> routine as C<id =E<gt> $user-E<gt>id> so you should ensure that your
+store's C<find_user()> can cope with that.
 
 =item supports( $feature, $subfeature ... )
 
 This method checks to see if the user class supports a particular feature.  It
-is implemented such that each argument provides a subfeature of the previous 
+is implemented such that each argument provides a subfeature of the previous
 argument. In other words, passing 'foo', 'bar'  would return true if the user
 supported the 'foo' feature, and the 'bar' feature of 'foo'.   This is implemented
 in Catalyst::Authentication::User, so if your class inherits from that, you
-do not need to implement this and can instead implement supported_features(). 
+do not need to implement this and can instead implement supported_features().
 
 B<Note:> If you want the authentication module to be able to save your user in
 the session you must return true when presented with the feature 'session'.
@@ -337,15 +337,15 @@ This method should return the value of the field matching fieldname provided,
 or undef if there is no field matching that fieldname. In most cases this will
 access the underlying storage mechanism for the user data and return the
 information. This is used as a standard method of accessing an authenticated
-user's data, and MUST be implemented by all user objects.  
+user's data, and MUST be implemented by all user objects.
 
-B<Note>: There is no equivalent 'set' method. Each user class is 
-likely to vary greatly in how data must be saved and it is 
-therefore impractical to try to provide a standard way of 
-accomplishing it. When an application developer needs to save 
-data, they should obtain the underlying object / data by 
+B<Note>: There is no equivalent 'set' method. Each user class is
+likely to vary greatly in how data must be saved and it is
+therefore impractical to try to provide a standard way of
+accomplishing it. When an application developer needs to save
+data, they should obtain the underlying object / data by
 calling get_object, and work with it directly.
-    
+
 
 =item get_object( )
 
@@ -358,7 +358,7 @@ across user classes. If your object is not backed by another class, or you
 need to provide additional intermediate functionality, it is perfectly
 reasonable to return C<$self>.
 
-=back 
+=back
 
 
 =head1 WRITING A CREDENTIAL
@@ -368,25 +368,25 @@ one class to implement, and it consists of only two required routines. They are:
 
     new()           - instantiates the credential object
     authenticate()  - performs the authentication and returns a user object
-    
+
 =head2 CREDENTIAL METHODS
 
 =over 4
 
 =item new( $config, $app, $realm )
 
-Like the Store method of the same name, the C<new()> method is called only 
-once, during the setup process of 
+Like the Store method of the same name, the C<new()> method is called only
+once, during the setup process of
 L<Catalyst::Plugin::Authentication|Catalyst::Plugin::Authentication>. The
 first argument, C<$config>, is a hash reference containing the configuration
-information for the credential module. The second argument is a reference 
+information for the credential module. The second argument is a reference
 to the Catalyst application.  $realm is the instantiated Realm object, which
 you may use to access realm routines - such as find_user.
 
-Again, when the credential's new() method is called, Catalyst 
-has not yet loaded the various controller and model classes. 
+Again, when the credential's new() method is called, Catalyst
+has not yet loaded the various controller and model classes.
 
-The new method should perform any necessary setup required and instantiate 
+The new method should perform any necessary setup required and instantiate
 your credential object.  It should return your instantiated credential.
 
 =item authenticate( $c, $realm, $authinfo )
@@ -394,16 +394,16 @@ your credential object.  It should return your instantiated credential.
 This is the workhorse of your credential.  When $c->authenticate() is called
 the L<Catalyst::Plugin::Authentication|Catalyst::Plugin::Authentication> module retrieves the
 realm object and passes it, along with the $authinfo hash
-to your credential's authenticate method.  Your module should use the 
-$authinfo hash to obtain the user from the realm passed, and then perform 
+to your credential's authenticate method.  Your module should use the
+$authinfo hash to obtain the user from the realm passed, and then perform
 any credential verification steps necessary to authenticate the user.  This
 method should return the user object returned by the authentication store if
-credential verification succeeded.  It should return undef on failure.  
+credential verification succeeded.  It should return undef on failure.
 
 How your credential module performs the credential verification is entirely
 up to you.  In most cases, the credential will retrieve a user from the store
-first (using the stores find_user() method), and then validate the user's 
-information.  However, this does not have to be the case.  
+first (using the stores find_user() method), and then validate the user's
+information.  However, this does not have to be the case.
 
 It is perfectly acceptable for your credential to perform other tasks prior to
 attempting to retrieve the user from the store. It may also make sense for
@@ -412,29 +412,29 @@ question, for example, finding a user id based on an encrypted token.
 In these scenarios, the $authinfo hash passed to find_user()
 can be different than that which is passed in to $c->authenticate(). Once
 again this is perfectly acceptable if it makes sense for your credential,
-though you are strongly advised to note this behavior clearly in your 
-credential's documentation - as application authors are almost 
-certainly expecting the user to be found using the information provided 
+though you are strongly advised to note this behavior clearly in your
+credential's documentation - as application authors are almost
+certainly expecting the user to be found using the information provided
 to $c->authenticate().
 
 Look at the L<Catalyst::Authentication::Credential::Password|Catalyst::Authentication::Credential::Password>
-module source to see this in action.  In order to avoid possible 
-mismatches between the encrypted and unencrypted passwords, the password 
-credential actually removes the provided password from the authinfo 
-array.  It does this because, in many cases, the store's password 
-field will be encrypted in some way, and the password passed to 
-$c->authenticate is almost certainly in plaintext. 
+module source to see this in action.  In order to avoid possible
+mismatches between the encrypted and unencrypted passwords, the password
+credential actually removes the provided password from the authinfo
+array.  It does this because, in many cases, the store's password
+field will be encrypted in some way, and the password passed to
+$c->authenticate is almost certainly in plaintext.
 
 NOTE: You should always assume that a store is going to use all
-the information passed to it to locate the user in question. 
+the information passed to it to locate the user in question.
 If there are fields in the $authinfo hash that you are sure
-are specific to your credential, you may want to consider 
+are specific to your credential, you may want to consider
 removing them before user retrieval.  A better solution is to
-place those arguments that are specific to your credential 
+place those arguments that are specific to your credential
 within their own subhash named after your module.
+
 The L<Catalyst::Authentication::Store::DBIx::Class|Catalyst::Authentication::Store::DBIx::Class> module does this
-in order to encapsulate arguments intended specifically for 
+in order to encapsulate arguments intended specifically for
 that module. See the L<Catalyst::Authentication::Store::DBIx::Class::User|Catalyst::Authentication::Store::DBIx::Class::User>
 source for details.