fixed support for query parameters in Mojolicious::Plugin::Charset (c…

…loses #379)
mojolicious · Sep 10, 2012 · 861136c · 861136c
1 parent 4b9a26d
commit 861136c
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 51 deletions.
diff --git a/Changes b/Changes
@@ -1,7 +1,9 @@
 
-3.39  2012-09-08
+3.39  2012-09-10
+  - Improved Mojo::URL and Mojo::Parameters performance.
   - Improved documentation.
   - Improved tests.
+  - Fixed support for query parameters in Mojolicious::Plugin::Charset.
 
 3.38  2012-09-07
   - Added xor_encode method to Mojo::ByteStream.

diff --git a/lib/Mojo/Parameters.pm b/lib/Mojo/Parameters.pm
@@ -10,17 +10,7 @@ use Mojo::Util qw(decode encode url_escape url_unescape);
 has charset        => 'UTF-8';
 has pair_separator => '&';
 
-sub new {
-  my $self = shift->SUPER::new;
-
-  # Pairs
-  if (@_ > 1) { $self->append(@_) }
-
-  # String
-  else { $self->{string} = $_[0] }
-
-  return $self;
-}
+sub new { shift->SUPER::new->parse(@_) }
 
 sub append {
   my ($self, @pairs) = @_;
@@ -78,41 +68,18 @@ sub param {
 sub params {
   my ($self, $params) = @_;
   if ($params) { $self->{params} = $params and return $self }
-  elsif (defined $self->{string}) { $self->parse }
+  elsif (defined $self->{string}) { $self->_parse }
   return $self->{params} ||= [];
 }
 
 sub parse {
   my $self = shift;
-  my $string = shift // $self->{string};
 
-  # Clear
-  delete $self->params([])->{string};
-
-  # Detect pair separator for reconstruction
-  return $self unless length($string // '');
-  $self->pair_separator(';') if $string =~ /;/ && $string !~ /\&/;
-
-  # W3C suggests to also accept ";" as a separator
-  my $charset = $self->charset;
-  for my $pair (split /[\&\;]+/, $string) {
-
-    # Parse
-    $pair =~ /^([^=]*)(?:=(.*))?$/;
-    my $name  = $1 // '';
-    my $value = $2 // '';
-
-    # Replace "+" with whitespace
-    s/\+/ /g for $name, $value;
-
-    # Unescape
-    $name  = url_unescape $name;
-    $name  = decode($charset, $name) // $name if $charset;
-    $value = url_unescape $value;
-    $value = decode($charset, $value) // $value if $charset;
+  # Pairs
+  if (@_ > 1) { $self->append(@_) }
 
-    push @{$self->params}, $name, $value;
-  }
+  # String
+  else { $self->{string} = $_[0] }
 
   return $self;
 }
@@ -187,6 +154,40 @@ sub to_string {
   return join $self->pair_separator, @pairs;
 }
 
+sub _parse {
+  my $self = shift;
+
+  # Clear
+  my $string = delete $self->params([])->{string};
+
+  # Detect pair separator for reconstruction
+  return $self unless length($string // '');
+  $self->pair_separator(';') if $string =~ /;/ && $string !~ /\&/;
+
+  # W3C suggests to also accept ";" as a separator
+  my $charset = $self->charset;
+  for my $pair (split /[\&\;]+/, $string) {
+
+    # Parse
+    $pair =~ /^([^=]*)(?:=(.*))?$/;
+    my $name  = $1 // '';
+    my $value = $2 // '';
+
+    # Replace "+" with whitespace
+    s/\+/ /g for $name, $value;
+
+    # Unescape
+    $name  = url_unescape $name;
+    $name  = decode($charset, $name) // $name if $charset;
+    $value = url_unescape $value;
+    $value = decode($charset, $value) // $value if $charset;
+
+    push @{$self->params}, $name, $value;
+  }
+
+  return $self;
+}
+
 1;
 
 =head1 NAME

diff --git a/lib/Mojo/URL.pm b/lib/Mojo/URL.pm
@@ -107,27 +107,25 @@ sub query {
   my $self = shift;
 
   # Old parameters
-  return $self->{query} ||= Mojo::Parameters->new unless @_;
+  my $q = $self->{query} ||= Mojo::Parameters->new;
+  return $q unless @_;
 
   # Replace with list
   if (@_ > 1) { $self->{query} = Mojo::Parameters->new(@_) }
 
   # Merge with array
   elsif (ref $_[0] eq 'ARRAY') {
-    my $q = $self->{query} ||= Mojo::Parameters->new;
     while (my $name = shift @{$_[0]}) {
       my $value = shift @{$_[0]};
       defined $value ? $q->param($name => $value) : $q->remove($name);
     }
   }
 
   # Append hash
-  elsif (ref $_[0] eq 'HASH') {
-    ($self->{query} ||= Mojo::Parameters->new)->append(%{$_[0]});
-  }
+  elsif (ref $_[0] eq 'HASH') { $q->append(%{$_[0]}) }
 
   # Replace with string
-  else { $self->{query} = Mojo::Parameters->new($_[0]) }
+  else { $q->parse($_[0]) }
 
   return $self;
 }

diff --git a/lib/Mojolicious/Plugin/Charset.pm b/lib/Mojolicious/Plugin/Charset.pm
@@ -8,7 +8,8 @@ sub register {
   return unless my $c = $conf->{charset};
   $app->types->type(html => "text/html;charset=$c");
   $app->renderer->encoding($c);
-  $app->hook(before_dispatch => sub { shift->req->default_charset($c) });
+  $app->hook(before_dispatch =>
+      sub { shift->req->default_charset($c)->url->query->charset($c) });
 }
 
 1;

diff --git a/t/mojo/request.t b/t/mojo/request.t
@@ -556,9 +556,9 @@ is $req->headers->content_type, 'x-application-urlencoded',
 ok !$req->content->asset->is_file, 'stored in memory';
 is $req->content->asset->size, 26, 'right size';
 is $req->content->asset->slurp, 'foo=bar& tset=23+;&foo=bar', 'right content';
-is $req->body_params, 'foo=bar&+tset=23+&foo=bar', 'right parameters';
 is_deeply $req->body_params->to_hash->{foo}, [qw(bar bar)], 'right values';
 is $req->body_params->to_hash->{' tset'}, '23 ', 'right value';
+is $req->body_params, 'foo=bar&+tset=23+&foo=bar', 'right parameters';
 is_deeply $req->params->to_hash->{foo}, [qw(bar bar 13)], 'right values';
 
 # Parse HTTP 1.1 "x-application-urlencoded" (too big for memory)
@@ -577,9 +577,9 @@ is $req->headers->content_type, 'x-application-urlencoded',
 ok $req->content->asset->is_file, 'stored in file';
 is $req->content->asset->size, 26, 'right size';
 is $req->content->asset->slurp, 'foo=bar& tset=23+;&foo=bar', 'right content';
-is $req->body_params, 'foo=bar&+tset=23+&foo=bar', 'right parameters';
 is_deeply $req->body_params->to_hash->{foo}, [qw(bar bar)], 'right values';
 is $req->body_params->to_hash->{' tset'}, '23 ', 'right value';
+is $req->body_params, 'foo=bar&+tset=23+&foo=bar', 'right parameters';
 is_deeply $req->params->to_hash->{foo}, [qw(bar bar 13)], 'right values';
 
 # Parse HTTP 1.1 "application/x-www-form-urlencoded"
@@ -596,9 +596,9 @@ is $req->headers->content_type, 'application/x-www-form-urlencoded',
   'right "Content-Type" value';
 is $req->content->asset->size, 26, 'right size';
 is $req->content->asset->slurp, 'foo=bar&+tset=23+;&foo=bar', 'right content';
-is $req->body_params, 'foo=bar&+tset=23+&foo=bar', 'right parameters';
 is_deeply $req->body_params->to_hash->{foo}, [qw(bar bar)], 'right values';
 is $req->body_params->to_hash->{' tset'}, '23 ', 'right value';
+is $req->body_params, 'foo=bar&+tset=23+&foo=bar', 'right parameters';
 is_deeply $req->params->to_hash->{foo}, [qw(bar bar 13)], 'right values';
 is_deeply [$req->param('foo')], [qw(bar bar 13)], 'right values';
 is $req->param(' tset'), '23 ', 'right value';

diff --git a/t/mojolicious/charset_lite_app.t b/t/mojolicious/charset_lite_app.t
@@ -8,7 +8,7 @@ BEGIN {
   $ENV{MOJO_REACTOR} = 'Mojo::Reactor::Poll';
 }
 
-use Test::More tests => 41;
+use Test::More tests => 44;
 
 use Mojo::ByteStream 'b';
 use Mojolicious::Lite;
@@ -56,6 +56,17 @@ get '/json' => sub { shift->render_json({test => $yatta}) };
 # GET /привет/мир
 get '/привет/мир' => sub { shift->render_json({foo => $yatta}) };
 
+# GET /params
+get '/params' => sub {
+  my $self = shift;
+  $self->render_json(
+    {
+      params => $self->req->url->query->to_hash,
+      yatta  => $self->param(['yatta'])
+    }
+  );
+};
+
 my $t = Test::Mojo->new;
 
 # Plain old ASCII
@@ -108,6 +119,12 @@ $t->get_ok('/json')->status_is(200)->content_type_is('application/json')
 $t->get_ok('/привет/мир')->status_is(200)
   ->content_type_is('application/json')->json_content_is({foo => $yatta});
 
+# Shift_JIS parameters
+my $url = $t->ua->app_url->path('/params')->query(foo => 3, yatta => $yatta);
+$url->query->charset('shift_jis');
+$t->get_ok($url)->status_is(200)
+  ->json_content_is({params => {foo => 3, yatta => $yatta}, yatta => $yatta});
+
 __DATA__
 @@ index.html.ep
 <p>やった</p>