Bug 15190 - Bad utf8 decode to unapi and fixing code status 200
Summary: Bad utf8 decode to unapi and fixing code status 200
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Héctor Eduardo Castro Avalos
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-13 21:46 UTC by Héctor Eduardo Castro Avalos
Modified: 2017-06-14 22:07 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:

Attachments
15190 - Bad utf8 decode to unapi and fixing blank screen error status (3.80 KB, patch)
2015-11-13 21:54 UTC, Héctor Eduardo Castro Avalos 		Details | Diff | Splinter Review
Bug 15190: Bad utf8 decode to unapi and fixing blank screen error status (3.80 KB, patch)
2015-11-16 19:38 UTC, Héctor Eduardo Castro Avalos 		Details | Diff | Splinter Review
Bug 15190: Bad utf8 decode to unapi and fixing blank screen error status (3.86 KB, patch)
2015-12-04 21:52 UTC, Héctor Eduardo Castro Avalos 		Details | Diff | Splinter Review
Bug 15190: Bad utf8 decode to unapi and fixing blank screen error status (3.92 KB, patch)
2015-12-04 22:36 UTC, Héctor Eduardo Castro Avalos 		Details | Diff | Splinter Review
Bug 15190: Bad utf8 decode to unapi and fixing blank screen error status (4.20 KB, patch)
2015-12-05 16:34 UTC, Marc Véron 		Details | Diff | Splinter Review
Bug 15190: Bad utf8 decode to unapi and fixing status code 200 (2.35 KB, patch)
2016-01-12 04:38 UTC, Héctor Eduardo Castro Avalos 		Details | Diff | Splinter Review
Bug 15190: Bad utf8 decode to unapi and fixing status code 200 (2.53 KB, patch)
2016-01-13 11:04 UTC, Marc Véron 		Details | Diff | Splinter Review
Bug 15190: Bad utf8 decode to unapi and fixing status code 200 (2.54 KB, patch)
2016-01-13 12:23 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Héctor Eduardo Castro Avalos 2015-11-13 21:46:41 UTC 
Fix bad utf8 decoding, also fix cgi->header(-status) to
redirect to one of the posible error pages in /cgi-bin/koha/errors/
Comment 1 Héctor Eduardo Castro Avalos 2015-11-13 21:54:13 UTC Comment hidden (obsolete)
Comment 2 Héctor Eduardo Castro Avalos 2015-11-16 19:38:24 UTC Comment hidden (obsolete)
Comment 3 Marc Véron 2015-12-04 21:01:18 UTC 
Hi Hector, I could reproduce the blank screen issue and it is resolved with your patch.

