From: Dan Scott <dan@coffeecode.net>
Date: Tue, 3 May 2011 20:48:48 +0000 (-0400)
Subject: Ensure ampersands in URLs are &amp; per HTML spec
X-Git-Tag: sprint4-merge-nov22~5128^2~72^2~60^2~1^2~4
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=c88c0ac5f43a408bca0fe337991517edc3aef2c1;p=working%2FEvergreen.git

Ensure ampersands in URLs are &amp; per HTML spec

The propagator variable defined in parts/header.tt2 holds the value
of the CGI query string - which, as it comes off the wire, is a simple
"&". The problem with this is perhaps best explained at
http://www.htmlhelp.com/tools/validator/problems.html#amp (and it
explains why "&copy_..." gets converted into a copyright symbol).

By passing the incoming query string through the TT url filter and
then replacing "&" with "&amp;" we avoid this problem at the source.
From there, we have to address the other locations in the code in
which we are manually appending GET variables.

Signed-off-by: Dan Scott <dan@coffeecode.net>
---

diff --git a/Open-ILS/web/templates/default/opac/parts/header.tt2 b/Open-ILS/web/templates/default/opac/parts/header.tt2
index 08c5ca69e7..4c68368f24 100644
--- a/Open-ILS/web/templates/default/opac/parts/header.tt2
+++ b/Open-ILS/web/templates/default/opac/parts/header.tt2
@@ -17,7 +17,8 @@
         replace(';x=\d+','') | replace(';y=\d+','') | replace(';page=\d*', '') |
         replace(';', '&');
 
-    propagator = '?' _ query_string;
+
+    propagator = '?' _ query_string | url | replace('&', '&amp;');
 
     is_advanced = CGI.param("_adv").size;
 %]
diff --git a/Open-ILS/web/templates/default/opac/parts/record/summary.tt2 b/Open-ILS/web/templates/default/opac/parts/record/summary.tt2
index 26e1056c67..c34cf9a15f 100644
--- a/Open-ILS/web/templates/default/opac/parts/record/summary.tt2
+++ b/Open-ILS/web/templates/default/opac/parts/record/summary.tt2
@@ -30,8 +30,8 @@
                             <span class='opac-auto-030'>[% l("Author") %]:</span>
                             <em><a title='[% l("Perform an author search") %]'
                                     id='rdetail_author'
-                                    href="[% ctx.opac_root %]/results?qtype=author&query=[%-
-                                        attrs.author | replace('[,\.:;]', '') | uri %]&loc=[% CGI.param('loc') | uri %]">[% attrs.author %]</a></em>
+                                    href="[% ctx.opac_root %]/results?qtype=author&amp;query=[%-
+                                        attrs.author | replace('[,\.:;]', '') | uri %]&amp;loc=[% CGI.param('loc') | uri %]">[% attrs.author %]</a></em>
                             [% END %]
                         </td>
                         <td align="right" valign="top" nowrap="nowrap" style="white-space:nowrap;">
@@ -39,7 +39,7 @@
                                 <div style="float:right;">
                                     <div class="rdetail_aux_utils opac-auto-010">
                                         <a href="[% ctx.opac_root %]/place_hold[%-
-                                            propagator; propagator.length > 1 ? "&" : ""; %]hold_target=[% ctx.bre_id %]&hold_type=T" id="rdetail_place_hold" class="no-dec"><img
+                                            propagator; propagator.length > 1 ? "&amp;" : ""; %]hold_target=[% ctx.bre_id %]&amp;hold_type=T" id="rdetail_place_hold" class="no-dec"><img
                                             src="[% ctx.media_prefix %]/images/green_check.png" alt="[% l('place hold') %]" /><span 
                                                 style="position:relative;top:-3px;left:3px;">[% l('Place Hold') %]</span></a>
                                     </div>
@@ -165,16 +165,14 @@
             new_offset = ctx.copy_offset - ctx.copy_limit;
             IF new_offset < 0; new_offset = 0; END %]
             <td>
-                [%# For some reason, browsers render &copy (as in, &copy_limit=foo) as the copyright 
-                    symbol, even though it's inside inside the href attribute string.  Use ; instead %]
-                <a href="[% ctx.opac_root %]/record/[% ctx.bre_id %]?copy_offset=[% new_offset %];copy_limit=[% ctx.copy_limit %]">&laquo; [%
+                <a href="[% ctx.opac_root %]/record/[% ctx.bre_id %]?copy_offset=[% new_offset %]&amp;copy_limit=[% ctx.copy_limit %]">&laquo; [%
                     l('Previous [_1]', ctx.copy_offset - new_offset)
                 %]</a>
             </td>
         [% END %]
         [% IF ctx.copies.size >= ctx.copy_limit %]
             <td>
-                <a href="[% ctx.opac_root %]/record/[% ctx.bre_id %]?copy_offset=[% ctx.copy_offset + ctx.copy_limit %];copy_limit=[% ctx.copy_limit %]">[%
+                <a href="[% ctx.opac_root %]/record/[% ctx.bre_id %]?copy_offset=[% ctx.copy_offset + ctx.copy_limit %]&amp;copy_limit=[% ctx.copy_limit %]">[%
                     l('Next [_1]', ctx.copy_limit)
                 %] &raquo;</a>
             </td>
@@ -221,7 +219,7 @@
                                 </div>
                             </td>
                             <td width="1" valign="top" align="right" style="white-space:nowrap;">
-                                <a href="[% ctx.opac_root %]/place_hold[% propagator; propagator.length > 1 ? "&" : ""; %]hold_target=[% ctx.bre_id %]&hold_type=T"><img alt="[% l('Place Hold') %]"
+                                <a href="[% ctx.opac_root %]/place_hold[% propagator; propagator.length > 1 ? "&amp;" : ""; %]hold_target=[% ctx.bre_id %]&amp;hold_type=T"><img alt="[% l('Place Hold') %]"
                                     src="[% ctx.media_prefix %]/images/place_hold.gif" /></a>
                                 <a href="#" id="rd_reviews_and_more" target="_blank"><img
                                     alt="[% l('Reviews and More') %]" src="[% ctx.media_prefix %]/images/reviews.gif" /></a>