[Bug 17427] New: Replace CGI::Session with Data::Session
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Bug ID: 17427 Summary: Replace CGI::Session with Data::Session Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56185&action=edit 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 This is just a POC and will only works with SessionStorage set to MySQL. Please test it and tell me if you find bug. If it looks bug free I will submit a follow-up to cover the other combinations of SessionStorage (temporary storage and Memcached) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion Assignee|gmcharlt@gmail.com |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |benjamin.rokseth@kul.oslo.k | |ommune.no --- Comment #2 from Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> --- This is great detective work, and it makes real sense to deprecate a deprecated session handler! I have tested with api and finally tests with autocommit set to null actually rolls back, so I no longer am left with a skewed db. please bring this on and I will happily test and sign off! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56185|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56504 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56504&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Benjamin Rokseth from comment #2)
This is great detective work, and it makes real sense to deprecate a deprecated session handler! I have tested with api and finally tests with autocommit set to null actually rolls back, so I no longer am left with a skewed db.
please bring this on and I will happily test and sign off!
It's your turn to play ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I should note: The first patch did not work as expected, it defaulted to File (instead of mysql). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56504|0 |1 is obsolete| | --- Comment #6 from Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> --- Created attachment 56572 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56572&action=edit 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. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #7 from Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> --- Works perfectly! Corrected a small typo in the PerlDependencies. No sessions lost in tempfiles, mysql or memcached, and they survived a plack restart. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 56572 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56572 Bug 17427: Replace CGI::Session with Data::Session Review of attachment 56572: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17427&attachment=56572) ----------------------------------------------------------------- Generally looks good.. minor question about using ENV though.. not sure about that part of the change. ::: C4/Auth.pm @@ +1111,4 @@
$session->param( 'branchname', $branchname ); $session->param( 'flags', $userflags ); $session->param( 'emailaddress', $emailaddress ); + $session->param( 'ip', $ENV{REMOTE_ADDR} );
Did we test this against plack? Can we really rely on ENV for remote_addr... and in fact.. do we not compare the session ip to the env remote_addr in places as a security check? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #8)
Comment on attachment 56572 [details] [review] Bug 17427: Replace CGI::Session with Data::Session
Review of attachment 56572 [details] [review]: -----------------------------------------------------------------
Generally looks good.. minor question about using ENV though.. not sure about that part of the change.
::: C4/Auth.pm @@ +1111,4 @@
$session->param( 'branchname', $branchname ); $session->param( 'flags', $userflags ); $session->param( 'emailaddress', $emailaddress ); + $session->param( 'ip', $ENV{REMOTE_ADDR} );
Did we test this against plack? Can we really rely on ENV for remote_addr... and in fact.. do we not compare the session ip to the env remote_addr in places as a security check?
$ENV{REMOTE_ADDR} is correctly filled on each request by the ReverseProxy middleware as far as I recall (I debugged plack+ENV several days before making the packages plack integration official). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |dependency Status|Signed Off |Patch doesn't apply --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Please rebase. I'd say this one depends only on Mirko's answer about packaging Data::Session. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mirko@abunchofthings.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56572|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57067 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57067&action=edit 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. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I do not recommend to push this patch into the next stable release (at least not at the beginning). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57067|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 60355 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60355&action=edit 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. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Just a rebase... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Benjamin Rokseth from comment #7)
Works perfectly! Corrected a small typo in the PerlDependencies.
No sessions lost in tempfiles, mysql or memcached, and they survived a plack restart.
Benjamin: What browsers did you test? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This seems to break scripts like stage-marc-import and tools/upload.pl. I am still testing a bit but after successfully uploading one file, I am not able to upload anymore. Also saw this error: stage-marc-import.pl: YAML::Tiny does not support C4::BackgroundJob references at /usr/local/share/perl/5.20.2/Data/Session/Serialize/YAML.pm line 18., Suspect that all scripts using BackgroungJob have issues now. Note that these jobs append information to the session. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Data::Session creates a CGI object. And we do not like that in scripts like upload-file.pl that call check_cookie_auth (calling get_session). The created CGI object in this case makes the upload disappear. Other scripts may have issues too going the similar cookie_auth way.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #17)
This seems to break scripts like stage-marc-import and tools/upload.pl. I am still testing a bit but after successfully uploading one file, I am not able to upload anymore. Also saw this error: stage-marc-import.pl: YAML::Tiny does not support C4::BackgroundJob references at /usr/local/share/perl/5.20.2/Data/Session/Serialize/YAML.pm line 18.,
That can be fixed using DataDumper or FreezeThaw instead of YAML as serializer. (In reply to Marcel de Rooy from comment #18)
Data::Session creates a CGI object. And we do not like that in scripts like upload-file.pl that call check_cookie_auth (calling get_session). The created CGI object in this case makes the upload disappear. Other scripts may have issues too going the similar cookie_auth way..
I do not know how to fix that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18976 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28317 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28158 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #60355|0 |1 is obsolete| | --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121337 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121337&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (patch rebased) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #22 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #21)
(patch rebased)
Should this be "Needs Signoff" now? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #22)
(In reply to Jonathan Druart from comment #21)
(patch rebased)
Should this be "Needs Signoff" now?
I rebased it to see if it fixed bug 28158, it doesn't. This patch can be tested, but I haven't much so far. In my opinion that's still one we need, maybe a good candidate for early 21.11 adoption? Feel free to test it! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28507 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #24 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121633 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121633&action=edit Bug 17427: Use JSON serialization -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #25 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- We are now aiming to provide a long-term solution for the encoding issue found on bug 28489. Using Data::Session using the JSON serializer seems to be the more robust solution. However there is still an issue if SessionStorage=tmp file Note that sessions.a_session must be longtext. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #26 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #25)
However there is still an issue if SessionStorage=tmp file
root@kohadevbox$ more /var/lib/koha/kohadev/tmp/cgisess_koha_kohadev/cgisess_6967e16ccb3dddff1c32735b7930e967 {"_SESSION_CTIME":1622821220,"_SESSION_ETIME":0,"id":"koha","interface":"intranet","number":51,"ip":"172.20.0.1","lasttime":"1622821226","_SESSION_PTIME":{},"_SESSION_ATIME":1622821226,"flags":1,"surname":"koha" ,"cardnumber":"42","_SESSION_ID":"6967e16ccb3dddff1c32735b7930e967","branchname":"✔️ ❤️ ★","branch":"✔️❤️★"} root@kohadevbox$ file /var/lib/koha/kohadev/tmp/cgisess_koha_kohadev/cgisess_6967e16ccb3dddff1c32735b7930e967 /var/lib/koha/kohadev/tmp/cgisess_koha_kohadev/cgisess_6967e16ccb3dddff1c32735b7930e967: JSON data -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #27 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Bug 28517 might be related to this? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28489 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28489 [Bug 28489] CGI::Session is incorrectly serialized to DB in production env / when strict_sql_modes = 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #28 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- JSON serialization does not work (when SessionStorage=tmp), selenium tests from bug 28489 are failing. With longblob: # Failed test 'logged-in-branch-name set - SessionStorage=mysql, branchname=Test2❤️' # at t/db_dependent/selenium/regressions.t line 306. # got: 'Test2â¤ï¸Â' # expected: 'Test2❤️' With longtext: # Failed test 'logged-in-branch-name set - SessionStorage=tmp, branchname=Test2❤️' # at t/db_dependent/selenium/regressions.t line 306. # got: 'Test2â¤ï¸' # expected: 'Test2❤️' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #29 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- And it's not possible to add a serializer that is not in the same dir as others, because of this code in Data::Session: 318 sub get_my_serializers 319 { 320 my($self) = @_; 321 my($path) = $self -> _get_pm_path('Serialize'); 322 323 # Warning: Use sort map{} read_dir, not map{} sort read_dir. But, why? 324 325 my(@serializer) = sort map{s/.pm//; $_} read_dir($path); 326 327 ($#serializer < 0) && die __PACKAGE__ . '. No serializers available'; 328 329 ($self -> verbose > 1) && $self -> log('Serializers: ' . join(', ', @serializer) ); 330 331 $self -> my_serializers(\@serializer); 332 333 } # End of get_my_serializers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #30 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #28) This comment was confusing, with JSON serialization we have: With longtext it's working with SessionStorage=mysql With longblob it's failing with mysql. But it does not work with SessionStorage=tmp. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121337|0 |1 is obsolete| | Attachment #121633|0 |1 is obsolete| | --- Comment #31 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121657&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #32 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121658&action=edit Bug 17427: Use JSON serialization -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #33 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121659 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121659&action=edit Bug 17427: Use our own YAML::XS serializer -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #34 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- And, FWIW, if I move the YAML::XS serializer (see last patch) in the Data/Session/Serialize directory, all tests pass (longblob). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #35 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #34)
And, FWIW, if I move the YAML::XS serializer (see last patch) in the Data/Session/Serialize directory, all tests pass (longblob).
Does this have an unlisted dependency on Bug 28519? I'm just curious since the serializers are in a "lib" directory. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|28489 |28519 --- Comment #36 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Yes David. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28489 [Bug 28489] CGI::Session is incorrectly serialized to DB in production env / when strict_sql_modes = 0 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28519 [Bug 28519] Add a 2nd directory for Perl modules -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_21_11_candidate, | |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |mtj@kohaaloha.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #37 from Mason James <mtj@kohaaloha.com> --- hi joubu i've uploaded a libdata-session-perl pkg to koha-staging/dev, for testing root@kohadevbox:/kohadevbox# apt install libdata-session-perl Reading package lists... Done Building dependency tree Reading state information... Done The following additional packages will be installed: libautovivification-perl libdbd-sqlite3-perl libdbix-admin-createtable-perl libfile-slurper-perl libhash-fieldhash-perl libperlio-utf8-strict-perl The following NEW packages will be installed: libautovivification-perl libdata-session-perl libdbd-sqlite3-perl libdbix-admin-createtable-perl libfile-slurper-perl libhash-fieldhash-perl libperlio-utf8-strict-perl 0 upgraded, 7 newly installed, 0 to remove and 2 not upgraded. Need to get 376 kB of archives. root@kohadevbox:/kohadevbox# apt-cache policy libdata-session-perl libdata-session-perl: Installed: 1.18-1 Candidate: 1.18-1 Version table: *** 1.18-1 500 500 http://debian.koha-community.org/koha-staging dev/main amd64 Packages 100 /var/lib/dpkg/status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #38 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #34)
And, FWIW, if I move the YAML::XS serializer (see last patch) in the Data/Session/Serialize directory, all tests pass (longblob).
Wait this works despite https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427#c29 ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #39 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #38)
(In reply to Jonathan Druart from comment #34)
And, FWIW, if I move the YAML::XS serializer (see last patch) in the Data/Session/Serialize directory, all tests pass (longblob).
Wait this works despite https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427#c29 ?
Oh maybe I misunderstand what you're saying... you meant in the vendor perl not the Koha perl lib... right? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #40 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #39)
(In reply to David Cook from comment #38)
(In reply to Jonathan Druart from comment #34)
And, FWIW, if I move the YAML::XS serializer (see last patch) in the Data/Session/Serialize directory, all tests pass (longblob).
Wait this works despite https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427#c29 ?
Oh maybe I misunderstand what you're saying... you meant in the vendor perl not the Koha perl lib... right?
Yes, I copied it to /path/Data/Session/Serialize/yamlxs.pm where /path/Data/Session/ is `pmpath Data::Session`. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #41 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #40)
Oh maybe I misunderstand what you're saying... you meant in the vendor perl not the Koha perl lib... right?
Yes, I copied it to /path/Data/Session/Serialize/yamlxs.pm where /path/Data/Session/ is `pmpath Data::Session`.
But that's not a real solution for production. Shouldn't "Needs Signoff" be changed back to 'Assigned' or "In Discussion"? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion --- Comment #42 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Ok, let's discuss then. What do we do? I am ok with having our module where Data::Session is expecting it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #43 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #42)
Ok, let's discuss then. What do we do?
I am ok with having our module where Data::Session is expecting it.
How do you propose to do that in production though? The only reasonable option I see would be creating our Data::Session::Serializer::yamlxs as a standalone library (with its own git repository), and then packaging that and storing it in the Koha Community APT repo, and then adding it as a Koha dependency. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_21_11_candidate, | |RM_priority | Assignee|jonathan.druart+koha@gmail. |koha-bugs@lists.koha-commun |com |ity.org -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #44 from Tomás Cohen Arazi <tomascohen@gmail.com> --- What about https://metacpan.org/pod/MojoX::Session -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29942 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #45 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #44)
What about https://metacpan.org/pod/MojoX::Session
It hasn't been updated in over 7 years and the developer says he doesn't want to develop it anymore (although he said he was open to pull requests): https://github.com/vti/mojox-session/issues/19 But then there's been a pull request sitting for over 8 years too https://github.com/vti/mojox-session/pull/14 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #46 from David Cook <dcook@prosentient.com.au> --- Marcel suggests writing Koha::Session at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29942#c2 Maybe we should just do that. Jonathan opened this issue nearly 6 years ago and we still haven't resolved it... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #47 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #43)
(In reply to Jonathan Druart from comment #42)
Ok, let's discuss then. What do we do?
I am ok with having our module where Data::Session is expecting it.
How do you propose to do that in production though?
The only reasonable option I see would be creating our Data::Session::Serializer::yamlxs as a standalone library (with its own git repository), and then packaging that and storing it in the Koha Community APT repo, and then adding it as a Koha dependency.
I don't understand. Why isn't this ok for production? It's what we do already for CGI::Session -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121657|0 |1 is obsolete| | Attachment #121658|0 |1 is obsolete| | Attachment #121659|0 |1 is obsolete| | --- Comment #48 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150569 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150569&action=edit 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. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #49 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150570 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150570&action=edit Bug 17427: Use JSON serialization -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #50 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150571 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150571&action=edit Bug 17427: Use our own YAML::XS serializer -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #51 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Patches rebased, status back to NSO. Why isn't this ready? What am I missing? Everything is working correctly in my tests. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29942 | CC| |andreas.jonsson@kreablo.se --- Comment #52 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 29942 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #53 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This needs more work to be ready for push: * Adjust deps (cpanfile) * `git grep CGI::Session` occurrences to be adjusted * Remove FIXMEs from api tests But please test and tell me if it's worth spending time to continue here. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #54 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #53)
This needs more work to be ready for push: * Adjust deps (cpanfile) * `git grep CGI::Session` occurrences to be adjusted * Remove FIXMEs from api tests
But please test and tell me if it's worth spending time to continue here.
I seems worthwhile to me if only because CGI::Session is unmaintained! -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #55 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #53)
This needs more work to be ready for push: * Adjust deps (cpanfile) * `git grep CGI::Session` occurrences to be adjusted * Remove FIXMEs from api tests
But please test and tell me if it's worth spending time to continue here.
+1 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_23_11_candidate -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #56 from Kyle M Hall <kyle@bywatersolutions.com> --- I'm getting the error "Data::Session. Unknown serialize 'yamlxs' at /usr/local/share/perl/5.32.1/Data/Session.pm line 752". I installed Data::Session, are there other deps I missed? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #57 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Kyle M Hall from comment #56)
I'm getting the error "Data::Session. Unknown serialize 'yamlxs' at /usr/local/share/perl/5.32.1/Data/Session.pm line 752".
I installed Data::Session, are there other deps I missed?
Yes, you need to copy the lib file cp lib/Data/Session/Serialize/yamlxs.pm /usr/share/perl5/Data/Session/Serialize/yamlxs.pm -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #58 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Can we (In reply to Jonathan Druart from comment #57)
(In reply to Kyle M Hall from comment #56)
I'm getting the error "Data::Session. Unknown serialize 'yamlxs' at /usr/local/share/perl/5.32.1/Data/Session.pm line 752".
I installed Data::Session, are there other deps I missed?
Yes, you need to copy the lib file cp lib/Data/Session/Serialize/yamlxs.pm /usr/share/perl5/Data/Session/Serialize/yamlxs.pm
Can we ship Data::Session inside the Koha project instead, and get rid of the need for that? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #59 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- No, please read the previous comments. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #60 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #59)
No, please read the previous comments.
Well, yes, we could, if we embed Data::Session and modify it. But what would we do that? Isn't the "lib trick" already in place anyway? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #61 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #59)
No, please read the previous comments.
Well, yes, we could, if we embed Data::Session and modify it. But why would we do that? Isn't the "lib trick" already in place anyway? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150569|0 |1 is obsolete| | Attachment #150570|0 |1 is obsolete| | Attachment #150571|0 |1 is obsolete| | --- Comment #62 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 150603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150603&action=edit 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. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #63 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 150604 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150604&action=edit Bug 17427: Use JSON serialization Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #64 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 150605 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150605&action=edit Bug 17427: Use our own YAML::XS serializer Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #65 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 150606 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150606&action=edit Bug 17427: Remove use lines for CGI::Session from Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #66 from Kyle M Hall <kyle@bywatersolutions.com> --- The big blocker is replacing the use of CGI::Session::Find (https://metacpan.org/pod/CGI::Session#find(-%5C&code-)) in C4::Auth_with_cas -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #67 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #61)
(In reply to Jonathan Druart from comment #59)
No, please read the previous comments.
Well, yes, we could, if we embed Data::Session and modify it. But why would we do that?
Isn't the "lib trick" already in place anyway?
I am wrong. The existing "lib trick" is copying our 'lib' dir content to /usr/share/koha/lib but we need yamlxs.pm in /usr/share/perl5/Data/Session/Serialize/ -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED Assignee|koha-bugs@lists.koha-commun |jonathan.druart+koha@gmail. |ity.org |com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #68 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150633 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150633&action=edit Bug 17427: Remove more occurrences of CGI::Session -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #69 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150634&action=edit Bug 17427: Remove SessionStorage mock from tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #70 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150635 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150635&action=edit Bug 17427: Embed Data::Session Test plan: Install libdata-session-perl and make sure our lib has priority over the installed version -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #71 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150636&action=edit Bug 17427: Remove confusion between session id and param id FIXME Certainly more work needed here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #72 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150637 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150637&action=edit Bug 17427: HACKY - force new session Look at Data::Session->new, it's calling $self -> load_session; Then look at load_session then user_id => we are retrieving the ID from the cookie But in our Auth.t tests we are mocking CGI->cookie to always return the previous CGISESSID We should certainly adjust the test here, and remove this patch. 194 # Note: We can test return values from checkauth here since we mocked the safe_exit after the Redirect 303 195 is( $return[0], $patron2->userid, 'Login of patron2 approved' ); 196 isnt( $return[2], $sessionID, 'Did not return previous session ID' ); 197 ok( $return[2], 'New session ID not empty' ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #73 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150638 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150638&action=edit Bug 17427: Retrieve sessionID from HTTP_COOKIE To make some tests pass (t/db_dependent/Auth.t) and mimick what did CGI::Session, but that is certainly useless and the tests should be adjusted. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #74 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150639&action=edit Bug 17427: Fix id vs userid in tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #75 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- still 2 tests failing /kohadevbox/koha/t/db_dependent/Auth.t .. 12/17 # Failed test at /kohadevbox/koha/t/db_dependent/Auth.t line 840. # got: 'anon' # expected: 'expired' # Failed test at /kohadevbox/koha/t/db_dependent/Auth.t line 841. # got: 'Data::Session=HASH(0x564c72143498)' # expected: undef # Looks like you failed 2 tests of 31. /kohadevbox/koha/t/db_dependent/Auth.t .. 15/17 # Failed test 'checkauth & check_cookie_auth' # at /kohadevbox/koha/t/db_dependent/Auth.t line 909. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #76 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Kyle M Hall from comment #66)
The big blocker is replacing the use of CGI::Session::Find (https://metacpan.org/pod/CGI::Session#find(-%5C&code-)) in C4::Auth_with_cas
Yes, that's a tricky one. And really ugly. First the method is experimental, and then we are looping over all the sessions to retrieve the corresponding one... outch! We could eventually generate the session ID from the CAS ticket (using Koha::Encryption for instance?). Would that work? I am not familiar with CAS! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW Assignee|jonathan.druart+koha@gmail. |koha-bugs@lists.koha-commun |com |ity.org --- Comment #77 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I am done for now, please help (review, feedback, test, see last commit messages)! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #78 from Kyle M Hall <kyle@bywatersolutions.com> --- After applying the patches and running reset_all in ktd, I get the following error when I try to log in: Data::Session::Driver::mysql::store(): DBI Exception: DBD::mysql::db do failed: Data too long for column 'id' at row 1 at /kohadevbox/koha/lib/Data/Session.pm line 262 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150634|0 |1 is obsolete| | Attachment #150635|0 |1 is obsolete| | Attachment #150636|0 |1 is obsolete| | Attachment #150637|0 |1 is obsolete| | Attachment #150638|0 |1 is obsolete| | Attachment #150639|0 |1 is obsolete| | --- Comment #79 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150926 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150926&action=edit Bug 17427: Remove SessionStorage mock from tests -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #80 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150927&action=edit Bug 17427: Embed Data::Session Test plan: Install libdata-session-perl and make sure our lib has priority over the installed version -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #81 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150928&action=edit Bug 17427: Remove confusion between session id and param id FIXME Certainly more work needed here -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #82 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150929&action=edit Bug 17427: HACKY - force new session Look at Data::Session->new, it's calling $self -> load_session; Then look at load_session then user_id => we are retrieving the ID from the cookie But in our Auth.t tests we are mocking CGI->cookie to always return the previous CGISESSID We should certainly adjust the test here, and remove this patch. 194 # Note: We can test return values from checkauth here since we mocked the safe_exit after the Redirect 303 195 is( $return[0], $patron2->userid, 'Login of patron2 approved' ); 196 isnt( $return[2], $sessionID, 'Did not return previous session ID' ); 197 ok( $return[2], 'New session ID not empty' ); -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #83 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150930&action=edit Bug 17427: Retrieve sessionID from HTTP_COOKIE To make some tests pass (t/db_dependent/Auth.t) and mimick what did CGI::Session, but that is certainly useless and the tests should be adjusted. Commit modified, added ';' see comment 78 - if ( $http_cookie && $http_cookie =~ m{CGISESSID=([^,:]*)} ) { + if ( $http_cookie && $http_cookie =~ m{CGISESSID=([^,:;]*)} ) { sessionID was "a035bf508448b8b3ecd1ca23609b90b3; bib_list=; KohaOpacLanguage=; JWT= " -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #84 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150931&action=edit Bug 17427: Fix id vs userid in tests -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #85 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Kyle M Hall from comment #78)
After applying the patches and running reset_all in ktd, I get the following error when I try to log in:
Data::Session::Driver::mysql::store(): DBI Exception: DBD::mysql::db do failed: Data too long for column 'id' at row 1 at /kohadevbox/koha/lib/Data/Session.pm line 262 at /usr/share/perl5/DBIx/Class/Exception.pm line 77
Fixed in "Retrieve sessionID from HTTP_COOKIE" Patches rebased against master. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #86 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #73)
Created attachment 150638 [details] [review] Bug 17427: Retrieve sessionID from HTTP_COOKIE
To make some tests pass (t/db_dependent/Auth.t) and mimick what did CGI::Session, but that is certainly useless and the tests should be adjusted.
This patch fixes some tests but make the sessionID handling very wrong. We always end up with the same sessionID (even after logout, change user, etc.) I think we should revert it and remove the related tests. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|dependency | --- Comment #87 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Mason James from comment #37)
hi joubu i've uploaded a libdata-session-perl pkg to koha-staging/dev, for testing
If we are going with the approach we have now we won't need it. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #88 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #80)
Created attachment 150927 [details] [review] Bug 17427: Embed Data::Session
Test plan: Install libdata-session-perl and make sure our lib has priority over the installed version
An alternative could be to contribute Data::Session::Serialize::yamlxs. Ron seems pretty responsive on Github, and it looks like Mason has contributed a couple patches in Feb 2023. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #89 from David Cook <dcook@prosentient.com.au> --- What's outstanding on this one? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #90 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #89)
What's outstanding on this one?
Read the commit messages. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #91 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #90)
(In reply to David Cook from comment #89)
What's outstanding on this one?
Read the commit messages.
I did and I couldn't make sense of them. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #92 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #91)
(In reply to Jonathan Druart from comment #90)
(In reply to David Cook from comment #89)
What's outstanding on this one?
Read the commit messages.
I did and I couldn't make sense of them.
Ok, trying to be more verbose then. 1. For discussion, waiting for review and feedback: (In reply to Jonathan Druart from comment #82)
Created attachment 150929 [details] [review] Bug 17427: HACKY - force new session
Look at Data::Session->new, it's calling $self -> load_session; Then look at load_session then user_id => we are retrieving the ID from the cookie
But in our Auth.t tests we are mocking CGI->cookie to always return the previous CGISESSID
We should certainly adjust the test here, and remove this patch.
194 # Note: We can test return values from checkauth here since we mocked the safe_exit after the Redirect 303 195 is( $return[0], $patron2->userid, 'Login of patron2 approved' ); 196 isnt( $return[2], $sessionID, 'Did not return previous session ID' ); 197 ok( $return[2], 'New session ID not empty' );
What do you think about this, should we adjust the test? 2. The behaviour is buggy, see the following comment: (In reply to Jonathan Druart from comment #86)
(In reply to Jonathan Druart from comment #73)
Created attachment 150638 [details] [review] [review] Bug 17427: Retrieve sessionID from HTTP_COOKIE
To make some tests pass (t/db_dependent/Auth.t) and mimick what did CGI::Session, but that is certainly useless and the tests should be adjusted.
This patch fixes some tests but make the sessionID handling very wrong. We always end up with the same sessionID (even after logout, change user, etc.)
I think we should revert it and remove the related tests.
Do you agree that we should remove the related tests? 3. This needs to be fully tested, but first we need to fix the failures and bugs highlighted in 1 and 2. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #93 from David Cook <dcook@prosentient.com.au> --- Thanks, Jonathan. I appreciate that. I think I'll have to look at the code as it's running, but it won't apply at the moment. I have some holidays this week and next, so I won't be around heaps, but if you get that rebased then I can take a look. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150603|0 |1 is obsolete| | Attachment #150604|0 |1 is obsolete| | Attachment #150605|0 |1 is obsolete| | Attachment #150606|0 |1 is obsolete| | Attachment #150633|0 |1 is obsolete| | Attachment #150926|0 |1 is obsolete| | Attachment #150927|0 |1 is obsolete| | Attachment #150928|0 |1 is obsolete| | Attachment #150929|0 |1 is obsolete| | Attachment #150930|0 |1 is obsolete| | Attachment #150931|0 |1 is obsolete| | --- Comment #94 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153030 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153030&action=edit 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. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #95 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153031 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153031&action=edit Bug 17427: Use JSON serialization Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #96 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153032 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153032&action=edit Bug 17427: Use our own YAML::XS serializer Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #97 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153033 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153033&action=edit Bug 17427: Remove use lines for CGI::Session from Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #98 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153034 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153034&action=edit Bug 17427: Remove more occurrences of CGI::Session -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #99 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153035 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153035&action=edit Bug 17427: Remove SessionStorage mock from tests -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #100 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153036 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153036&action=edit Bug 17427: Embed Data::Session Test plan: Install libdata-session-perl and make sure our lib has priority over the installed version -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #101 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153037&action=edit Bug 17427: Remove confusion between session id and param id FIXME Certainly more work needed here -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #102 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153038&action=edit Bug 17427: HACKY - force new session Look at Data::Session->new, it's calling $self -> load_session; Then look at load_session then user_id => we are retrieving the ID from the cookie But in our Auth.t tests we are mocking CGI->cookie to always return the previous CGISESSID We should certainly adjust the test here, and remove this patch. 194 # Note: We can test return values from checkauth here since we mocked the safe_exit after the Redirect 303 195 is( $return[0], $patron2->userid, 'Login of patron2 approved' ); 196 isnt( $return[2], $sessionID, 'Did not return previous session ID' ); 197 ok( $return[2], 'New session ID not empty' ); -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #103 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153039&action=edit Bug 17427: Retrieve sessionID from HTTP_COOKIE To make some tests pass (t/db_dependent/Auth.t) and mimick what did CGI::Session, but that is certainly useless and the tests should be adjusted. Commit modified, added ';' see comment 78 - if ( $http_cookie && $http_cookie =~ m{CGISESSID=([^,:]*)} ) { + if ( $http_cookie && $http_cookie =~ m{CGISESSID=([^,:;]*)} ) { sessionID was "a035bf508448b8b3ecd1ca23609b90b3; bib_list=; KohaOpacLanguage=; JWT= " -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #104 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153040&action=edit Bug 17427: Fix id vs userid in tests -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #105 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #76)
(In reply to Kyle M Hall from comment #66)
The big blocker is replacing the use of CGI::Session::Find (https://metacpan.org/pod/CGI::Session#find(-%5C&code-)) in C4::Auth_with_cas
Yes, that's a tricky one. And really ugly. First the method is experimental, and then we are looping over all the sessions to retrieve the corresponding one... outch!
We could eventually generate the session ID from the CAS ticket (using Koha::Encryption for instance?). Would that work? I am not familiar with CAS!
And I forgot this one, it is a blocker! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #106 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Hum, I had to install the following packages: apt install libautovivification-perl libhash-fieldhash-perl libfile-slurper-perl -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 --- Comment #107 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #106)
Hum, I had to install the following packages: apt install libautovivification-perl libhash-fieldhash-perl libfile-slurper-perl
Those are libdata-session-perl deps. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36098 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org