Bug 2600 - HTML tags in titles not escaped in many places
Summary: HTML tags in titles not escaped in many places
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P3 critical (vote)
Assignee: Owen Leonard
QA Contact:
URL: circ/returns.pl
Keywords:
Depends on: 3652
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-11 06:22 UTC by Galen Charlton
Modified: 2019-06-27 09:24 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
An illustration of a potential "exploit" (15.99 KB, image/png)
2009-12-16 14:05 UTC, Chris Cormack
Details
Some fixes (32.75 KB, patch)
2010-02-03 18:49 UTC, Chris Cormack
Details | Diff | Splinter Review
Some fixes (77.24 KB, patch)
2010-02-03 20:20 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:53:19 UTC


---- Reported by gmcharlt@gmail.com 2008-09-11 06:22:47 ----

If a bib has a title containing HTML tags (e.g., "My <strong>bold</strong> statement", the HTML is not escaped in the list of returned items in circ/returns.pl.



---- Additional Comments From gmcharlt@gmail.com 2008-09-11 06:24:25 ----

Same problem applies to list items recently checked out (circ/circulation.pl).  Problem does *not* occur in the loaned items list in memberdetails.pl.



---- Additional Comments From oleonard@myacpl.org 2008-09-15 10:16:52 ----

I submitted a patch with a LOT of changes that should go a long way towards addressing this bug. I looked for instances of <!-- TMPL_VAR NAME="title" --> and escaped where appropriate.



---- Additional Comments From gmcharlt@gmail.com 2008-09-26 07:29:57 ----

Patch pushed to 3.1 master.



---- Additional Comments From rch@liblime.com 2008-10-17 13:20:14 ----


This update conflicts with search term highlighting in cataloging search.

Titles now appear e.g. like:
<span class="term">Poetry</span> for <span class="term">students</span>, v. 2:  
in search results.

There may be other search results in other modules that are so affected; I haven't grepped.



---- Additional Comments From oleonard@myacpl.org 2008-10-18 08:27:51 ----

Ryan, I think you're referring to addbooks.tmpl? I submitted patch that covers that problem on 10/14: "[PATCH] Two template bug-fixes: removing debugging output from opac-detail.tmpl, and turning off html-escaping in addbooks.tmpl to prevent incorrect HTML output. Need to figure out how to allow HTML tags in titles /and/ do search term highlighting." Just waiting for approval.



---- Additional Comments From joe.atzberger@liblime.com 2009-05-20 16:29:33 ----

I recommend search term highlighting be moved to an entirely jquery-based model, so that we can maintain consistent expectations about HTML escaping of TMPL_VAR's.  And of course this has already been done by others.

Examples:
http://weblogtoolscollection.com/archives/2009/04/10/how-to-highlight-search-terms-with-jquery/

http://devthought.com/blog/client-side/2009/04/javascript-regexp-based-highlighting-function-for-mootools-and-jquery/

So basically:

jQuery.fn.extend({ 
	highlight: function(search, insensitive, klass){
		var regex = new RegExp('(<[^>]*>)|(\\b'+ search.replace(/([-.*+?^${}()|[\]\/\\])/g,"\\$1") +')', insensitive ? 'ig' : 'g');
		return this.html(this.html().replace(regex, function(a, b, c){
			return (a.charAt(0) == '<') ? a : '<strong class="'+ klass +'">' + c + '</strong>'; 
		}));
	}
 
});



---- Additional Comments From chris@bigballofwax.co.nz 2009-12-15 23:03:04 ----

Owen my man, what's the status of this one?



---- Additional Comments From oleonard@myacpl.org 2009-12-16 14:05:39 ----

Created an attachment
An illustration of a potential "exploit"

I only regret my screenshot is not animated.



---- Additional Comments From oleonard@myacpl.org 2009-12-16 15:36:32 ----

Found so far:

- catalogue/results.tmpl
- catalogue/MARCdetail.tmpl
- catalogue/ISBDdetail.tmpl
- addbookbybiblionumber.tmpl
- reserve/request.tmpl (title in hidden form field)
- opac-results.tmpl
- opac-MARCdetail.tmpl
- opac-ISBDdetail.tmpl
- opac-addbybiblionumber.tmpl
- opac-readingrecord.tmpl
- opac-search-history.tmpl
- opac-showmarc.tmpl



---- Additional Comments From chris@bigballofwax.co.nz 2009-12-16 21:52:14 ----

Ill help out with this if you like Owen.
You do the opac templates .. ill do intranet?



---- Additional Comments From oleonard@myacpl.org 2010-02-03 18:49:28 ----

Created an attachment
Some fixes





---- Additional Comments From oleonard@myacpl.org 2010-02-03 18:55:15 ----

opac-showmarc.tmpl
catalogue/showmarc.tmpl
catalogue/ISBDdetail.tmpl
opac-ISBDdetail.tmpl

These templates output content via single variables containing *all* content ("<!-- TMPL_VAR name="ISBD" -->"), so this issue will have to be resolved on the script side.




---- Additional Comments From oleonard@myacpl.org 2010-02-03 20:20:46 ----

Created an attachment
Some fixes





---- Additional Comments From gmcharlt@gmail.com 2010-02-03 21:57:39 ----

Pushed patches to HEAD.  What's left?



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:53 UTC  ---

This bug was previously known as _bug_ 2600 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2600
Imported an attachment (id=754)
Imported an attachment (id=755)
Imported an attachment (id=756)

Actual time not defined. Setting to 0.0
The original submitter of attachment 754 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The original submitter of attachment 755 [details] [review] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The original submitter of attachment 756 [details] [review] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.

Comment 1 Jared Camins-Esakov 2012-02-12 20:10:12 UTC
Seems to have been pushed years ago.