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"
Created attachment 105920 [details] [review] 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)
Created attachment 105930 [details] [review] 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>
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};
Created attachment 105932 [details] [review] 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.
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.
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
Created attachment 105935 [details] [review] 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)
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.
(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; ?
(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
(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.
Created attachment 106109 [details] [review] 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>
Created attachment 106110 [details] [review] 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>
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.01
Does not apply cleanly to 19.11.x. Please rebase if required on 19.11.x