Bug 9951

Summary: Resolve encoding problems where script parameters are not decoded
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Architecture, internals, and plumbingAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED WONTFIX QA Contact:
Severity: normal    
Priority: P5 - low CC: dpavlin
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10019
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 6554    
Bug Blocks:    
Attachments: Bug 9951: Followup for Tools
Bug 9951: Followup for tools/viewlog.pl
Bug 9951: Followup for Tools
Bug 9951: Followup for tools/viewlog.pl

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.