diff --git a/C4/Auth.pm b/C4/Auth.pm index 492c8b7635..278f8eb995 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -46,7 +46,9 @@ BEGIN { $debug = $ENV{DEBUG}; @ISA = qw(Exporter); @EXPORT = qw(&checkauth &get_template_and_user &haspermission &get_user_subpermissions); - @EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &get_all_subpermissions &get_user_subpermissions); + @EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &get_all_subpermissions &get_user_subpermissions + ParseSearchHistoryCookie + ); %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] ); $ldap = C4::Context->config('useldapserver') || 0; $cas = C4::Context->preference('casAuthentication'); @@ -249,10 +251,7 @@ sub get_template_and_user { # And if there's a cookie with searches performed when the user was not logged in, # we add them to the logged-in search history - my $searchcookie = $in->{'query'}->cookie('KohaOpacRecentSearches'); - if ($searchcookie){ - $searchcookie = uri_unescape($searchcookie); - my @recentSearches = @{decode_json($searchcookie) || []}; + my @recentSearches = ParseSearchHistoryCookie($in->{'query'}); if (@recentSearches) { my $sth = $dbh->prepare($SEARCH_HISTORY_INSERT_SQL); $sth->execute( $borrowernumber, @@ -274,7 +273,6 @@ sub get_template_and_user { } } } - } else { # if this is an anonymous session, setup to display public lists... $template->param( sessionID => $sessionID ); @@ -288,16 +286,11 @@ sub get_template_and_user { # Anonymous opac search history # If opac search history is enabled and at least one search has already been performed if (C4::Context->preference('EnableOpacSearchHistory')) { - my $searchcookie = $in->{'query'}->cookie('KohaOpacRecentSearches'); - if ($searchcookie){ - $searchcookie = uri_unescape($searchcookie); - my @recentSearches = @{decode_json($searchcookie) || []}; - # We show the link in opac + my @recentSearches = ParseSearchHistoryCookie($in->{'query'}); if (@recentSearches) { $template->param(ShowOpacRecentSearchLink => 1); } } - } if(C4::Context->preference('dateformat')){ if(C4::Context->preference('dateformat') eq "metric"){ @@ -1686,6 +1679,15 @@ sub getborrowernumber { return 0; } +sub ParseSearchHistoryCookie { + my $input = shift; + my $search_cookie = $input->cookie('KohaOpacRecentSearches'); + return () unless $search_cookie; + my $obj = eval { decode_json(uri_unescape($search_cookie)) }; + return () unless defined $obj; + return () unless ref $obj eq 'ARRAY'; + return @{ $obj }; +} END { } # module clean-up code here (global destructor) 1; diff --git a/opac/opac-search-history.pl b/opac/opac-search-history.pl index 9331e6e8b7..35ec54e66f 100755 --- a/opac/opac-search-history.pl +++ b/opac/opac-search-history.pl @@ -20,7 +20,7 @@ use strict; use warnings; -use C4::Auth qw(:DEFAULT get_session); +use C4::Auth qw(:DEFAULT get_session ParseSearchHistoryCookie); use CGI; use JSON qw/decode_json encode_json/; use C4::Context; @@ -67,10 +67,7 @@ if (!$loggedinuser) { # Showing search history } else { - # Getting the cookie - my $searchcookie = $cgi->cookie('KohaOpacRecentSearches'); - if ($searchcookie && decode_json(uri_unescape($searchcookie))) { - my @recentSearches = @{decode_json(uri_unescape($searchcookie))}; + my @recentSearches = ParseSearchHistoryCookie($cgi); if (@recentSearches) { # As the dates are stored as unix timestamps, let's do some formatting @@ -93,7 +90,6 @@ if (!$loggedinuser) { $template->param(recentSearches => \@recentSearches); } } - } } else { # And if the user is logged in, we deal with the database diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 6528e723c8..4daef5c81f 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -30,7 +30,7 @@ use warnings; ## load Koha modules use C4::Context; use C4::Output; -use C4::Auth qw(:DEFAULT get_session); +use C4::Auth qw(:DEFAULT get_session ParseSearchHistoryCookie); use C4::Languages qw(getAllLanguages); use C4::Search; use C4::Biblio; # GetBiblioData @@ -576,16 +576,7 @@ for (my $i=0;$i<@servers;$i++) { # Opac search history my $newsearchcookie; if (C4::Context->preference('EnableOpacSearchHistory')) { - my @recentSearches; - - # Getting the (maybe) already sent cookie - my $searchcookie = $cgi->cookie('KohaOpacRecentSearches'); - if ($searchcookie){ - $searchcookie = uri_unescape($searchcookie); - if (decode_json($searchcookie)) { - @recentSearches = @{decode_json($searchcookie)}; - } - } + my @recentSearches = ParseSearchHistoryCookie($cgi); # Adding the new search if needed if (!$borrowernumber || $borrowernumber eq '') { diff --git a/t/Auth_ParseSearchHistoryCookie.t b/t/Auth_ParseSearchHistoryCookie.t new file mode 100644 index 0000000000..507ac91b71 --- /dev/null +++ b/t/Auth_ParseSearchHistoryCookie.t @@ -0,0 +1,43 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use Test::More tests => 3; + +use_ok('C4::Auth', qw/ParseSearchHistoryCookie/); + +my $valid_cookie = "%5B%7B%22time%22%3A1374978877%2C%22query_cgi%22%3A%22idx%3D%26q%3Dhistory%26branch_group_limit%3D%22%2C%22total%22%3A2%2C%22query_desc%22%3A%22kw%2Cwrdl%3A%20history%2C%20%22%7D%5D"; +my $expected_recent_searches = [ + { + 'time' => 1374978877, + 'query_cgi' => 'idx=&q=history&branch_group_limit=', + 'total' => 2, + 'query_desc' => 'kw,wrdl: history, ' + } +]; + +my $input = CookieSimulator->new($valid_cookie); +my @recent_searches = ParseSearchHistoryCookie($input); +is_deeply(\@recent_searches, $expected_recent_searches, 'parsed valid search history cookie value'); + +# simulate bit of a Storable-based search history cookie +my $invalid_cookie = "%04%08%0812345"; +$input = CookieSimulator->new($invalid_cookie); +@recent_searches = ParseSearchHistoryCookie($input); +is_deeply(\@recent_searches, [], 'got back empty search history list if given invalid cookie'); + +package CookieSimulator; + +sub new { + my ($class, $str) = @_; + my $val = [ $str ]; + return bless $val, $class; +} + +sub cookie { + my $self = shift; + return $self->[0]; +} + +1;