Fixed so that session_expires == stored session expires
Robert Rothenberg [Fri, 13 Sep 2013 09:43:32 +0000 (10:43 +0100)]
lib/Catalyst/Plugin/Session.pm
t/lib/SessionExpiry.pm [new file with mode: 0644]
t/lib/SessionExpiry/Controller/Root.pm [new file with mode: 0644]
t/lib/SessionTestApp.pm
t/lib/SessionTestApp/Controller/Root.pm
t/live_expiry_threshold.t

index 9d65185..9e11a2b 100644 (file)
@@ -150,19 +150,15 @@ sub _save_session_expires {
 
     if ( defined($c->_session_expires) ) {
 
-        my $threshold = $c->_session_plugin_config->{expiry_threshold} || 0;
-
-        if ( my $sid = $c->sessionid ) {
-            my $cutoff = $c->_get_stored_session_expires - $threshold;
-
-            if (!$threshold || $cutoff <= time) {
-                my $expires = $c->session_expires; # force extension
-                $c->store_session_data( "expires:$sid" => $expires );
+        if (my $sid = $c->sessionid) {
 
+            my $current  = $c->_get_stored_session_expires;
+            my $extended = $c->session_expires;
+            if ($extended > $current) {
+                $c->store_session_data( "expires:$sid" => $extended );
             }
 
         }
-
     }
 }
 
@@ -373,9 +369,32 @@ sub session_expires {
 
 sub extend_session_expires {
     my ( $c, $expires ) = @_;
-    $c->_extended_session_expires( my $updated = $c->calculate_initial_session_expires() );
-    $c->extend_session_id( $c->sessionid, $updated );
-    return $updated;
+
+    my $threshold = $c->_session_plugin_config->{expiry_threshold} || 0;
+
+    if ( my $sid = $c->sessionid ) {
+        my $expires = $c->_get_stored_session_expires;
+        my $cutoff  = $expires - $threshold;
+
+        if (!$threshold || $cutoff <= time) {
+
+            $c->_extended_session_expires( my $updated = $c->calculate_initial_session_expires() );
+            $c->extend_session_id( $sid, $updated );
+
+            return $updated;
+
+        } else {
+
+            return $expires;
+
+        }
+
+    } else {
+
+        return;
+
+    }
+
 }
 
 sub change_session_expires {
diff --git a/t/lib/SessionExpiry.pm b/t/lib/SessionExpiry.pm
new file mode 100644 (file)
index 0000000..3397a7d
--- /dev/null
@@ -0,0 +1,21 @@
+package SessionExpiry;
+use Catalyst
+    qw/Session Session::Store::Dummy Session::State::Cookie Authentication/;
+
+use strict;
+use warnings;
+
+__PACKAGE__->config(
+    'Plugin::Session' => {
+
+        expires          => 20,
+        expiry_threshold => 10,
+
+    },
+
+);
+
+__PACKAGE__->setup;
+
+__PACKAGE__;
+
diff --git a/t/lib/SessionExpiry/Controller/Root.pm b/t/lib/SessionExpiry/Controller/Root.pm
new file mode 100644 (file)
index 0000000..1e4b676
--- /dev/null
@@ -0,0 +1,22 @@
+package SessionExpiry::Controller::Root;
+use strict;
+use warnings;
+
+use base qw/Catalyst::Controller/;
+
+__PACKAGE__->config( namespace => '' );
+
+sub session_data_expires : Global {
+    my ( $self, $c ) = @_;
+    $c->session;
+    if (my $sid = $c->sessionid) {
+        $c->finalize_headers(); # force expiration to be updated
+        $c->res->output($c->get_session_data("expires:$sid"));
+    }
+}
+
+sub session_expires : Global {
+    my ($self, $c) = @_;
+    $c->session;
+    $c->res->output($c->session_expires);
+}
index 0a93392..d2a0f77 100644 (file)
@@ -10,10 +10,6 @@ __PACKAGE__->config('Plugin::Session' => {
         # needed for live_verify_user_agent.t; should be harmless for other tests
         verify_user_agent => 1,
         verify_address => 1,
-
-        expires => 20,
-        expiry_threshold => 10,
-
     },
 
     'Plugin::Authentication' => {
index 9a38bc8..0afc633 100644 (file)
@@ -133,13 +133,4 @@ sub reset_session_expires : Global {
     $c->res->output($c->session_expires);
 }
 
-sub get_expires : Global {
-    my ( $self, $c ) = @_;
-    $c->session;
-    if (my $sid = $c->sessionid) {
-        $c->finalize_headers(); # force expiration to be updated
-        $c->res->output($c->get_session_data("expires:$sid"));
-    }
-}
-
 1;
index 9d4fcf5..ea3f83f 100644 (file)
@@ -19,27 +19,41 @@ BEGIN {
 }
 
 use lib "t/lib";
-use Test::WWW::Mechanize::Catalyst "SessionTestApp";
+use Test::WWW::Mechanize::Catalyst "SessionExpiry";
 
 my $ua = Test::WWW::Mechanize::Catalyst->new;
 
-my $res = $ua->get( "http://localhost/get_expires" );
-ok($res->is_success, "get expires");
+my $res = $ua->get( "http://localhost/session_data_expires" );
+ok($res->is_success, "session_data_expires");
 
-my $expiry = $res->decoded_content;
+my $expiry = $res->decoded_content + 0;
+
+$res = $ua->get( "http://localhost/session_expires" );
+ok($res->is_success, "session_expires");
+is($res->decoded_content, $expiry, "session_expires == session_data_expires");
 
 sleep(1);
 
-$res = $ua->get( "http://localhost/get_expires" );
-ok($res->is_success, "get expires");
+$res = $ua->get( "http://localhost/session_data_expires" );
+ok($res->is_success, "session_data_expires");
 
 is($res->decoded_content, $expiry, "expiration not updated");
 
+$res = $ua->get( "http://localhost/session_expires" );
+ok($res->is_success, "session_expires");
+is($res->decoded_content, $expiry, "session_expires == session_data_expires");
+
 sleep(10);
 
-$res = $ua->get( "http://localhost/get_expires" );
-ok($res->is_success, "get expires");
+$res = $ua->get( "http://localhost/session_data_expires" );
+ok($res->is_success, "session_data_expires");
+
+my $updated = $res->decoded_content + 0;
+ok($updated > $expiry, "expiration updated");
+
+$res = $ua->get( "http://localhost/session_expires" );
+ok($res->is_success, "session_expires");
+is($res->decoded_content, $updated, "session_expires == session_data_expires");
 
-isnt($res->decoded_content, $expiry, "expiration updated");
 
 done_testing;