Bug 9951 - Resolve encoding problems where script parameters are not decoded
Summary: Resolve encoding problems where script parameters are not decoded
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact:
URL:
Keywords:
Depends on: 6554
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-29 08:57 UTC by Marcel de Rooy
Modified: 2014-12-07 20:03 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9951: Followup for Tools (1.43 KB, patch)
2013-04-02 13:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9951: Followup for tools/viewlog.pl (731 bytes, patch)
2013-04-02 13:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9951: Followup for Tools (1.45 KB, patch)
2013-04-07 02:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9951: Followup for tools/viewlog.pl (1.13 KB, patch)
2013-04-07 02:27 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 Marcel de Rooy 2013-03-29 08:57:10 UTC
Bug 6554 corrects the handling of UTF-8 in template processing.
This may actually reveal some bugs at other places in Koha where cgi parameters are not decoded. (I resolved some of these places with some followups for 6554, but like to freeze that report now.)

There is an easy fix. We have two options:
1 Add the -utf8 parameter when using CGI at the start of the script.
2 Decode the parameters individually where utf8 chars are to be expected.
For both methods, we could say something. I will send a mail to the dev list also to trigger a discussion and hopefully reach a best [Koha?] practice.

I you still find such an instance, add it here and possibly include a patch.
Comment 1 Marcel de Rooy 2013-03-29 10:10:10 UTC
I took some time to glance through all opac scripts. With the following results:

A nice one to start with:
opac-password.pl: The password may contain diacritics. It should be decoded. But this needs a dive into C4/Auth.pm also (and probably in staff too).
And we still have the problem with old incorrect hashes in the table. If you would have a old password with diacritics, you would still like to login.
This is not just adding a flag to CGI, but a report on its own. So I leave that for now; these hashes still work :)

Most scripts look fine to me; some have been corrected already. I only have some doubts on:
ilsdi.pl
oai.pl
search.pl
tracklinks.pl
Comment 2 Marcel de Rooy 2013-04-02 13:02:51 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-04-02 13:09:36 UTC
It seems that the scripts that are posting data generally do not have problems (at this moment..) but when GET is used for searching on names etc., we need to decode the parameters. This has to do with CGI.pm internals: the GET params are coming from the environment, the POST values are read from STDIN.
Comment 4 Marcel de Rooy 2013-04-02 13:20:19 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2013-04-07 02:27:26 UTC
Created attachment 17221 [details] [review]
Bug 9951: Followup for Tools

Adds decoding for Patron Cards/Patron Search and Tags (moderation).

Test plan:
Goto Tools. Add accent with diacritics in both areas. Check if you do not see
converted characters.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 6 Chris Cormack 2013-04-07 02:27:43 UTC
Created attachment 17222 [details] [review]
Bug 9951: Followup for tools/viewlog.pl

Adds utf8 cgi parameter decoding.

Test plan: Enter diacritics in librarian field. Submit. Check.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

This is ok in this case, as we are not doing any post/get of binary
data, but I note from the man page

This makes CGI.pm treat all parameters as UTF-8 strings. Use this with
care, as it will interfere
with the processing of binary uploads. It is better to
manually select which fields are expected
to return utf-8 strings and convert them using code like
this:

  use Encode;
  my $arg = decode utf8=>param('foo');
Comment 7 Marcel de Rooy 2013-04-07 12:00:27 UTC
(In reply to comment #6)
> Created attachment 17222 [details] [review] [review]
> This is ok in this case, as we are not doing any post/get of binary
> data, but I note from the man page
> 
> This makes CGI.pm treat all parameters as UTF-8 strings. Use this with
> care, as it will interfere

Thx for your SO. I discussed this also with Dobrica on the original report 6554. I did not yet send a message to the list. Currently, both approaches are used in Koha.
Comment 8 Marcel de Rooy 2013-04-19 14:04:23 UTC
Taking the liberty to let this pass qa now too in connection with 6554 and the other encoding reports.
Note that these small patches are actually qa followups for 6554. So not a real patch coming from me :)

Passed QA
Comment 9 Jared Camins-Esakov 2013-04-20 03:26:55 UTC
This patch has been pushed to master and 3.12.x.
Comment 10 Marcel de Rooy 2013-05-06 13:26:57 UTC
Has been reverted. Closing this bug.