From d3f924350d0a2878ffd8135b13e470a9bf967312 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 28 Jul 2022 12:43:20 +0000 Subject: [PATCH] Bug 31250: Deny clearing cookies with numeric suffix Content-Type: text/plain; charset=utf-8 This change allows us to add catalogue_editor_ to the deny list in koha-conf.xml and keep cookies like catalogue_editor_123. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy --- Koha/CookieManager.pm | 6 +++++- t/CookieManager.t | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Koha/CookieManager.pm b/Koha/CookieManager.pm index 018c8068e1..0c665efc61 100644 --- a/Koha/CookieManager.pm +++ b/Koha/CookieManager.pm @@ -101,7 +101,11 @@ sub clear_unless { } next if !$name; - if( !$self->{_remove_unless}->{$name} ) { + # Try stripping _\d+ from name for cookiea like catalogue_editor_123 + my $stripped_name = $name; + $stripped_name =~ s/_\d+$/_/; + + if( !$self->{_remove_unless}->{$stripped_name} && !$self->{_remove_unless}->{$name} ) { next if $seen->{$name}; push @rv, CGI::Cookie->new( # -expires explicitly omitted to create shortlived 'session' cookie diff --git a/t/CookieManager.t b/t/CookieManager.t index 1efc80e37a..0011e9f118 100755 --- a/t/CookieManager.t +++ b/t/CookieManager.t @@ -41,7 +41,7 @@ subtest 'new' => sub { }; subtest 'clear_unless' => sub { - plan tests => 15; + plan tests => 16; t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'aap', 'noot' ] ); @@ -73,6 +73,20 @@ subtest 'clear_unless' => sub { is( $rv[4]->value, q{}, 'zus empty' ); is( $rv[1]->httponly, 0, 'cleared wim is not httponly' ); is( $rv[2]->httponly, 1, 'aap httponly' ); + + # Test with _123 prefix + t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'catalogue_editor_' ] ); + $cmgr = Koha::CookieManager->new; + $cookie1 = $q->cookie( -name => 'catalogue_editor_234', -value => '1', -expires => '+1y' ); + $cookie2 = $q->cookie( -name => 'catalogue_editor_345', -value => '1', -expires => '+1y' ); + $cookie3 = $q->cookie( -name => 'catalogue_editor_', -value => '1', -expires => '+1y' ); + $cookie4 = $q->cookie( -name => 'catalogue_editor', -value => '1', -expires => '+1y' ); + + $list = [ $cookie1, $cookie2, $cookie3, $cookie4 ]; + @rv = @{$cmgr->clear_unless( @$list )}; + is_deeply( [ map { $_->value ? $_->name : () } @rv ], + [ 'catalogue_editor_234', 'catalogue_editor_345', 'catalogue_editor_' ], + 'Only cookie4 should have been cleared' ); }; subtest 'replace_in_list' => sub { -- 2.20.1