Attachment 140166 Details for Bug 31250 – Bug 31250: (QA follow-up) The future will be easier

[patch] Bug 31250: (QA follow-up) The future will be easier

Bug-31250-QA-follow-up-The-future-will-be-easier.patch (text/plain), 6.13 KB, created by Kyle M Hall (khall) on 2022-09-02 16:06:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2022-09-02 16:06:29 UTC

Size: 6.13 KB

patch

obsolete

>From 8be6c08d483b94c2f29df252bd42cd9cb5f1e5ea Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>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 <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> 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__
>  </message_broker>
> 
>  <do_not_remove_cookie>__KEEP_COOKIE__</do_not_remove_cookie>
>- <do_not_remove_cookie>catalogue_editor_</do_not_remove_cookie>
>+ <do_not_remove_cookie>catalogue_editor_\d+</do_not_remove_cookie>
>  
> 
>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 @@
>    <vhost></vhost>
>  </message_broker>
> 
>- <do_not_remove_cookie>catalogue_editor_</do_not_remove_cookie>
>+ <do_not_remove_cookie>catalogue_editor_\d+</do_not_remove_cookie>
>  
> 
>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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 31250: 138203 | 138204 | 138981 | 138982 | 139389 | 139571 | 140163 | 140164 | 140165 | 140166