Bug 1497343 - Add some rudimentary type checking to Bugzilla::WebServe::Util::validate()

3 files changed, 24 insertions, 7 deletions

diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 9003f3480..61a95e07d 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -1462,7 +1462,8 @@ sub _bug_to_hash {
         elsif ($field->type == FIELD_TYPE_DATETIME
                || $field->type == FIELD_TYPE_DATE)
         {
-            $item{$name} = $self->type('dateTime', $bug->$name);
+            my $value = $bug->$name;
+            $item{$name} = defined($value) ? $self->type('dateTime', $value) : undef;
         }
         elsif ($field->type == FIELD_TYPE_MULTI_SELECT) {
             my @values = map { $self->type('string', $_) } @{ $bug->$name };
diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm
index d462c884a..ce5586911 100644
--- a/Bugzilla/WebService/Util.pm
+++ b/Bugzilla/WebService/Util.pm
@@ -11,6 +11,7 @@ use 5.10.1;
 use strict;
 use warnings;
 
+use Bugzilla::Logging;
 use Bugzilla::Flag;
 use Bugzilla::FlagType;
 use Bugzilla::Error;
@@ -18,6 +19,8 @@ use Bugzilla::WebService::Constants;
 
 use Storable qw(dclone);
 use URI::Escape qw(uri_unescape);
+use Type::Params qw( compile );
+use Types::Standard -all;
 
 use base qw(Exporter);
 
@@ -217,6 +220,17 @@ sub _delete_bad_keys {
 
 sub validate  {
     my ($self, $params, @keys) = @_;
+    my $cache_key = join('|', (caller(1))[3], sort @keys);
+    # Type->of() is the same as Type[], used here because it is easier
+    # to chain with plus_coercions.
+    state $array_of_nonrefs = ArrayRef->of(Maybe[Value])->plus_coercions(
+        Maybe[Value], q{ [ $_ ] },
+    );
+    state $type_cache = {};
+    my $params_type = $type_cache->{$cache_key} //= do {
+        my %fields = map { $_ => Optional[$array_of_nonrefs] } @keys;
+        Maybe[ Dict[%fields, slurpy Any] ];
+    };
 
     # If $params is defined but not a reference, then we weren't
     # sent any parameters at all, and we're getting @keys where
@@ -226,12 +240,10 @@ sub validate  {
     # If @keys is not empty then we convert any named
     # parameters that have scalar values to arrayrefs
     # that match.
-    foreach my $key (@keys) {
-        if (exists $params->{$key}) {
-            $params->{$key} = ref $params->{$key}
-                              ? $params->{$key}
-                              : [ $params->{$key} ];
-        }
+    $params = $params_type->coerce($params);
+    if (my $type_error = $params_type->validate($params)) {
+        FATAL("validate() found type error: $type_error");
+        ThrowUserError('invalid_params', { type_error => $type_error } ) if $type_error;
     }
 
     return ($self, $params);
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 9b0583009..53de6a420 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1037,6 +1037,10 @@
     [% title = "Invalid Authentication Method" %]
     API key authentication is required.
 
+  [% ELSIF error == "invalid_params" %]
+    [% title = "Invalid Params" %]
+    API method received invalid params: [% type_error FILTER html %]
+
   [% ELSIF error == "bug_id_does_not_exist" %]
     [% title = BLOCK %]Invalid [% terms.Bug %] ID[% END %]
     [% terms.Bug %] [%= bug_id FILTER html %] does not exist.