Summary: | Quotation marks are wrongly escaped in several places | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Cataloging | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | major | ||
Priority: | P5 - low | CC: | apirak, b.pastern4k, eb, fridolin.somers, kelly, lucas, m.de.rooy, magnus, martin.renvoize, mtj, nick |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22316 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22395 |
||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 21986: Do not escape quotation marks when cataloguing
image of the problem Bug 21986: Do not escape quotation marks when cataloguing Bug 21986: Do not escape quotation marks when cataloguing |
Description
Jonathan Druart
2018-12-12 14:04:10 UTC
Created attachment 83120 [details] [review] Bug 21986: Do not escape quotation marks when cataloguing In several places we escape quotation marks using $value =~ s/"/"/g; All the occurrences are wrong and must be removed. Most of them are leftover of bug 11638 (Remove HTML from addbiblio.pl), which removes the construction of html from pl scripts. The problem has been highlighted by bug 13618, I did not track down why the issue did not exist before (?) Test plan: 0/ Use strings with quotation marks, like: 'Fiddle tune history : "bad" tunes' You can also use other html characters to make the tests more complete, like 'Fiddle tune history : <"bad" tunes>' 1/ authorities/authorities.pl a. Edit an authority filling different fields with quotation marks b. Edit it again => The display (inputs' values) is wrong, if you save the escaped quotes will be inserted 2/ cataloguing/addbiblio.pl Same editing a bibliographic record 3/ cataloguing/additem.pl Same editing items 4/ members/memberentry.pl Edit a patron's record and fill some fields with quotation marks + fields borrowernotes and opacnotes => The quotes are inserted directly in DB (escape is done before the insert!) 5/ opac/opac-review.pl For QA only: $js_ok_review is never used 6/ tools/batchMod.pl For QA only: $value is always undefined at that point Created attachment 83143 [details]
image of the problem
(In reply to Elaine Bradtke from comment #2) > Created attachment 83143 [details] > image of the problem Hi Elaine, do you still have this problem with the patch applied? Created attachment 83179 [details] [review] Bug 21986: Do not escape quotation marks when cataloguing In several places we escape quotation marks using $value =~ s/"/"/g; All the occurrences are wrong and must be removed. Most of them are leftover of bug 11638 (Remove HTML from addbiblio.pl), which removes the construction of html from pl scripts. The problem has been highlighted by bug 13618, I did not track down why the issue did not exist before (?) Test plan: 0/ Use strings with quotation marks, like: 'Fiddle tune history : "bad" tunes' You can also use other html characters to make the tests more complete, like 'Fiddle tune history : <"bad" tunes>' 1/ authorities/authorities.pl a. Edit an authority filling different fields with quotation marks b. Edit it again => The display (inputs' values) is wrong, if you save the escaped quotes will be inserted 2/ cataloguing/addbiblio.pl Same editing a bibliographic record 3/ cataloguing/additem.pl Same editing items 4/ members/memberentry.pl Edit a patron's record and fill some fields with quotation marks + fields borrowernotes and opacnotes => The quotes are inserted directly in DB (escape is done before the insert!) 5/ opac/opac-review.pl For QA only: $js_ok_review is never used 6/ tools/batchMod.pl For QA only: $value is always undefined at that point Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I cannot replicate the issue after applying the patches.. Signing Off QA: Looking here > The problem has been highlighted by bug 13618, I did not track down why
> the issue did not exist before (?)
If you pass a&b in a template var thru the html filter, it will run the ampersand conversion once again. Resulting in: a&amp;b
So we need to choose where to escape.
(In reply to Martin Renvoize from comment #4) > For QA only: $js_ok_review is never used > 6/ tools/batchMod.pl > For QA only: $value is always undefined at that point Thx. Just seeing these remarks by accident. They were quite hidden.. (In reply to Marcel de Rooy from comment #7) > > The problem has been highlighted by bug 13618, I did not track down why > > the issue did not exist before (?) > > If you pass a&b in a template var thru the html filter, it will run the > ampersand conversion once again. Resulting in: a&amp;b > > So we need to choose where to escape. Should be quotation. But same thing applies. Created attachment 83195 [details] [review] Bug 21986: Do not escape quotation marks when cataloguing In several places we escape quotation marks using $value =~ s/"/"/g; All the occurrences are wrong and must be removed. Most of them are leftover of bug 11638 (Remove HTML from addbiblio.pl), which removes the construction of html from pl scripts. The problem has been highlighted by bug 13618, I did not track down why the issue did not exist before (?) Test plan: 0/ Use strings with quotation marks, like: 'Fiddle tune history : "bad" tunes' You can also use other html characters to make the tests more complete, like 'Fiddle tune history : <"bad" tunes>' 1/ authorities/authorities.pl a. Edit an authority filling different fields with quotation marks b. Edit it again => The display (inputs' values) is wrong, if you save the escaped quotes will be inserted 2/ cataloguing/addbiblio.pl Same editing a bibliographic record 3/ cataloguing/additem.pl Same editing items 4/ members/memberentry.pl Edit a patron's record and fill some fields with quotation marks + fields borrowernotes and opacnotes => The quotes are inserted directly in DB (escape is done before the insert!) 5/ opac/opac-review.pl For QA only: $js_ok_review is never used 6/ tools/batchMod.pl For QA only: $value is always undefined at that point Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Awesome work all! Pushed to master for 19.05 Pushed to 18.11.x for 18.11.01 backported to 18.05 for 18.05.08 Pushed to 17.11.x for 17.11.14 Bug 22316 is similar to this, but only manifests when you try to sort the patron search result table. Tested with 18.11.02. On version Koha 18.05.08, still experiencing issues with this same thing. When editing a record, or Edit as New - a title with quotation marks will be removed. Filing a new bug: 22395 Lucas and Fridolin, take a look at bug 22395, it seems that this patch should not have been backported into your versions. (In reply to Jonathan Druart from comment #17) > Lucas and Fridolin, take a look at bug 22395, it seems that this patch > should not have been backported into your versions. Oh indeed I see the problem. Reverted from 17.11.x, will be removed in 17.11.16. reverted in 18.05.x and will be removed in 18.05.10 *** Bug 22367 has been marked as a duplicate of this bug. *** *** Bug 22769 has been marked as a duplicate of this bug. *** |