Bug #112679 for Catalyst-Plugin-Session-Store-DBIC: Error calling $c->change_session

Thu Mar 03 14:23:04 2016 cees-perl [...] crtconsulting.ca - Ticket created

Subject:

Error calling $c->change_session_id (patch included)

The DBIC session store plugin errors out with the following message when calling $c->change_session_id: ERROR - Caught exception in engine "DBIx::Class::Row::update(): Can't update MyApp::Model::DB::Session=HASH(0x7f8f3d4f5c20): row not found at ~/perl5/perlbrew/perls/perl-5.20.2/lib/site_perl/5.20.2/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm line 124" I have attached a patch to fix the problem, which includes a new set of tests that can be used to recreate the issue. The problem occurs when the old session is deleted, but the Delegate object keeps an old copy of the DBIx::Class row object in an accessor and continues to use that old object. This means the cached object keeps it's old session ID, and when it tries to flush to the database, since the ->in_storage flag for the row is still set to true, it attempts an update of the row in the database which fails. The fix is simple. When asking for the row object, check to make sure that the row object that is cached matches the session ID that is being requested. This just requires a small change to the 'session' method in Catalyst/Plugin/Session/Store/DBIC/Delegate.pm. I would have provided a pull request through github, but it looks like the distribution there is out of date. I hope a patch is acceptable. Cheers, Cees Hek

Subject:

Catalyst-Plugin-Session-Store-DBIC-0.14.patch

