Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow decoding of JSON messages in non-standard character sets #2093

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Allow decoding of JSON messages in non-standard character sets #2093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/Mojo/Message.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use Mojo::JSON qw(j);
use Mojo::JSON::Pointer;
use Mojo::Parameters;
use Mojo::Upload;
use Mojo::Util qw(decode);
use Mojo::Util qw(encode decode);

has content => sub { Mojo::Content::Single->new };
has default_charset => 'UTF-8';
Expand Down Expand Up @@ -132,7 +132,8 @@ sub is_limit_exceeded { !!shift->{limit} }
sub json {
my ($self, $pointer) = @_;
return undef if $self->content->is_multipart;
my $data = $self->{json} //= j($self->body);
my $charset = $self->body_params->charset || $self->default_charset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what the performance cost of this is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, $self->content->charset is the way to go.

my $data = $self->{json} //= j($charset eq 'UTF-8' ? $self->body : encode('UTF-8', decode($charset, $self->body)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

j(encode(decode())) seems super inefficient.

Copy link
Author

@codehead codehead Aug 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Tried this before:

my $data = $charset eq 'UTF-8' ? j($self->body) : from_json(decode($charset,$self->body));

which seems more natural but breaks the last test in t/mojo/response.t subtest "Parse response and extract JSON data"

t/mojo/response.t .......................... 1/?
    #   Failed test 'right result'
    #   at t/mojo/response.t line 1075.
    #     Structures begin differing at:
    #          $got->[1] = '2'
    #     $expected->[1] = '4'
    # Looks like you failed 1 test of 9.

#   Failed test 'Parse response and extract JSON data'
#   at t/mojo/response.t line 1076.
# Looks like you failed 1 test of 56.

i.e. this one:

  is_deeply $res->json('/baz'), [1, 2, 3], 'right result';
  $res->json->{baz}[1] = 4;
  is_deeply $res->json('/baz'), [1, 4, 3], 'right result'; # <-- This one here

j(encode(decode())) is awkward but avoids that subtle breakage.

I might be missing something, so all suggestions are welcome.

return $pointer ? Mojo::JSON::Pointer->new($data)->get($pointer) : $data;
}

Expand Down
61 changes: 59 additions & 2 deletions t/mojo/response.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ use Mojo::Asset::File;
use Mojo::Content::Single;
use Mojo::Content::MultiPart;
use Mojo::File qw(tempdir);
use Mojo::JSON qw(encode_json);
use Mojo::JSON qw(encode_json to_json);
use Mojo::Message::Response;
use Mojo::Util qw(encode gzip);
use Mojo::Util qw(encode gzip);
use Mojo::ByteStream qw(b);

