Summary: | Improper escaping of quotes during z39.50 queries leads to broken html | ||
---|---|---|---|
Product: | Koha | Reporter: | William Hurley <bhurley> |
Component: | Z39.50 / SRU / OpenSearch Servers | Assignee: | Owen Leonard <oleonard> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P5 - low | CC: | chris, paul.poulain |
Version: | 3.8 | ||
Hardware: | All | ||
OS: | Linux | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Test of proposed patch
Bug 8171 - Improper escaping of quotes during z39.50 queries leads to broken html Bug 8171 - Improper escaping of quotes during z39.50 queries leads to broken html Bug 8171 - Improper escaping of quotes during z39.50 queries leads to broken html |
Description
William Hurley
2012-05-31 18:18:49 UTC
Hi William On line 228 term is the name of the column. It is not the variable. The variable is in $sth->execute($query, $type, $serverlist, $requestid); The query is escaped by use of placeholders ie the (?,?,?,?) Which means $query is escaped and replaces the first ? in that list. So that part is not the problem, I suspect the actual problem is that the "" are not escaped when doing the actual search. Ill leave this open because it is a valid but, bug I don't think your solution will work (or addresses the problem :)) If you want to read up about placeholders please look here http://search.cpan.org/dist/DBI/DBI.pm#Placeholders_and_Bind_Values You may be correct that I have not identified the source of the bug. You are incorrect when you say that the problem is that the quotation marks are not escaped during the actual search. The problem is that the quotation marks are not escaped when one chooses edit-> replace record via z39.50. If the title contains double quotes, everything from the first set of quotes back disappears, and is not transferred into the search window. The quoted text never makes it into the actual search. It is easy to replicate this bug. Pick any title in your collection and edit marc field $245a by appending "test quotes" at the end of the title. Then click on z39.50 search. The words "test quote" do not appear in the pop-up search window. Therefore, whatever module controls this behavior is not properly escaping the quotes. If you would kindly identify this module I would be greatly appreciative. Thanks for your quick response and all your help in this matter. (In reply to comment #1) > Hi William > > On line 228 term is the name of the column. It is not the variable. > > The variable is in > $sth->execute($query, $type, $serverlist, $requestid); > > The query is escaped by use of placeholders ie the (?,?,?,?) > > Which means $query is escaped and replaces the first ? in that list. > > So that part is not the problem, I suspect the actual problem is that the "" > are not escaped when doing the actual search. > > Ill leave this open because it is a valid but, bug I don't think your > solution will work (or addresses the problem :)) > > If you want to read up about placeholders please look here > http://search.cpan.org/dist/DBI/DBI.pm#Placeholders_and_Bind_Values (In reply to comment #2) > You may be correct that I have not identified the source of the bug. You > are incorrect when you say that the problem is that the quotation marks are > not escaped during the actual search. The problem is that the quotation > marks are not escaped when one chooses edit-> replace record via z39.50. If > the title contains double quotes, everything from the first set of quotes > back disappears, and is not transferred into the search window. The quoted > text never makes it into the actual search. I was just guessing where the problem was, since I haven't tried to replicate. But I can guarantee that changing the column name in the sql, will have utterly no effect on value that is stored in that column. > > It is easy to replicate this bug. Pick any title in your collection and > edit marc field $245a by appending "test quotes" at the end of the title. > Then click on z39.50 search. The words "test quote" do not appear in the > pop-up search window. Therefore, whatever module controls this behavior is > not properly escaping the quotes. If you would kindly identify this module > I would be greatly appreciative. Thanks for your quick response and all > your help in this matter. > I would look into the perl code that is outputting it and/or the template. It is not being stored in the database incorrectly, the placeholders are escaping all bad characters. Check line 95 in koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/ <ol><li><label for="title">Title: </label><input type="text" id="title" name="title" value="[% title %]" /></li> My theory is if [% title %] starts with a " then you get value="" and there is the problem. So you could try adding [% title |html %] Ok cool. I will try this patch on a fresh install of the latest stable branch on my development box and see if the problem resolves it's self. Should be able to let you know by early next week, as I almost certainly won't finish that before the end of the work day here in NYC. (In reply to comment #4) > Check line 95 in koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/ > > <ol><li><label for="title">Title: </label><input type="text" id="title" > name="title" value="[% title %]" /></li> > > My theory is if [% title %] starts with a " then you get value="" and there > is the problem. > > So you could try adding [% title |html %] (In reply to comment #5) > Ok cool. I will try this patch on a fresh install of the latest stable > branch on my development box and see if the problem resolves it's self. > Should be able to let you know by early next week, as I almost certainly > won't finish that before the end of the work day here in NYC. That's ok, its already 8.30am on Saturday here anyway, if it does work you could make an actual patch and attach it to here, and set the bug to needs signoff Created attachment 9915 [details]
Test of proposed patch
I tested the patch suggested last week and it works as advertised. Please Q&A when possible. I used the most current development version as provided by git, on lubuntu 12.04. Thanks for all your help in this matter.
Created attachment 9916 [details] [review] Bug 8171 - Improper escaping of quotes during z39.50 queries leads to broken html Implementing fix as suggested by Chris Cormack: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8171#c4 Patch tested with a sandbox, by Paul Poulain demoing at KohaCon12 <paul.poulain@biblibre.com> Created attachment 9920 [details] [review] Bug 8171 - Improper escaping of quotes during z39.50 queries leads to broken html Implementing fix as suggested by Chris Cormack: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8171#c4 Signed-off-by: Paul Poulain demoing at KohaCon12 <paul.poulain@biblibre.com> switching back to need signoff, it was just for the KohaCon12 demo Created attachment 10094 [details] [review] Bug 8171 - Improper escaping of quotes during z39.50 queries leads to broken html Implementing fix as suggested by Chris Cormack: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8171#c4 Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> QA comment: tiny patch that fixes a problem using T::T features Pushed to 3.8.x, will be in 3.8.2 |