From c3624cf8ef2c36b5e23a12f4267d19d65c40c3f7 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 18 Aug 2022 14:32:46 +0000 Subject: [PATCH] Bug 31250: (QA follow-up) The future will be easier Content-Type: text/plain; charset=utf-8 As requested by Jonathan, we need more flexibility ;) Here it comes. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy --- Koha/CookieManager.pm | 20 +++++++++++++++----- debian/templates/koha-conf-site.xml.in | 3 ++- etc/koha-conf.xml | 3 ++- t/CookieManager.t | 23 ++++++++++++++++++----- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/Koha/CookieManager.pm b/Koha/CookieManager.pm index 0c665efc61..b3e383d5b7 100644 --- a/Koha/CookieManager.pm +++ b/Koha/CookieManager.pm @@ -101,11 +101,7 @@ sub clear_unless { } next if !$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} ) { + if( $self->_should_be_cleared($name) ) { next if $seen->{$name}; push @rv, CGI::Cookie->new( # -expires explicitly omitted to create shortlived 'session' cookie @@ -122,6 +118,20 @@ sub clear_unless { return \@rv; } +sub _should_be_cleared { # when it is not on the deny list in koha-conf + my ( $self, $name ) = @_; + + return if $self->{_remove_unless}->{$name}; # exact match + + # Now try the entries as regex + foreach my $k ( keys %{$self->{_remove_unless}} ) { + my $reg = $self->{_remove_unless}->{$k}; + # The entry in koha-conf is always considered to be from the start + return if $name =~ qr/^${k}/; + } + return 1; +} + =head2 replace_in_list $list2 = $mgr->replace_in_list( $list1, $cookie ); diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in index bf77b0839f..bcf6bab0f6 100644 --- a/debian/templates/koha-conf-site.xml.in +++ b/debian/templates/koha-conf-site.xml.in @@ -457,9 +457,10 @@ __END_SRU_PUBLICSERVER__ __KEEP_COOKIE__ - catalogue_editor_ + catalogue_editor_\d+ diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml index dae6d96c39..793420437d 100644 --- a/etc/koha-conf.xml +++ b/etc/koha-conf.xml @@ -273,9 +273,10 @@ - catalogue_editor_ + catalogue_editor_\d+ diff --git a/t/CookieManager.t b/t/CookieManager.t index 0011e9f118..e9954799bd 100755 --- a/t/CookieManager.t +++ b/t/CookieManager.t @@ -41,7 +41,7 @@ subtest 'new' => sub { }; subtest 'clear_unless' => sub { - plan tests => 16; + plan tests => 17; t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'aap', 'noot' ] ); @@ -74,8 +74,8 @@ subtest 'clear_unless' => sub { 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_' ] ); + # Test with numeric suffix (via regex) + t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'catalogue_editor_\d+' ] ); $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' ); @@ -85,8 +85,21 @@ subtest 'clear_unless' => sub { $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' ); + [ 'catalogue_editor_234', 'catalogue_editor_345' ], + 'Cookie3 and cookie4 should have been cleared' ); + + # Test with another regex (yes, highly realistic examples :) + t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'next_\w+_number\d{2}_(now|never).*' ] ); + $cmgr = Koha::CookieManager->new; + my $cookie5; + $cookie1 = $q->cookie( -name => 'next_mynewword_number99_neverrr', -value => '1', -expires => '+1y' ); #fine + $cookie2 = $q->cookie( -name => 'prefixed_next_mynewword_number99_never_better_123', -value => '1', -expires => '+1y' ); # wrong prefix + $cookie3 = $q->cookie( -name => 'next_mynew-word_number99_never_better_123', -value => '1', -expires => '+1y' ); # wrong: hyphen in word + $cookie4 = $q->cookie( -name => 'mynewword_number999_never_better_123', -value => '1', -expires => '+1y' ); # wrong: three digits + $cookie5 = $q->cookie( -name => 'next_mynewword_number99_nooit_meer', -value => '1', -expires => '+1y' ); # wrong: nooit + @rv = @{$cmgr->clear_unless( $cookie1, $cookie2, $cookie3, $cookie4, $cookie5 )}; + is_deeply( [ map { $_->value ? $_->name : () } @rv ], [ 'next_mynewword_number99_neverrr' ], 'Only cookie1' ); + }; subtest 'replace_in_list' => sub { -- 2.20.1