Only in Catalyst-Plugin-Session-Store-DBIC-0.14: MANIFEST Only in Catalyst-Plugin-Session-Store-DBIC-0.14: META.yml Only in Catalyst-Plugin-Session-Store-DBIC-0.14: MYMETA.json diff -ru Catalyst-Plugin-Session-Store-DBIC-0.14/lib/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm Catalyst-Plugin-Session-Store-DBIC-0.14.patched/lib/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm --- Catalyst-Plugin-Session-Store-DBIC-0.14/lib/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm 2013-07-27 04:33:16.000000000 -0400 +++ Catalyst-Plugin-Session-Store-DBIC-0.14.patched/lib/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm 2016-03-03 13:52:59.000000000 -0500 @@ -31,7 +31,7 @@ my $row = $self->_session_row; - unless ($row) { + if (not $row or $row->id ne $key) { $row = $self->_load_row($key); $self->_session_row($row); } diff -ru Catalyst-Plugin-Session-Store-DBIC-0.14/t/05dbic-schema.t Catalyst-Plugin-Session-Store-DBIC-0.14.patched/t/05dbic-schema.t --- Catalyst-Plugin-Session-Store-DBIC-0.14/t/05dbic-schema.t 2013-07-27 04:33:16.000000000 -0400 +++ Catalyst-Plugin-Session-Store-DBIC-0.14.patched/t/05dbic-schema.t 2016-03-03 13:55:22.000000000 -0500 @@ -21,7 +21,7 @@ eval { require Catalyst::Model::DBIC::Schema } or plan skip_all => "Catalyst::Model::DBIC::Schema is required for this test"; - plan tests => 14; + plan tests => 21; $TestApp::DB_FILE = "$FindBin::Bin/session.db"; @@ -64,6 +64,16 @@ $mech->get_ok("http://localhost/session/output?key=$key", 'request to get session value'); $mech->content_is($value, 'got session value back'); +# Check change_session_id +$mech->get_ok("http://localhost/session/sessionid", 'request current session ID'); +my $sid = $mech->content; +$mech->get_ok("http://localhost/session/change", 'request to change session ID'); +$mech->content_is('ok', 'successful'); +$mech->get_ok("http://localhost/session/sessionid", 'request current session ID'); +ok($mech->content ne $sid, 'got a new session ID'); +$mech->get_ok("http://localhost/session/output?key=$key", 'request to get session value'); +$mech->content_is($value, 'got session value back'); + # Exceed our session storage capactity $value = "blah" x 200; warnings_exist { diff -ru Catalyst-Plugin-Session-Store-DBIC-0.14/t/lib/TestApp/Controller/Session.pm Catalyst-Plugin-Session-Store-DBIC-0.14.patched/t/lib/TestApp/Controller/Session.pm --- Catalyst-Plugin-Session-Store-DBIC-0.14/t/lib/TestApp/Controller/Session.pm 2013-07-27 04:33:16.000000000 -0400 +++ Catalyst-Plugin-Session-Store-DBIC-0.14.patched/t/lib/TestApp/Controller/Session.pm 2016-03-03 13:54:33.000000000 -0500 @@ -22,6 +22,22 @@ $c->res->body($c->session->{$key}); } +sub change : Local { + my ($self, $c) = @_; + + my $oldsid = $c->sessionid; + $c->change_session_id; + my $newsid = $c->sessionid; + + $c->res->body($oldsid ne $newsid ? 'ok' : 'not ok'); +} + +sub sessionid : Local { + my ($self, $c) = @_; + + $c->res->body($c->sessionid); +} + sub delete : Local { my ($self, $c) = @_;

Sat Sep 24 19:22:33 2016 violapiratejunky [...] gmail.com - Correspondence added

Bump!!! I would love for this to be fixed. On Thu Mar 03 14:23:04 2016, CEESHEK wrote: Show quoted text

> > The DBIC session store plugin errors out with the following message > when calling $c->change_session_id: > > ERROR - Caught exception in engine "DBIx::Class::Row::update(): Can't > update MyApp::Model::DB::Session=HASH(0x7f8f3d4f5c20): row not found > at ~/perl5/perlbrew/perls/perl- > 5.20.2/lib/site_perl/5.20.2/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm > line 124" > > I have attached a patch to fix the problem, which includes a new set > of tests that can be used to recreate the issue. > > The problem occurs when the old session is deleted, but the Delegate > object keeps an old copy of the DBIx::Class row object in an accessor > and continues to use that old object. This means the cached object > keeps it's old session ID, and when it tries to flush to the database, > since the ->in_storage flag for the row is still set to true, it > attempts an update of the row in the database which fails. > > The fix is simple. When asking for the row object, check to make sure > that the row object that is cached matches the session ID that is > being requested. This just requires a small change to the 'session' > method in Catalyst/Plugin/Session/Store/DBIC/Delegate.pm. > > I would have provided a pull request through github, but it looks like > the distribution there is out of date. I hope a patch is acceptable. > > Cheers, > > Cees Hek

Sat Sep 24 19:22:34 2016 The RT System itself - Status changed from 'new' to 'open'

Sat Sep 24 19:52:24 2016 violapiratejunky [...] gmail.com - Correspondence added

By the way, I've noticed that your fix works when they are called in this order: $c->delete_session; $c->change_session_id; But NOT when they are called in this order: $c->change_session_id; $c->delete_session; Thanks for the fix though! On Thu Mar 03 14:23:04 2016, CEESHEK wrote: Show quoted text

> > The DBIC session store plugin errors out with the following message > when calling $c->change_session_id: > > ERROR - Caught exception in engine "DBIx::Class::Row::update(): Can't > update MyApp::Model::DB::Session=HASH(0x7f8f3d4f5c20): row not found > at ~/perl5/perlbrew/perls/perl- > 5.20.2/lib/site_perl/5.20.2/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm > line 124" > > I have attached a patch to fix the problem, which includes a new set > of tests that can be used to recreate the issue. > > The problem occurs when the old session is deleted, but the Delegate > object keeps an old copy of the DBIx::Class row object in an accessor > and continues to use that old object. This means the cached object > keeps it's old session ID, and when it tries to flush to the database, > since the ->in_storage flag for the row is still set to true, it > attempts an update of the row in the database which fails. > > The fix is simple. When asking for the row object, check to make sure > that the row object that is cached matches the session ID that is > being requested. This just requires a small change to the 'session' > method in Catalyst/Plugin/Session/Store/DBIC/Delegate.pm. > > I would have provided a pull request through github, but it looks like > the distribution there is out of date. I hope a patch is acceptable. > > Cheers, > > Cees Hek

Thu Sep 29 02:40:17 2016 violapiratejunky [...] gmail.com - Correspondence added

I also realized that this doesn't work when you call delete_session_data in DBIC.pm. This deletes the row out of the database, but then when DBIC/Delegate.pm is called that row hasn't been unset and so it tries to update it. On Sat Sep 24 19:52:24 2016, srchulo wrote: Show quoted text

> By the way, > > I've noticed that your fix works when they are called in this order: > > $c->delete_session; > $c->change_session_id; > > But NOT when they are called in this order: > > $c->change_session_id; > $c->delete_session; > > Thanks for the fix though! > > On Thu Mar 03 14:23:04 2016, CEESHEK wrote:

> > > > The DBIC session store plugin errors out with the following message > > when calling $c->change_session_id: > > > > ERROR - Caught exception in engine "DBIx::Class::Row::update(): Can't > > update MyApp::Model::DB::Session=HASH(0x7f8f3d4f5c20): row not found > > at ~/perl5/perlbrew/perls/perl- > > 5.20.2/lib/site_perl/5.20.2/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm > > line 124" > > > > I have attached a patch to fix the problem, which includes a new set > > of tests that can be used to recreate the issue. > > > > The problem occurs when the old session is deleted, but the Delegate > > object keeps an old copy of the DBIx::Class row object in an accessor > > and continues to use that old object. This means the cached object > > keeps it's old session ID, and when it tries to flush to the database, > > since the ->in_storage flag for the row is still set to true, it > > attempts an update of the row in the database which fails. > > > > The fix is simple. When asking for the row object, check to make sure > > that the row object that is cached matches the session ID that is > > being requested. This just requires a small change to the 'session' > > method in Catalyst/Plugin/Session/Store/DBIC/Delegate.pm. > > > > I would have provided a pull request through github, but it looks like > > the distribution there is out of date. I hope a patch is acceptable. > > > > Cheers, > > > > Cees Hek

> >

Thu Sep 29 03:02:35 2016 violapiratejunky [...] gmail.com - Correspondence added

Okay, so I actually think the approach mentioned above in the first comment was wrong. It works for that case, but it doesn't really solve the issue. The issue is that in DBIC.pm in delete_session_data the session is deleted from the database, but it is not cleared from DBIC/Delegate.pm, so when flush is called on DBIC/Delegate.pm then it tries to update a deleted row. I changed delete_session_data in DBIC.pm to: sub delete_session_data { my ($c, $key) = @_; # expires is stored on the session row for compatibility with Store::DBI return if $key =~ /^expires/; $c->session_store_model->search({ $c->session_store_dbic_id_field => $key, })->delete; #my additions begin here my ($field) = split(':', $key); if ($field eq 'session') { $c->_session_store_delegate->delete_session; } elsif ($field eq 'flash') { $c->_session_store_delegate->delete_flash; } } And I added these two methods to DBIC/Delegate.pm: =head2 delete_session Deletes the session row LOCALLY for this Delegate (sets the accessor to undef). =cut sub delete_session { my ($self, $c) = @_; $self->_session_row(undef); } =head2 delete_flash Deletes the flash row LOCALLY for this Delegate (sets the accessor to undef). =cut sub delete_flash { my ($self, $c) = @_; $self->_flash_row(undef); } I'm not sure if you would need to clear any local session or flash rows in DBIC/Delegate.pm if delete_expired_sessions is called in DBIC.pm. However, I think you would need to if any of the deleted expired sessions was the current row in _session_row or _flash_row in DBIC/Delegate.pm. Please advise if I'm wrong! This seems to work, so this is what I will be using for now. I would love to have this fixed in an update-- it's getting tricky between me and my teammates to patch modules manually. On Thu Sep 29 02:40:17 2016, srchulo wrote: Show quoted text

> I also realized that this doesn't work when you call > delete_session_data in DBIC.pm. This deletes the row out of the > database, but then when DBIC/Delegate.pm is called that row hasn't > been unset and so it tries to update it. > > On Sat Sep 24 19:52:24 2016, srchulo wrote:

> > By the way, > > > > I've noticed that your fix works when they are called in this order: > > > > $c->delete_session; > > $c->change_session_id; > > > > But NOT when they are called in this order: > > > > $c->change_session_id; > > $c->delete_session; > > > > Thanks for the fix though! > > > > On Thu Mar 03 14:23:04 2016, CEESHEK wrote:

> > > > > > The DBIC session store plugin errors out with the following message > > > when calling $c->change_session_id: > > > > > > ERROR - Caught exception in engine "DBIx::Class::Row::update(): > > > Can't > > > update MyApp::Model::DB::Session=HASH(0x7f8f3d4f5c20): row not > > > found > > > at ~/perl5/perlbrew/perls/perl- > > > 5.20.2/lib/site_perl/5.20.2/Catalyst/Plugin/Session/Store/DBIC/Delegate.pm > > > line 124" > > > > > > I have attached a patch to fix the problem, which includes a new > > > set > > > of tests that can be used to recreate the issue. > > > > > > The problem occurs when the old session is deleted, but the > > > Delegate > > > object keeps an old copy of the DBIx::Class row object in an > > > accessor > > > and continues to use that old object. This means the cached object > > > keeps it's old session ID, and when it tries to flush to the > > > database, > > > since the ->in_storage flag for the row is still set to true, it > > > attempts an update of the row in the database which fails. > > > > > > The fix is simple. When asking for the row object, check to make > > > sure > > > that the row object that is cached matches the session ID that is > > > being requested. This just requires a small change to the > > > 'session' > > > method in Catalyst/Plugin/Session/Store/DBIC/Delegate.pm. > > > > > > I would have provided a pull request through github, but it looks > > > like > > > the distribution there is out of date. I hope a patch is > > > acceptable. > > > > > > Cheers, > > > > > > Cees Hek

> > > >

Mon Feb 04 20:26:44 2019 bokutin [...] cpan.org - Correspondence added

I also encountered this bug. CatalystX::SimpleLogin did not work. https://metacpan.org/source/ABRAXXA/CatalystX-SimpleLogin-0.20/lib/CatalystX/SimpleLogin/Controller/Login.pm#L110 I am glad if it resolves.

Bug #112679 for Catalyst-Plugin-Session-Store-DBIC: Error calling $c->change_session_id (patch included)