Bug 10019 - Encoding problem with utf8 chars in userid
Summary: Encoding problem with utf8 chars in userid
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Marcel de Rooy
QA Contact:
URL:
Keywords:
Depends on: 6554
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-10 07:47 UTC by Marcel de Rooy
Modified: 2014-12-07 20:02 UTC (History)
3 users (show)

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


Attachments
Bug 10019: Fix for userid containing UTF8 chars (1.54 KB, patch)
2013-04-10 08:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10019: Fix for userid containing UTF8 chars (1.71 KB, patch)
2013-04-10 11:10 UTC, Bernardo Gonzalez Kriegel
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-04-10 07:47:13 UTC
As reported by Karam under 6554:

if you create a new staff user named with non latin characters like : كرم  " my name in Arabic " 
when log in there is no problem the name is appears but when you change the page go to any other page " like Circulation" the name is gooing to be like this :( ÙرÙ,
and if we go back to the main page the name still in these characters ÙرÙ

If we go the the Global system preferences page there is no problem there
Comment 1 Marcel de Rooy 2013-04-10 08:10:37 UTC
A observation to start with:
The line in admin/preferences.pl:
$YAML::Syck::ImplicitUnicode = 1;
somehow makes the userid appear correctly.
Looking at it from a distance, this seems a unwanted side-effect. This statement influences more than it looks like. We could also enable this YAML var only when needed (during the load of the pref file). But this is another report.

My solution for the current problem does not interfere with this side-effect..
Comment 2 Marcel de Rooy 2013-04-10 08:21:06 UTC
To solve this problem, I followed Bernardo's idea about CGI::Session. But I checked the session info and it was good.
But deeper in Auth.pm, the userid parameter was not decoded. When coming from CGI Session in one instance (it is decoded in setuserenv but later used again from CGI::Session) and when coming from CGI in another instance.
So a two-liner patch should resolve the problem..
To test this, you really need to login/logout on opac/staff, delete sessions, etc. :)
Comment 3 Marcel de Rooy 2013-04-10 08:21:15 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2013-04-10 08:30:10 UTC
Karam, Bernardo:
Have a look too at bug 9951 where I describe in comment1 the problem with utf8 chars in your password..
That is a really nice one to solve :)
Comment 5 Bernardo Gonzalez Kriegel 2013-04-10 11:10:23 UTC
Created attachment 17332 [details] [review]
Bug 10019: Fix for userid containing UTF8 chars

Decodes userid on two places in checkauth of C4/Auth.pm

Test plan:
Include some non-Latin characters in your userid (loginname). Arab, Chinese?
Login into opac and check user page.
Go to staff (no new login), check your login name at various places.
Logout, login via staff.
Do the same.
Go to opac again (no new login), check user page.
Optionally: Remove all your sessions from table. Do a login. Check sessions.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Works as described. No errors.
This patch fixes this problem, but I wonder if
there is a general solution that handle all as utf8.
Tested in opac and staff.
Comment 6 Marcel de Rooy 2013-04-10 11:18:36 UTC
(In reply to comment #5)
> This patch fixes this problem, but I wonder if
> there is a general solution that handle all as utf8.
> Tested in opac and staff.

Thanks for your SO. A simpler solution for UTF-8 would be really welcome, but IMO we are now in a transition period.
Currently, some scripts use CGI now with -utf8 flag for decoding all parameters. But we already have more modules like Auth.pm that have decodes here and there.
Comment 7 Karam Qubsi 2013-04-10 17:10:12 UTC
The problem is fixed using this patch :)
Thanks alot for your fast respond :)
Comment 8 Marcel de Rooy 2013-04-19 14:01:24 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:14:13 UTC
This patch has been pushed to master and 3.12.x.
Comment 10 Marcel de Rooy 2013-05-06 07:36:25 UTC
Reverted. Closing this report.