Bugzilla – Attachment 121337 Details for
Bug 17427
Replace CGI::Session with Data::Session
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17427: Replace CGI::Session with Data::Session
Bug-17427-Replace-CGISession-with-DataSession.patch (text/plain), 11.27 KB, created by
Jonathan Druart
on 2021-05-24 14:14:44 UTC
(
hide
)
Description:
Bug 17427: Replace CGI::Session with Data::Session
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2021-05-24 14:14:44 UTC
Size:
11.27 KB
patch
obsolete
>From 8af5e7d52fb59f64be1c818775008fae633780b4 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 11 Oct 2016 14:35:48 +0100 >Subject: [PATCH] Bug 17427: Replace CGI::Session with Data::Session > >CGI::Session is not maintained for ages and seems quite buggy. >It would be the culprit of random logout problems. > >Another issue is the following crazy code in CGI::Session::Driver::DBI > >sub DESTROY { > my $self = shift; > > unless ( defined $self->{Handle} && $self->{Handle} -> ping ) { > $self->set_error(__PACKAGE__ . '::DESTROY(). Database handle has gone away'); > return; > } > > unless ( $self->{Handle}->{AutoCommit} ) { > $self->{Handle}->commit; > } > if ( $self->{_disconnect} ) { > $self->{Handle}->disconnect; > } >} > >If AutoCommit is off, it will commit anyway... > >It causes some tests to fail (api) if SessionStorage is set to MySQL > >Note that PostgreSQL is not supported yet so no need to implement it. > >Test plan: >0/ The dependencies are not packaged for debian so far, so install it >via cpan >1/ Test the 3 different values for SessionStorage >2/ Using different browsers (or sessions) login into Koha with different >users. Naviguate => you should not be logged out >3/ Set SessionStorage to Memcached, stop memcached make sure that it >defaults to file. >--- > C4/Auth.pm | 43 +++++++++++-------- > C4/InstallAuth.pm | 18 +++++--- > circ/circulation.pl | 2 +- > cpanfile | 4 +- > .../en/modules/admin/preferences/admin.pref | 1 - > members/routing-lists.pl | 1 - > tools/background-job-progress.pl | 1 - > 7 files changed, 39 insertions(+), 31 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index f281def2ce0..c7c1b91ba97 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -24,7 +24,7 @@ use Carp qw/croak/; > use Digest::MD5 qw(md5_base64); > use JSON qw/encode_json/; > use URI::Escape; >-use CGI::Session; >+use Data::Session; > > require Exporter; > use C4::Context; >@@ -1232,7 +1232,7 @@ sub checkauth { > $session->param( 'desk_name', $desk_name); > $session->param( 'flags', $userflags ); > $session->param( 'emailaddress', $emailaddress ); >- $session->param( 'ip', $session->remote_addr() ); >+ $session->param( 'ip', $ENV{REMOTE_ADDR} ); > $session->param( 'lasttime', time() ); > $session->param( 'interface', $type); > $session->param( 'shibboleth', $shibSuccess ); >@@ -1261,7 +1261,7 @@ sub checkauth { > C4::Context->_unset_userenv($sessionID); > } > $session->param( 'lasttime', time() ); >- $session->param( 'ip', $session->remote_addr() ); >+ $session->param( 'ip', $ENV{REMOTE_ADDR} ); > $session->param( 'sessiontype', 'anon' ); > $session->param( 'interface', $type); > } >@@ -1273,7 +1273,7 @@ sub checkauth { > $debug and warn "Initiating an anonymous session..."; > > # setting a couple of other session vars... >- $session->param( 'ip', $session->remote_addr() ); >+ $session->param( 'ip', $ENV{REMOTE_ADDR} ); > $session->param( 'lasttime', time() ); > $session->param( 'sessiontype', 'anon' ); > $session->param( 'interface', $type); >@@ -1686,7 +1686,7 @@ sub check_api_auth { > $session->param( 'branchname', $branchname ); > $session->param( 'flags', $userflags ); > $session->param( 'emailaddress', $emailaddress ); >- $session->param( 'ip', $session->remote_addr() ); >+ $session->param( 'ip', $ENV{REMOTE_ADDR} ); > $session->param( 'lasttime', time() ); > $session->param( 'interface', 'api' ); > } >@@ -1833,10 +1833,9 @@ sub check_cookie_auth { > > =head2 get_session > >- use CGI::Session; > my $session = get_session($sessionID); > >-Given a session ID, retrieve the CGI::Session object used to store >+Given a session ID, retrieve the Data::Session object used to store > the session's state. The session object can be used to store > data that needs to be accessed by different scripts during a > user's session. >@@ -1850,28 +1849,34 @@ sub _get_session_params { > my $storage_method = C4::Context->preference('SessionStorage'); > if ( $storage_method eq 'mysql' ) { > my $dbh = C4::Context->dbh; >- return { dsn => "driver:MySQL;id:md5", dsn_args => { Handle => $dbh } }; >- } >- elsif ( $storage_method eq 'Pg' ) { >- my $dbh = C4::Context->dbh; >- return { dsn => "driver:PostgreSQL;id:md5", dsn_args => { Handle => $dbh } }; >- } >- elsif ( $storage_method eq 'memcached' && Koha::Caches->get_instance->memcached_cache ) { >- my $memcached = Koha::Caches->get_instance()->memcached_cache; >- return { dsn => "driver:memcached;id:md5", dsn_args => { Memcached => $memcached } }; >+ # Note that the doc is wrong, type is required, if not provided it default to File >+ return { dbh => $dbh, type => 'driver:mysql;id:MD5' }; >+ } elsif ( $storage_method eq 'Pg' ) { >+ die "The system preference SessionStorage is set to 'Pg' but this is not supported yet!"; >+ } elsif ( $storage_method eq 'memcached' and my $memcached = Koha::Caches->get_instance->memcached_cache ) { >+ return { >+ type => 'driver:Memcached;id:SHA1;serialize:DataDumper', >+ cache => $memcached, >+ }; >+ } elsif ( $storage_method ne 'tmp' ) { >+ warn "The system preference SessionStorage is set to an invalid value '$storage_method'"; > } > else { > # catch all defaults to tmp should work on all systems >+ if ( $storage_method ne 'tmp' ) { >+ warn "The session has not been correctly retrieved, defaulting to temporary storage: " . $Data::Session::errstr; >+ } > my $dir = C4::Context::temporary_directory; >- my $instance = C4::Context->config( 'database' ); #actually for packages not exactly the instance name, but generally safer to leave it as it is >- return { dsn => "driver:File;id:md5", dsn_args => { Directory => "$dir/cgisess_$instance" } }; >+ my $instance = C4::Context->config('database'); #actually for packages not exactly the instance name, but generally safer to leave it as it is >+ return { type => 'driver:File;id:MD5', directory => "$dir/cgisess_$instance" }; > } > } > > sub get_session { > my $sessionID = shift; > my $params = _get_session_params(); >- return CGI::Session->new( $params->{dsn}, $sessionID, $params->{dsn_args} ); >+ my $session = Data::Session->new( %$params ) || die $Data::Session::errstr; >+ return $session; > } > > >diff --git a/C4/InstallAuth.pm b/C4/InstallAuth.pm >index c3f5adcaa13..db73f2cef3e 100644 >--- a/C4/InstallAuth.pm >+++ b/C4/InstallAuth.pm >@@ -19,7 +19,7 @@ package C4::InstallAuth; > > use Modern::Perl; > use Digest::MD5 qw(md5_base64); >-use CGI::Session; >+use Data::Session; > use File::Spec; > > require Exporter; >@@ -240,11 +240,13 @@ sub checkauth { > my %info; > my ( $userid, $cookie, $sessionID, $flags, $envcookie ); > my $logout = $query->param('logout.x'); >+ my $dir = File::Spec->tmpdir; > if ( $sessionID = $query->cookie("CGISESSID") ) { > C4::Context->_new_userenv($sessionID); >- my $session = >- CGI::Session->new( "driver:File", $sessionID, >- { Directory => $sessdir } ); >+ my $session = Data::Session->new( >+ type => 'driver:File;id:MD5', >+ directory => $dir >+ ) or die $Data::Session::errstr; > if ( $session->param('cardnumber') ) { > C4::Context->set_userenv( > $session->param('number'), >@@ -282,8 +284,10 @@ sub checkauth { > } > } > unless ($userid) { >- my $session = >- CGI::Session->new( "driver:File", undef, { Directory => $sessdir } ); >+ my $session = Data::Session->new( >+ type => 'driver:File;id:MD5', >+ directory => $dir >+ ) or die $Data::Session::errstr; > $sessionID = $session->id; > $userid = $query->param('userid'); > C4::Context->_new_userenv($sessionID); >@@ -325,7 +329,7 @@ sub checkauth { > $session->param( 'flags', 1 ); > $session->param( 'emailaddress', > C4::Context->preference('KohaAdminEmailAddress') ); >- $session->param( 'ip', $session->remote_addr() ); >+ $session->param( 'ip', $ENV{REMOTE_ADDR} ); > $session->param( 'lasttime', time() ); > $userid = C4::Context->config('user'); > } >diff --git a/circ/circulation.pl b/circ/circulation.pl >index f6dd318f971..2da81ccf919 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -41,7 +41,7 @@ use MARC::Record; > use C4::Reserves; > use Koha::Holds; > use C4::Context; >-use CGI::Session; >+use Data::Session; > use Koha::AuthorisedValues; > use Koha::CsvProfiles; > use Koha::Patrons; >diff --git a/cpanfile b/cpanfile >index 6e13f049320..1ac3b62f945 100644 >--- a/cpanfile >+++ b/cpanfile >@@ -9,7 +9,6 @@ requires 'CGI', '3.15'; > requires 'CGI::Carp', '1.29'; > requires 'CGI::Compile', '>= 0.17, != 0.24'; > requires 'CGI::Emulate::PSGI', '0.20'; >-requires 'CGI::Session', '4.2'; > requires 'CPAN::Meta', '2.150006'; > requires 'Cache::Memcached', '1.30'; > requires 'Cache::Memcached::Fast::Safe', '0.06'; >@@ -25,6 +24,9 @@ requires 'DBIx::Class::Schema::Loader', '0.07039'; > requires 'DBIx::RunSQL', '0.14'; > requires 'Data::Dumper', '2.121'; > requires 'Data::ICal', '0.13'; >+requires 'Data::Session', '1.18'; >+requires 'Data::Session::Driver::Memcached', '1.18'; >+requires 'Data::Session::Driver::mysql', '1.18'; > requires 'Date::Calc', '5.4'; > requires 'Date::Manip', '5.44'; > requires 'DateTime', '0.58'; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref >index a858b1d0da5..7f3d071efb0 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref >@@ -105,7 +105,6 @@ Administration: > default: mysql > choices: > mysql: MySQL database >- Pg: PostgreSQL database (not supported) > tmp: Temporary files > memcached: Memcached server > - >diff --git a/members/routing-lists.pl b/members/routing-lists.pl >index dee5845a0dd..d72f53e74f7 100755 >--- a/members/routing-lists.pl >+++ b/members/routing-lists.pl >@@ -25,7 +25,6 @@ use C4::Members; > use C4::Context; > use C4::Serials; > use Koha::Patrons; >-use CGI::Session; > > my $query = CGI->new; > >diff --git a/tools/background-job-progress.pl b/tools/background-job-progress.pl >index 625389f9029..b9d48f2f3df 100755 >--- a/tools/background-job-progress.pl >+++ b/tools/background-job-progress.pl >@@ -22,7 +22,6 @@ use Modern::Perl; > # standard or CPAN modules used > use IO::File; > use CGI qw ( -utf8 ); >-use CGI::Session; > use C4::Context; > use C4::Auth qw/check_cookie_auth/; > use C4::BackgroundJob; >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17427
:
56185
|
56504
|
56572
|
57067
|
60355
|
121337
|
121633
|
121657
|
121658
|
121659
|
150569
|
150570
|
150571
|
150603
|
150604
|
150605
|
150606
|
150633
|
150634
|
150635
|
150636
|
150637
|
150638
|
150639
|
150926
|
150927
|
150928
|
150929
|
150930
|
150931
|
153030
|
153031
|
153032
|
153033
|
153034
|
153035
|
153036
|
153037
|
153038
|
153039
|
153040