fix bug in Mojolicious::Validator::Validation where empty optional

values were not being handled correctly (closes #824)
mojolicious · Aug 7, 2015 · 0bfc963 · 0bfc963
1 parent 5a399f7
commit 0bfc963
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 18 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,7 @@
 
-6.15  2015-07-17
+6.15  2015-08-07
+  - Fixed bug in Mojolicious::Validator::Validation where empty optional values
+    were not being handled correctly.
   - Fixed warnings in Mojo::DOM.
 
 6.14  2015-07-12

diff --git a/lib/Mojo/Headers.pm b/lib/Mojo/Headers.pm
@@ -7,7 +7,7 @@ has max_line_size => sub { $ENV{MOJO_MAX_LINE_SIZE} || 8192 };
 has max_lines     => sub { $ENV{MOJO_MAX_LINES}     || 100 };
 
 # Common headers
-my %NORMALCASE = map { lc($_) => $_ } (
+my %NORMALCASE = map { lc() => $_ } (
   qw(Accept Accept-Charset Accept-Encoding Accept-Language Accept-Ranges),
   qw(Access-Control-Allow-Origin Allow Authorization Cache-Control Connection),
   qw(Content-Disposition Content-Encoding Content-Language Content-Length),

diff --git a/lib/Mojo/JSON.pm b/lib/Mojo/JSON.pm
@@ -161,11 +161,11 @@ sub _decode_string {
 
         # High surrogate
         ($ord & 0xfc00) == 0xd800
-          or pos($_) = $pos + pos($str), _throw('Missing high-surrogate');
+          or pos = $pos + pos($str), _throw('Missing high-surrogate');
 
         # Low surrogate
         $str =~ /\G\\u([Dd][C-Fc-f]..)/gc
-          or pos($_) = $pos + pos($str), _throw('Missing low-surrogate');
+          or pos = $pos + pos($str), _throw('Missing low-surrogate');
 
         $ord = 0x10000 + ($ord - 0xd800) * 0x400 + (hex($1) - 0xdc00);
       }

diff --git a/lib/Mojolicious/Validator/Validation.pm b/lib/Mojolicious/Validator/Validation.pm
@@ -63,27 +63,32 @@ sub has_error { $_[1] ? exists $_[0]{error}{$_[1]} : !!keys %{$_[0]{error}} }
 
 sub is_valid { exists $_[0]->output->{$_[1] // $_[0]->topic} }
 
-sub optional {
-  my ($self, $name) = @_;
-
-  my $input = $self->input->{$name};
-  my @input = ref $input eq 'ARRAY' ? @$input : $input;
-  $self->output->{$name} = $input
-    unless grep { !defined($_) || !length($_) } @input;
-
-  return $self->topic($name);
-}
+sub optional { shift->_copy(shift, 0) }
 
 sub param { shift->every_param(shift)->[-1] }
 
 sub passed { [sort keys %{shift->output}] }
 
 sub required {
   my ($self, $name) = @_;
-  return $self if $self->optional($name)->is_valid;
+  return $self if $self->_copy($name, 1)->is_valid;
   return $self->error($name => ['required']);
 }
 
+sub _copy {
+  my ($self, $name, $all) = @_;
+
+  my $input  = $self->input->{$name};
+  my @input  = ref $input eq 'ARRAY' ? @$input : $input;
+  my @output = grep { defined && length } @input;
+
+  if ($all && @output && @input == @output) { $self->output->{$name} = $input }
+  elsif (!$all && @output > 1) { $self->output->{$name} = \@output }
+  elsif (!$all && @output)     { $self->output->{$name} = $output[0] }
+
+  return $self->topic($name);
+}
+
 1;
 
 =encoding utf8

diff --git a/t/mojolicious/validation_lite_app.t b/t/mojolicious/validation_lite_app.t
@@ -157,10 +157,13 @@ is_deeply $validation->failed, ['bar'], 'right names';
 # Multiple empty values
 $validation = $t->app->validation;
 ok !$validation->has_data, 'no data';
-$validation->input({foo => ['', 'bar', '']});
+$validation->input({foo => ['', 'bar', ''], bar => ['', 'baz', '']});
 ok $validation->has_data, 'has data';
+ok $validation->optional('bar')->is_valid, 'valid';
+is_deeply $validation->output, {bar => 'baz'}, 'right result';
+ok !$validation->has_error, 'no error';
 ok !$validation->required('foo')->is_valid, 'not valid';
-is_deeply $validation->output, {}, 'right result';
+is_deeply $validation->output, {bar => 'baz'}, 'right result';
 ok $validation->has_error, 'has error';
 is_deeply $validation->error('foo'), ['required'], 'right error';
 
@@ -223,7 +226,7 @@ $t->get_ok('/' => form => {foo => '☃☃'})->status_is(200)
   ->element_exists('input[type="password"]');
 
 # Validation failed for required fields
-$t->post_ok('/' => form => {foo => 'no'})->status_is(200)
+$t->post_ok('/' => form => {foo => ['no']})->status_is(200)
   ->text_is('div:root'                                 => 'in 1')
   ->text_is('label.custom.field-with-error[for="foo"]' => '<Foo>')
   ->element_exists('input.custom.field-with-error[type="text"][value="no"]')