Bug 11515 - Encoding problem with OpacBrowseResults

When OpacBrowseResults syspref is on, the detail page contains a results browser.
When search terms contains non-ASCII character, the search query is not well encoded in this browser.

This patch adds the URI-coding to search terms into session to avoid any encoding problem with diacritics and URI specific characters like ?,& ...
So that TT parameter 'busc' is already URI encoded and can be used to recreate seach URL.

Test plan :
- Set OpacBrowseResults on
- At OPAC, perform a search with a diacritique. For example 'déjà'
- Go to detail page of a result
=> You see browser under "Browser results"
- Click on "Back to results"
=> You get same results and same search term with correct encoding

Signed-off-by: Broust <jean-manuel.broust@univ-lyon2.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Bug still present on master. I note that latin characters are taken by ISO-8859-1 encoding
(Perl's default) and that's why real UTF8 ones don't break (Perl notices they are UTF-8).

koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt

@@ -1055,7 +1055,8 @@
                                     <span>Previous</span>
                                 [% END %]
                             </li>
-                            <li class="back_results"><a href="opac-search.pl?[% busc |url %]" title="Back to the results search list">Back to results</a></li>
+                            [%# busc is already URI encoded %]
+                            <li class="back_results"><a href="opac-search.pl?[% busc %]" title="Back to the results search list">Back to results</a></li>
                             <li class="right_results">
                                 [% IF ( nextBiblionumber ) %]
                                     <a href="opac-detail.pl?biblionumber=[% nextBiblionumber %][% IF ( query_desc && OpacHighlightedWords ) %]&query_desc=[% query_desc |uri %][% END %]" title="See: [% IF ( nextTitle ) %][% nextTitle |html %][% ELSE %]next biblio[% END %]">Next &raquo;</a>

opac/opac-detail.pl

@@ -154,8 +154,10 @@ my $session = get_session($query->cookie("CGISESSID"));
 my %paging = (previous => {}, next => {});
 if ($session->param('busc')) {
     use C4::Search;
+    use URI::Escape;
 
     # Rebuild the string to store on session
+    # param value is URI encoded and params separator is HTML encode (&)
     sub rebuildBuscParam
     {
         my $arrParamsBusc = shift;
@@ -166,13 +168,13 @@ if ($session->param('busc')) {
             if ($_ =~ /^(?:query|listBiblios|newlistBiblios|query_type|simple_query|total|offset|offsetSearch|next|previous|count|expand|scan)/) {
                 if (defined($arrParamsBusc->{$_})) {
                     $pasarParams .= '&' if ($j);
-                    $pasarParams .= $_ . '=' . $arrParamsBusc->{$_};
+                    $pasarParams .= $_ . '=' . uri_escape( $arrParamsBusc->{$_} );
                     $j++;
                 }
             } else {
                 for my $value (@{$arrParamsBusc->{$_}}) {
                     $pasarParams .= '&' if ($j);
-                    $pasarParams .= $_ . '=' . $value;
+                    $pasarParams .= $_ . '=' . uri_escape($value);
                     $j++;
                 }
             }
@@ -234,12 +236,12 @@ if ($session->param('busc')) {
     for (@arrBusc) {
         ($key, $value) = split(/=/, $_, 2);
         if ($key =~ /^(?:query|listBiblios|newlistBiblios|query_type|simple_query|next|previous|total|offset|offsetSearch|count|expand|scan)/) {
-            $arrParamsBusc{$key} = $value;
+            $arrParamsBusc{$key} = uri_unescape($value);
         } else {
             unless (exists($arrParamsBusc{$key})) {
                 $arrParamsBusc{$key} = [];
             }
-            push @{$arrParamsBusc{$key}}, $value;
+            push @{$arrParamsBusc{$key}}, uri_unescape($value);
         }
     }
     my $searchAgain = 0;
@@ -312,7 +314,7 @@ if ($session->param('busc')) {
     for (@arrBusc) {
         unless ($_ =~ /^(?:query|listBiblios|newlistBiblios|query_type|simple_query|next|previous|total|count|offsetSearch)/) {
             $buscParam .= '&' unless ($j == 0);
-            $buscParam .= $_;
+            $buscParam .= $_; # string already URI encoded
             $j++;
         }
     }

opac/opac-search.pl

@@ -349,13 +349,14 @@ $tag = $params->{tag} if $params->{tag};
 
 
 # String with params with the search criteria for the paging in opac-detail
+# param value is URI encoded and params separator is HTML encode (&)
 my $pasarParams = '';
 my $j = 0;
 for (keys %$params) {
     my @pasarParam = $cgi->param($_);
     for my $paramValue(@pasarParam) {
         $pasarParams .= '&' if ($j > 0);
-        $pasarParams .= $_ . '=' . $paramValue;
+        $pasarParams .= $_ . '=' . uri_escape($paramValue);
         $j++;
     }
 }
@@ -542,10 +543,10 @@ if ($tag) {
         ($error, $results_hashref, $facets) = C4::Search::pazGetRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,$branches,$query_type,$scan);
     };
 } else {
-    $pasarParams .= '&query=' . $query;
-    $pasarParams .= '&count=' . $results_per_page;
-    $pasarParams .= '&simple_query=' . $simple_query;
-    $pasarParams .= '&query_type=' . $query_type if ($query_type);
+    $pasarParams .= '&query=' . uri_escape($query);
+    $pasarParams .= '&count=' . uri_escape($results_per_page);
+    $pasarParams .= '&simple_query=' . uri_escape($simple_query);
+    $pasarParams .= '&query_type=' . uri_escape($query_type) if ($query_type);
     eval {
         ($error, $results_hashref, $facets) = getRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,$branches,$itemtypes,$query_type,$scan,1);
     };
@@ -697,12 +698,12 @@ for (my $i=0;$i<@servers;$i++) {
                 my $j = 0;
                 foreach (@newresults) {
                     my $bibnum = ($_->{biblionumber})?$_->{biblionumber}:0;
-                    $pasarParams .= $bibnum . ',';
+                    $pasarParams .= uri_escape($bibnum) . ',';
                     $j++;
                     last if ($j == $results_per_page);
                 }
                 chop $pasarParams if ($pasarParams =~ /,$/);
-                $pasarParams .= '&amp;total=' . int($total) if ($pasarParams !~ /total=(?:[0-9]+)?/);
+                $pasarParams .= '&amp;total=' . uri_escape( int($total) ) if ($pasarParams !~ /total=(?:[0-9]+)?/);
                 if ($pasarParams) {
                     my $session = get_session($cgi->cookie("CGISESSID"));
                     $session->param('busc' => $pasarParams);