Bug 27748 - Encoding problem in link to OverDrive results
Summary: Encoding problem in link to OverDrive results
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Magnus Enger
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-22 14:31 UTC by Magnus Enger
Modified: 2021-12-13 21:11 UTC (History)
4 users (show)

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


Attachments
Bug 27748 - Encoding problem in link to OverDrive results (3.29 KB, patch)
2021-02-23 11:42 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 27748 - Encoding problem in link to OverDrive results (3.34 KB, patch)
2021-03-19 04:06 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 27748: Encoding problem in link to OverDrive results (3.40 KB, patch)
2021-04-03 22:14 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 Magnus Enger 2021-02-22 14:31:22 UTC
If you have OverDrive enabled and do a search in the OPAC, you get a link that says e.g. "Found 1 results in the library's OverDrive collection". But if you search contains non-ASCII characters, the search goes wrong. 

Example: 

Do a search for "göteborg":
https://kohaopac.alingsas.se/cgi-bin/koha/opac-search.pl?q=g%C3%B6teborg
(The URL shows the query as q=göteborg, but when I paste it here it is turned into q=g%C3%B6teborg.)

The link says:
Found 4 results in the library's OverDrive collection

The actual link is:
https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%F6teborg

If you foloow that link you get to this page:
https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%F6teborg
which says "OverDrive search for 'g�teborg'" and shows 53 results, because the query was cut off at the "ö", and a search was actually made for just the letter "g" (whatever comes before the first non-ascii-char).

If I change the encoding in the link from:
https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%F6teborg
to:
https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%C3%B6teborg
or 
https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=göteborg
the page shows 4 results, as promised.
Comment 1 Magnus Enger 2021-02-22 14:56:53 UTC
Tested on 20.05.05 and 20.05.08. 

I *think* the problem is the "escape" on line 934 of koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt:

$overdrive_results.html( '<a href="/cgi-bin/koha/opac-overdrive-search.pl?q=' + escape( od_querystring ) + '">' + MSG_RESULTS_FOUND_IN_COLLECTION.format(data.totalItems, 'OverDrive') + '</a>' );
Comment 2 Magnus Enger 2021-02-22 14:59:54 UTC
"Bug 26578: Clean overdrive searches before passing to OD" touches the relevant parts of the code, but as far as I can see, it does not fix the encoding problem.
Comment 3 Magnus Enger 2021-02-23 11:42:54 UTC
Created attachment 117194 [details] [review]
Bug 27748 - Encoding problem in link to OverDrive results

If you have OverDrive enabled and do a search for e.g. "Göteborg",
you will get a link above the regular Koha search results that
says e.g. "Found 1 results in the library's OverDrive collection".
If you follow this link, a search will be made in OverDrive, but only
for "G", because the search term is cut off at the first non-ASCII
char.

To test:
- Make sure OverDrive is enabled
- Do a search that contains a non-ASCII char, like "Göteborg"
- Click on the link "Found x results in the library's OverDrive
  collection"
- Verify the query string is cut off at the first non-ASCII char,
  and that the number of hits is different from the number given
  in the link on the previous page
- Apply the patch, restart all the things
- Repeat the search, and verify that the numbers match and that the
  query string is intact after clicking on the OverDrive link

Lookin at the code, the same problem seems to be shared between
OverDrive and RecordedBooks. I have applied the same fix to both,
but I do not have access to RecordedBooks, so I have not been able
to verify that the problem exists there and is fixed by my patch.
Comment 4 Magnus Enger 2021-02-23 11:53:48 UTC
Technical note:

It looks like the use of escape() has been deprecated in favour of encodeURI
and encodeURIComponent:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/escape

I have replaced two occurrences of escape with encodeURIComponent.
Comment 5 Phil Ringnalda 2021-03-19 04:06:32 UTC
Created attachment 118481 [details] [review]
Bug 27748 - Encoding problem in link to OverDrive results

If you have OverDrive enabled and do a search for e.g. "Göteborg",
you will get a link above the regular Koha search results that
says e.g. "Found 1 results in the library's OverDrive collection".
If you follow this link, a search will be made in OverDrive, but only
for "G", because the search term is cut off at the first non-ASCII
char.

To test:
- Make sure OverDrive is enabled
- Do a search that contains a non-ASCII char, like "Göteborg"
- Click on the link "Found x results in the library's OverDrive
  collection"
- Verify the query string is cut off at the first non-ASCII char,
  and that the number of hits is different from the number given
  in the link on the previous page
- Apply the patch, restart all the things
- Repeat the search, and verify that the numbers match and that the
  query string is intact after clicking on the OverDrive link

Lookin at the code, the same problem seems to be shared between
OverDrive and RecordedBooks. I have applied the same fix to both,
but I do not have access to RecordedBooks, so I have not been able
to verify that the problem exists there and is fixed by my patch.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 6 Katrin Fischer 2021-04-03 22:14:19 UTC
Created attachment 119166 [details] [review]
Bug 27748: Encoding problem in link to OverDrive results

If you have OverDrive enabled and do a search for e.g. "Göteborg",
you will get a link above the regular Koha search results that
says e.g. "Found 1 results in the library's OverDrive collection".
If you follow this link, a search will be made in OverDrive, but only
for "G", because the search term is cut off at the first non-ASCII
char.

To test:
- Make sure OverDrive is enabled
- Do a search that contains a non-ASCII char, like "Göteborg"
- Click on the link "Found x results in the library's OverDrive
  collection"
- Verify the query string is cut off at the first non-ASCII char,
  and that the number of hits is different from the number given
  in the link on the previous page
- Apply the patch, restart all the things
- Repeat the search, and verify that the numbers match and that the
  query string is intact after clicking on the OverDrive link

Lookin at the code, the same problem seems to be shared between
OverDrive and RecordedBooks. I have applied the same fix to both,
but I do not have access to RecordedBooks, so I have not been able
to verify that the problem exists there and is fixed by my patch.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2021-04-03 22:15:14 UTC
Trusting that Magnus and Phil have tested this with access to Overdrive. Change makes sense and I will move this to PQA with a code review only.
Comment 8 Phil Ringnalda 2021-04-06 00:11:02 UTC
Correct: I tested with Overdrive, and Magnus' link from comment 0 to show the problem no longer shows the problem, because he is running with his patch :)
Comment 9 Jonathan Druart 2021-04-06 13:57:49 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 10 Fridolin Somers 2021-04-15 11:04:59 UTC
Pushed to 20.11.x for 20.11.05
Comment 11 Andrew Fuerste-Henry 2021-04-25 13:48:55 UTC
Doesn't apply cleanly to 20.05, please rebase for backport. I'm not entirely confident in my ability to clear the merge errors correctly and don't have credentials with which to check my work with a real search. Thanks!