Bug 14044 - Backslashes ( and possibly other characters ) break patron search
Summary: Backslashes ( and possibly other characters ) break patron search
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 13021
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-22 13:39 UTC by Kyle M Hall
Modified: 2016-06-21 21:38 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14044 - Backslashes ( and possibly other characters ) break patron search (3.92 KB, patch)
2015-04-22 13:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14044 - Backslashes ( and possibly other characters ) break patron search (3.99 KB, patch)
2015-04-22 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14044: Escape json strings for patron searches (6.18 KB, patch)
2015-04-27 09:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14044: Escape json strings for patron searches (6.75 KB, patch)
2015-04-29 07:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF]Bug 14044: Escape json strings for patron searches (6.81 KB, patch)
2015-04-29 15:57 UTC, Christopher Brannon
Details | Diff | Splinter Review
Bug 14044: Don't escape html characters (1.81 KB, patch)
2015-04-30 08:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 14044: Escape json strings for patron searches (6.88 KB, patch)
2015-05-03 20:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14044: Don't escape html characters (1.87 KB, patch)
2015-05-03 20:40 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-04-22 13:39:41 UTC
If a patron has a backslash character ( \ ) in a field displayed in the patron search, and that patron is part of a set of search results, those search results will never display. Instead, it will just sit at the "Processing..." message forever.
Comment 1 Kyle M Hall 2015-04-22 13:42:17 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-04-22 14:23:43 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-04-22 14:25:55 UTC
The bad news is that the issue is certainly also present on members/tables/guarantor_search.tt patroncards/tables/members_results.tt, serials/tables/members_results.tt and common/patron_search.tt

But some of the patron information is not displayed on them.
Comment 4 Katrin Fischer 2015-04-22 20:07:59 UTC
With the patch applied, there are "" appearing in the result table, even for empty fields.
Comment 5 Jonathan Druart 2015-04-27 09:59:25 UTC Comment hidden (obsolete)
Comment 6 Christopher Brannon 2015-04-27 21:09:51 UTC
Works in circulation.pl.  Problem still exists in member.pl and guarantor_search.pl.  Don't know about other areas.

