[Bug 25774] New: REST API searches don't handle correctly utf8 characters
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Bug ID: 25774 Summary: REST API searches don't handle correctly utf8 characters Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: REST API Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org In the search box, search "❤": [2020/06/16 14:11:37] [ERROR] GET /api/v1/libraries: unhandled exception (Mojo::Exception)<<Wide character in subroutine entry at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 107.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73) 172.18.0.1 - - [16/Jun/2020:14:11:37 +0000] "GET /api/v1/app.pl/api/v1/libraries?_page=1&_per_page=20&q=%5B%7B%22me.name%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.library_id%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.marc_org_code%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.ip%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.pickup_location%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address1%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address2%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address3%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.city%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.state%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.postal_code%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.country%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.opac_info%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.notes%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%5D&_match=contains&_order_by=%2Bme.name HTTP/1.1" 500 - "http://kohadev-intra.mydnsname.org:8081/cgi-bin/koha/admin/branches.pl" "Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 105920 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105920&action=edit Bug 25774: Handle utf8 chars in REST API queries We must utf8 encode the string before json decoding it. Test plan: Use "❤" and play with the library search (bug 25288) and Postman to generate queries using it. This patch prevents 500: [2020/06/16 14:11:37] [ERROR] GET /api/v1/libraries: unhandled exception (Mojo::Exception)<<Wide character in subroutine entry at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 107.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105920|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 105930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105930&action=edit Bug 25774: Handle utf8 chars in REST API queries We must utf8 encode the string before json decoding it. Test plan: Use "❤" and play with the library search (bug 25288) and Postman to generate queries using it. This patch prevents 500: [2020/06/16 14:11:37] [ERROR] GET /api/v1/libraries: unhandled exception (Mojo::Exception)<<Wide character in subroutine entry at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 107.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- I don't think that this is the most elegant/maintainable solution. If you change "decode_json" to "from_json", you shouldn't need to use encode_utf8, since decode_json expects bytes and from_json expects a string. By using encode_utf8, you're taking a string and turning it into bytes, when you could just keep it as a string. The documentation for the JSON module is pretty terrible, but I've proved this this with the following three sample files: {"test":"❤"} #!/usr/bin/perl use strict; use warnings; use JSON; open(my $fh, '<', 'test.txt'); my $line = <$fh>; my $perl = decode_json($line); warn $perl->{test}; #!/usr/bin/perl use strict; use warnings; use JSON; open(my $fh, '<', 'test.txt'); my $line = <$fh>; my $perl = from_json($line); warn $perl->{test}; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- Created attachment 105932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105932&action=edit Bug 25774: replace decode_json with from_json decode_json expects octets/bytes but from_json expects a string. We don't need to encode the query parameters as bytes to give to decode_json. Instead, we can just pass the string to from_json, and it will work as expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Found this one difficult to test, since the added unit test doesn't test the change. Applied Bug 25288 and its dependencies... Changed the "Name" of the first library to ❤. Tried the AJAX search at /cgi-bin/koha/admin/branches.pl without Bug 25288 and got a 500 error. Tried the AJAX search at /cgi-bin/koha/admin/branches.pl with Bug 25288 (including my follow-up) and it successfully/correctly filtered. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- With Bug 25288 applied, you can also test this by hitting the following URL in koha-testing-docker: http://localhost:8081/api/v1/libraries?_page=1&_per_page=20&q=%5B%7B%22me.name%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.library_id%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.marc_org_code%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.ip%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.pickup_location%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address1%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address2%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.address3%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.city%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.state%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.postal_code%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.country%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.opac_info%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%2C%7B%22me.notes%22%3A%7B%22like%22%3A%22%25%E2%9D%A4%25%22%7D%7D%5D&_match=contains&_order_by=%2Bme.name -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105930|0 |1 is obsolete| | Attachment #105932|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 105935 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105935&action=edit Bug 25774: Handle utf8 chars in REST API queries We must utf8 encode the string before json decoding it. Test plan: Use "❤" and play with the library search (bug 25288) and Postman to generate queries using it. This patch prevents 500: [2020/06/16 14:11:37] [ERROR] GET /api/v1/libraries: unhandled exception (Mojo::Exception)<<Wide character in subroutine entry at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 107.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Hum weird, no idea what I did with the test. It should test correctly the changes now. David, from the JSON pod: """ This module also exports to_json and from_json for backward compatibility. These are slower, and may expect/generate different stuff from what encode_json and decode_json do, depending on their options. It's better just to use Object-Oriented interfaces than using these two functions. """ We should use JSON->decode, see the last patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #8)
Hum weird, no idea what I did with the test. It should test correctly the changes now.
David, from the JSON pod: """ This module also exports to_json and from_json for backward compatibility. These are slower, and may expect/generate different stuff from what encode_json and decode_json do, depending on their options. It's better just to use Object-Oriented interfaces than using these two functions. """
We should use JSON->decode, see the last patch.
What about my $json = JSON->new->utf8; ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #9)
(In reply to Jonathan Druart from comment #8)
Hum weird, no idea what I did with the test. It should test correctly the changes now.
David, from the JSON pod: """ This module also exports to_json and from_json for backward compatibility. These are slower, and may expect/generate different stuff from what encode_json and decode_json do, depending on their options. It's better just to use Object-Oriented interfaces than using these two functions. """
We should use JSON->decode, see the last patch.
What about
my $json = JSON->new->utf8;
?
Forget about that last comment. Setting ->utf8 would mean we would need to explicitly call encode_utf8 on the string as the original patch did [1] [1] utf8 flag disabled section in https://metacpan.org/pod/JSON#ENCODING/CODESET-FLAG-NOTES -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #8)
Hum weird, no idea what I did with the test. It should test correctly the changes now.
David, from the JSON pod: """ This module also exports to_json and from_json for backward compatibility. These are slower, and may expect/generate different stuff from what encode_json and decode_json do, depending on their options. It's better just to use Object-Oriented interfaces than using these two functions. """
We should use JSON->decode, see the last patch.
Sounds good to me. Looking at the code for "from_json", I see what they mean, although they don't articulate it very well. I have some more urgent things to attend to, but I'll keep this in my inbox. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105935|0 |1 is obsolete| | --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 106109 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106109&action=edit Bug 25774: Handle utf8 chars in REST API queries We must utf8 encode the string before json decoding it. Test plan: Use "❤" and play with the library search (bug 25288) and Postman to generate queries using it. This patch prevents 500: [2020/06/16 14:11:37] [ERROR] GET /api/v1/libraries: unhandled exception (Mojo::Exception)<<Wide character in subroutine entry at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 107.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106109|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 106110 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106110&action=edit Bug 25774: Handle utf8 chars in REST API queries We must utf8 encode the string before json decoding it. Test plan: Use "❤" and play with the library search (bug 25288) and Postman to generate queries using it. This patch prevents 500: [2020/06/16 14:11:37] [ERROR] GET /api/v1/libraries: unhandled exception (Mojo::Exception)<<Wide character in subroutine entry at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 107.>> Koha::REST::Plugin::Exceptions::__ANON__ /kohadevbox/koha/Koha/REST/Plugin/Exceptions.pm (73) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|20.11.00 |20.11.00 20.05.01 released in| | --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- backported to 20.05.x for 20.05.01 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25774 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #16 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Does not apply cleanly to 19.11.x. Please rebase if required on 19.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org