Regarding the bad utf8 encoding (#4 in test plan), can you give a hint on what is wrong?
Comment 4 Héctor Eduardo Castro Avalos 2015-12-04 21:31:57 UTC 
(In reply to Marc Véron from comment #3)
> Hi Hector, I could reproduce the blank screen issue and it is resolved with
> your patch.
> 
> Regarding the bad utf8 encoding (#4 in test plan), can you give a hint on
> what is wrong?

Hi Marc, for me the patch works well, maybe I need to reword message of commit. Let me improve.
Comment 5 Héctor Eduardo Castro Avalos 2015-12-04 21:52:20 UTC Comment hidden (obsolete)
Comment 6 Héctor Eduardo Castro Avalos 2015-12-04 22:31:31 UTC 
(In reply to Marc Véron from comment #3)

> Regarding the bad utf8 encoding (#4 in test plan), can you give a hint on
> what is wrong?

Ahh.. Ok I understand the deconding problem only happend with oai_dc, srw_dc, mods, mods-full, mods3, mods3-full, rdfdc

The format marcxml works well doesn't have utf8 decoding problem.

<dc:title>Antología de entremeses del Siglo de Oro /</dc:title><dc:creator>
    Arellano, Ignacio.
  </dc:creator><dc:creator>
    García Valdés, Celsa Carmen,
    ed.
  </dc:creator>

If you apply the patch
<dc:title>Antología de entremeses del Siglo de Oro /</dc:title><dc:creator>
    Arellano, Ignacio.
  </dc:creator><dc:creator>
    García Valdés, Celsa Carmen,
    ed.
  </dc:creator>

I saw the problem in 3.14 or 3.16 but I think in some way someone corrected the problem, Zotero take metadata from marcxml.

Whe i was testing this bug I only tested with oai_dc, srw_dc and rdfdc.

That's my mistake
Comment 7 Héctor Eduardo Castro Avalos 2015-12-04 22:36:08 UTC Comment hidden (obsolete)
Comment 8 Marc Véron 2015-12-05 16:34:37 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-12-11 09:35:32 UTC 
Looks good to me, but I don't have enough expertise in this script to be 100% confident with the changes.
Galen, could we get your opinion on this patch please?
I am not sure about 2 points:
- replace printing header with a redirect
- move error 406 to 400
Comment 10 Marcel de Rooy 2015-12-24 11:43:40 UTC 
Hector:
Thanks for your patch.
I think we still need some changes before moving this to PQA:

[1] The encoding problem is imo concentrated in these lines:

Around line 155 we should do:
print $cgi->header( -type =>'application/xml', -charset => 'UTF-8' );
Add the character set ! (Maybe this is needed a few times more..)

Around line 147 we should use chars instead of toString:
# $content = $record_dom->toString();
$content = $transformer->output_as_chars( $record_dom );

According to my testing that should resolve the encoding problem. Note that I would prefer to stick to the binmode statement and not add the decodes.

[2] The change from 406 to 400 is arguable. Why change it now?

[3] Do not redirect to other Koha scripts. This is a separate unAPI implementation. What does unAPI exactly say about such statuses? (I could not reach its site for now..) Note that the browser may affect the results displayed too. I saw differences here between FF and IE.
Comment 12 Héctor Eduardo Castro Avalos 2016-01-12 04:25:50 UTC 
Thank you very much, Marcel.

I've changed the name of the bug according with the info you sent me.
I make the changes that you recommend it. I only changed the 200 Ok status if no parameters passed to script.
Comment 13 Héctor Eduardo Castro Avalos 2016-01-12 04:38:59 UTC Comment hidden (obsolete)
Comment 14 Marcel de Rooy 2016-01-12 07:36:51 UTC 
Great, Hector.
Marc: if you can your signoff again, I can QA again..
Comment 15 Marc Véron 2016-01-12 09:44:00 UTC 
Hi Héctor,

The encoding is OK with the newest patch for example with:
cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=oai_dc
(tested with all of: oai_dc, srw_dc, mods, mods-full, mods3, mods3-full, rdfdc)

What should happen if I do:

cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=xxx
cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=

What should happen if I use a biblionumber that does not exist?
cgi-bin/koha/unapi?id=koha:biblionumber:999999999&format=oai_dc

At the moment I get a blank page for all cases.
Comment 16 Marcel de Rooy 2016-01-12 14:49:56 UTC 
(In reply to Marc Véron from comment #15)
> Hi Héctor,
> 
> The encoding is OK with the newest patch for example with:
> cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=oai_dc
> (tested with all of: oai_dc, srw_dc, mods, mods-full, mods3, mods3-full,
> rdfdc)
> 
> What should happen if I do:
> 
> cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=xxx
> cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=
> 
> What should happen if I use a biblionumber that does not exist?
> cgi-bin/koha/unapi?id=koha:biblionumber:999999999&format=oai_dc
> 
> At the moment I get a blank page for all cases.

Blank page is not an error. Unapi should return a http status code for bad requests. See comment11
Comment 17 Héctor Eduardo Castro Avalos 2016-01-12 19:05:02 UTC 
(In reply to Marcel de Rooy from comment #16)

Hi Marc

Thank you for keep testing. As Marcel comments the way of the bug has change since the documentation in comment 11. ( I was in a mistake)

> (In reply to Marc Véron from comment #15)

> > What should happen if I do:
> > 
> > cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=xxx
> > cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=

According with the first revision, the unapi must return an code status 300 multiple choice.
 
> > What should happen if I use a biblionumber that does not exist?
> > cgi-bin/koha/unapi?id=koha:biblionumber:999999999&format=oai_dc

This must return a 400 or 404 code.

> > 
> > At the moment I get a blank page for all cases.
> 
> Blank page is not an error. Unapi should return a http status code for bad
> requests. See comment11

Regards
Comment 18 Marc Véron 2016-01-13 10:02:45 UTC 
Hi Héctor, i did more testing:


cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=xxx
cgi-bin/koha/unapi?id=koha:biblionumber:45100&format=

Result for both:
  406 invalid format requested
  Empty page

http://10.0.0.62/cgi-bin/koha/unapi?id=koha:biblionumber:999999999&format=oai_dc
Result:
  404 record not found
  Empty page

http://10.0.0.62/cgi-bin/koha/unapi?id=koha:biblionumber:xxxxxxxxxxxxx&format=oai_dc
Result:
  404 record not found
  Empty page

http://10.0.0.62/cgi-bin/koha/unapi?id=koha:blabla:1&format=oai_dc
Result:
  404 record not found
  Empty page

http://10.0.0.62/cgi-bin/koha/unapi?id=koha:biblionumber:1xxxxxxxxxxxxx&format=oai_dc
Result:
  200 OK
  Page beginning with: 
  oai_dc:dc xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/oai_dc/   http://www.openarchives.org/OAI/2.0/oai_dc.xsd">
  (...)
  Same as with koha:biblionumber:1 (existing number)

http://10.0.0.62/cgi-bin/koha/unapi?id=
Result:
  300 multiple choices
  Page beginning with:
  <formats id=""><format name="marcxml" type="application/xml" 
  (...)
Comment 19 Marcel de Rooy 2016-01-13 10:59:02 UTC 
Marc:
This looks reasonable to me.
Note that the biblionumber is extracted like this:
if ($id =~ /koha:biblionumber:(\d+)/) {
        my $biblionumber = $1;
This is current behavior and could be considered as outside the scope of this report..
The id= (empty) case is actually the same as without any parameters. So should present formats.
I do not have a problem with the 406; this particular case was a 400.
Comment 20 Marc Véron 2016-01-13 11:04:28 UTC Comment hidden (obsolete)
Comment 21 Marcel de Rooy 2016-01-13 12:23:13 UTC 
Created attachment 46581 [details] [review]
Bug 15190: Bad utf8 decode to unapi and fixing status code 200

Fix bad utf8 decoding, also fix status code 200 for no parameters passed to
script

To reproduce the issue in OPAC
1) Copy from LOC (Z39.50) ISBN 8467020113 or test it with whichever record with
   special characters
2) Open URL in OPAC cgi-bin/koha/unapi?id=koha:biblionumber:4&format=oai_dc
   play with oai_dc, srw_dc, mods, mods-full, mods3, mods3-full, rdfdc.
   Note: marcxml does not fail
3) Enter the new biblionumber for ISBN 8467020113 or the record of your choise
4) An oai_dc record will display, notice about the bad utf8
   decoding characters
5) Search the record in OPAC
6) To reproduce code 200 Ok and 300 Multiple choices play with:
    cgi-bin/koha/unapi
    cgi-bin/koha/unapi?id=koha:biblionumber:4
7) If possible test in NORMARC (UNIMARC will launch an error because bug 15162)

Test plan
-Apply patch
-Follow steps 2 to 6
-You will see no errors in utf8

Followed test plan, works as expected.
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Brendan Gallagher 2016-01-27 05:29:10 UTC 
Pushed to Master - Should be in the May 2016 release. Thanks!
Comment 23 Julian Maurice 2016-01-29 15:05:19 UTC 
Patch pushed to 3.22.x, will be in 3.22.3
Comment 24 Frédéric Demians 2016-02-12 06:44:53 UTC 
This patch has been pushed to 3.20.x, will be in 3.20.9.