Christopher
Comment 7 Jonathan Druart 2015-04-28 07:10:45 UTC
(In reply to Christopher Brannon from comment #6)
> Works in circulation.pl.  Problem still exists in member.pl and
> guarantor_search.pl.  Don't know about other areas.
> 
> Christopher

Hi Christopher,
What does not work? I have tried again it seems to work.
Note that this patch does not modify the circulation.pl
Comment 8 Christopher Brannon 2015-04-28 18:10:11 UTC
(In reply to Jonathan Druart from comment #7)

> Hi Christopher,
> What does not work? I have tried again it seems to work.
> Note that this patch does not modify the circulation.pl

Turns out the patch is not applying.  Please take a look at the patch when you can.

Christopher
Comment 9 Jonathan Druart 2015-04-29 07:51:43 UTC Comment hidden (obsolete)
Comment 10 Christopher Brannon 2015-04-29 15:57:10 UTC Comment hidden (obsolete)
Comment 11 Christopher Brannon 2015-04-29 16:10:44 UTC
Did the sign off, but had to figure out how to work around issues the special characters in your test plan created.  FYI.  Otherwise, looks good.  Works great.

Christopher
Comment 12 Katrin Fischer 2015-04-29 20:37:04 UTC
Sorry, I found another something - try adding a " to the address data - it will display as " on the patron result list.
Comment 13 Jonathan Druart 2015-04-30 08:06:58 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2015-04-30 08:09:16 UTC
(In reply to Katrin Fischer from comment #12)
> Sorry, I found another something - try adding a " to the address data - it
> will display as " on the patron result list.

Yes, either we escape html characters or not :)
It seems that there are not in other places, but in this case if a <td> or <tr> is found, it will make the table explodes.
But... who does put these tags or quote in an address field? :)
Comment 15 Katrin Fischer 2015-05-03 20:27:09 UTC
It literally showed up as &quot; in the result list - but retesting it now it is ok.
Comment 16 Katrin Fischer 2015-05-03 20:29:17 UTC
Created attachment 38790 [details] [review]
[PASSED QA] Bug 14044: Escape json strings for patron searches

In a couple of patron searches, the address is displayed.
If the address contain invalid json characters, DataTables won't manage
to parse the results and display them.
This patch makes the templates more readable by creating a BLOCK instead
of a long line and escape the invalid json characters calling the TT
plugin (To.json).

Test plan:
Edit a patron and add something like:
    patron <address> \123 ☎ ⚑
in the address field.

Search for this patron using the main patron search (members/member.pl)
and the guarantor search.
Confirm that a result with this patron is correctly displayed.

Regression test: Since Koha::Template::Plugin::To::json is modified,
confirm that the add users to basket search (on acqui/basket.pl?basketno=X
click on the "Add user" button) still works as expected.

Signed-off-by: Christopher Brannon <cbrannon@cdalibary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2015-05-03 20:40:17 UTC
Created attachment 38791 [details] [review]
[PASSED QA] Bug 14044: Don't escape html characters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Tomás Cohen Arazi 2015-05-06 13:42:52 UTC
Patches pushed to master.

Thanks Jonathan!
Comment 19 Liz Rea 2015-07-07 09:00:21 UTC
Feeling like this might be a candidate for 3.18, so making note to myself to check it.

-Liz
Comment 20 Liz Rea 2015-07-07 09:33:17 UTC
This is a problem all the way back into 3.18, and the patches won't come over cleanly. Might we have a 3.18 version of this?

Cheers,
Liz
Comment 21 Liz Rea 2015-07-08 03:51:13 UTC
Actually I was wrong, it wasn't the slash, it was \r\n in the address fields that are doing it - so yeah, backslashes explode this in 3.18.
Comment 22 Liz Rea 2015-07-29 21:13:55 UTC
Mason did you mean to change the status on this bug?
Comment 23 Mason James 2015-07-30 01:09:11 UTC
(In reply to Liz Rea from comment #22)
> Mason did you mean to change the status on this bug?

hi, yes... is the new status not correct?
Comment 24 Liz Rea 2015-07-30 21:23:40 UTC
:) nope, it needs a patch for 3.18, but there isn't one yet. :)
Comment 25 Tomás Cohen Arazi 2015-07-30 21:50:12 UTC
(In reply to Mason James from comment #23)
> (In reply to Liz Rea from comment #22)
> > Mason did you mean to change the status on this bug?
> 
> hi, yes... is the new status not correct?

Chris didn't pick it for 3.20 yet. So it should remain 'Pushed to master' so he notices it. Well, or Liz could just tell him :-D
Comment 26 Liz Rea 2015-07-30 22:11:00 UTC
Hehe I don't tell Chris what to do. ^.^
Comment 27 Mason James 2015-07-30 23:50:56 UTC
(In reply to Tomás Cohen Arazi from comment #25)
> (In reply to Mason James from comment #23)
> > (In reply to Liz Rea from comment #22)
> > > Mason did you mean to change the status on this bug?
> > 
> > hi, yes... is the new status not correct?
> 
> Chris didn't pick it for 3.20 yet. 

both patches for this bug seem to be in the stable branch?

http://git.koha-community.org/gitweb/?p=koha.git;a=shortlog;h=refs/heads/3.20.x;pg=3

2015-05-06	Jonathan Druart	Bug 14044: Don't escape html characters
2015-05-06	Jonathan Druart	Bug 14044: Escape json strings for patron searches
Comment 28 Liz Rea 2015-07-31 00:34:06 UTC
This is in 3.20.1, looks like it was missed for a bug update though. Sorry Mason. :)
Comment 29 Fridolin SOMERS 2015-09-30 07:50:54 UTC
Depends on 13021 for TT plugin To