subtest 'Defaults' => sub {
my $res = Mojo::Message::Response->new;
Expand Down Expand Up @@ -1074,6 +1075,62 @@ subtest 'Parse response and extract JSON data' => sub {
is_deeply $res->json('/baz'), [1, 4, 3], 'right result';
};

subtest 'Parse response and extract JSON data in shift_jis encoding' => sub {
my $yatta = 'やった';
my $yatta_sjis = b($yatta)->encode('shift_jis')->to_string;
my $res = Mojo::Message::Response->new;
$res->parse("HTTP/1.1 200 OK\x0a");
$res->parse("Content-Type: application/json;charset=shift_jis\x0a");
$res->parse("Content-Length: 16\x0a\x0a");
$res->parse(to_json({foo => $yatta_sjis}));
ok $res->is_finished, 'response is finished';
is $res->code, 200, 'right status';
is $res->message, 'OK', 'right message';
is $res->version, '1.1', 'right version';
is length($res->body), 16, 'right length';
is $res->body_params->charset, "shift_jis", 'right charset';
is_deeply $res->json, {foo => $yatta}, 'right JSON data';
is $res->json('/foo'), $yatta, 'right result';
};

subtest 'Parse response and extract JSON data in euc-jp encoding' => sub {
my $miyagawa = '宮川';
my $miyagawa_jp = b($miyagawa)->encode('euc-jp')->to_string;
is $miyagawa_jp, "\x{B5}\x{DC}\x{C0}\x{EE}", 'correct encoding';
my $res = Mojo::Message::Response->new;
$res->parse("HTTP/1.1 200 OK\x0a");
$res->parse("Content-Type: application/json;charset=euc-jp\x0a");
$res->parse("Content-Length: 14\x0a\x0a");
$res->parse(to_json({foo => $miyagawa_jp}));
ok $res->is_finished, 'response is finished';
is $res->code, 200, 'right status';
is $res->message, 'OK', 'right message';
is $res->version, '1.1', 'right version';
is length($res->body), 14, 'right length';
is $res->body_params->charset, "euc-jp", 'right charset';
is_deeply $res->json, {foo => $miyagawa}, 'right JSON data';
is $res->json('/foo'), $miyagawa, 'right result';
};

subtest 'Parse response and extract JSON data in ISO-8859-1 encoding' => sub {
my $hola = "áèñ";
my $hola_latin1 = b($hola)->encode('ISO-8859-1')->to_string;
is $hola_latin1, "\x{e1}\x{e8}\x{f1}", 'correct encoding';
my $res = Mojo::Message::Response->new;
$res->parse("HTTP/1.1 200 OK\x0a");
$res->parse("Content-Type: application/json;charset=ISO-8859-1\x0a");
$res->parse("Content-Length: 13\x0a\x0a");
$res->parse(to_json({foo => $hola_latin1}));
ok $res->is_finished, 'response is finished';
is $res->code, 200, 'right status';
is $res->message, 'OK', 'right message';
is $res->version, '1.1', 'right version';
is length($res->body), 13, 'right length';
is $res->body_params->charset, "ISO-8859-1", 'right charset';
is_deeply $res->json, {foo => $hola}, 'right JSON data';
is $res->json('/foo'), $hola, 'right result';
};

subtest 'Parse response and extract HTML' => sub {
my $res = Mojo::Message::Response->new;
$res->parse("HTTP/1.1 200 OK\x0a");
Expand Down
63 changes: 63 additions & 0 deletions t/mojolicious/json_charset_lite_app.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use Mojo::Base -strict;

BEGIN {$ENV{MOJO_REACTOR} = 'Mojo::Reactor::Poll'}

use Test::Mojo;
use Test::More;
use Mojo::ByteStream qw(b);
use Mojolicious::Lite;

my $ascii = 'abc';
my $yatta = 'やった';
my $yatta_sjis = b($yatta)->encode('shift_jis')->to_string;
my $miyagawa = '宮川';
my $miyagawa_jp = b($miyagawa)->encode('euc-jp')->to_string;
my $hola = "áèñ";
my $hola_latin1 = "\x{e1}\x{e8}\x{f1}";

get '/' => [ format => [ $yatta ] ] => { format => undef } => 'index';

post '/' => sub {
my $c = shift;
$c->render(text => "foo: " . $c->param('foo'));
};

get '/ascii' => sub {
my $c = shift;
$c->render(json => { test => $ascii })
};

get '/unicode' => sub {
my $c = shift;
$c->render(json => { test => $yatta })
};

get '/shift_jis' => sub {
my $c = shift;
$c->res->headers->content_type("application/json;charset=Shift_JIS");
$c->render(data => qq({"test":"$yatta_sjis"}));
};

get '/euc_jp' => sub {
my $c = shift;
$c->res->headers->content_type("application/json;charset=euc-jp");
$c->render(data => qq({"test":"$miyagawa_jp"}));
};

get '/latin1' => sub {
my $c = shift;
$c->res->headers->content_type("application/json;charset=ISO-8859-1");
$c->stash(data => qq({"test":"$hola_latin1"}));
};

my $t = Test::Mojo->new;

$t->get_ok('/ascii')->status_is(200)->content_is('{"test":"abc"}');
$t->get_ok('/ascii')->status_is(200)->json_is('/test' => $ascii);
$t->get_ok('/unicode')->status_is(200)->content_type_is('application/json;charset=UTF-8')->json_is('/test' => $yatta);
$t->get_ok('/shift_jis')->status_is(200)->content_type_is('application/json;charset=Shift_JIS')->json_is('/test' => $yatta);
$t->get_ok('/euc_jp')->status_is(200)->content_type_is('application/json;charset=euc-jp')->json_is('/test' => $miyagawa);
$t->get_ok('/latin1')->status_is(200)->content_type_is('application/json;charset=ISO-8859-1')->json_is('/test' => $hola);


done_testing();