From cffd36bcb47439d80401e52091f0d2808bc52cb6 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 As requested by Jonathan, we need more flexibility ;) Here it comes. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- Koha/CookieManager.pm | 21 +++++++++++++++----- debian/templates/koha-conf-site.xml.in | 3 ++- etc/koha-conf.xml | 3 ++- t/CookieManager.t | 27 +++++++++++++++++++------- 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/Koha/CookieManager.pm b/Koha/CookieManager.pm index 0c665efc61..07795ee4d8 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,21 @@ 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 should match the complete string + # So adding a ^ and $ + 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..3df60d24a5 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,19 +74,32 @@ 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' ); + $cookie1 = $q->cookie( -name => 'catalogue_editor_abc', -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' ); + $cookie4 = $q->cookie( -name => 'catalogue_editor_123x', -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' ); + [ 'catalogue_editor_345' ], + 'Cookie2 should be found only' ); + + # 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_never', -value => '1', -expires => '+1y' ); #fine + $cookie2 = $q->cookie( -name => 'prefixed_next_mynewword_number99_never', -value => '1', -expires => '+1y' ); # wrong prefix + $cookie3 = $q->cookie( -name => 'next_mynew-word_number99_never', -value => '1', -expires => '+1y' ); # wrong: hyphen in word + $cookie4 = $q->cookie( -name => 'mynewword_number999_never', -value => '1', -expires => '+1y' ); # wrong: three digits + $cookie5 = $q->cookie( -name => 'next_mynewword_number99_always', -value => '1', -expires => '+1y' ); # wrong: always + @rv = @{$cmgr->clear_unless( $cookie1, $cookie2, $cookie3, $cookie4, $cookie5 )}; + is_deeply( [ map { $_->value ? $_->name : () } @rv ], [ 'next_mynewword_number99_never' ], 'Only cookie1 matched' ); + }; subtest 'replace_in_list' => sub { -- 2.39.5