Bug 473646 - WebService methods should check list parameters for scalars and convert before use

Patch by Dave Lawrence <dkl@redhat.com> - r/a=mkanat

4 files changed, 37 insertions, 8 deletions

diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 1c0df32ac..21645af3d 100755
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -27,7 +27,7 @@ use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Field;
 use Bugzilla::WebService::Constants;
-use Bugzilla::WebService::Util qw(filter);
+use Bugzilla::WebService::Util qw(filter validate);
 use Bugzilla::Bug;
 use Bugzilla::BugMail;
 use Bugzilla::Util qw(trim);
@@ -67,7 +67,8 @@ BEGIN { *get_bugs = \&get }
 ###########
 
 sub comments {
-    my ($self, $params) = @_;
+    my ($self, $params) = validate(@_, 'bug_ids', 'comment_ids');
+
     if (!(defined $params->{bug_ids} || defined $params->{comment_ids})) {
         ThrowCodeError('params_required',
                        { function => 'Bug.comments',
@@ -145,7 +146,8 @@ sub _translate_comment {
 }
 
 sub get {
-    my ($self, $params) = @_;
+    my ($self, $params) = validate(@_, 'ids');
+
     my $ids = $params->{ids};
     defined $ids || ThrowCodeError('param_required', { param => 'ids' });
 
@@ -162,7 +164,7 @@ sub get {
 # it can be called as the following:
 # $call = $rpc->call( 'Bug.get_history', { ids => [1,2] });
 sub get_history {
-    my ($self, $params) = @_;
+    my ($self, $params) = validate(@_, 'ids');
 
     my $ids = $params->{ids};
     defined $ids || ThrowCodeError('param_required', { param => 'ids' });
diff --git a/Bugzilla/WebService/Product.pm b/Bugzilla/WebService/Product.pm
index 4dd894453..eaec012a4 100755
--- a/Bugzilla/WebService/Product.pm
+++ b/Bugzilla/WebService/Product.pm
@@ -21,6 +21,7 @@ use strict;
 use base qw(Bugzilla::WebService);
 use Bugzilla::Product;
 use Bugzilla::User;
+use Bugzilla::WebService::Util qw(validate);
 
 ##################################################
 # Add aliases here for method name compatibility #
@@ -45,7 +46,7 @@ sub get_accessible_products {
 
 # Get a list of actual products, based on list of ids
 sub get {
-    my ($self, $params) = @_;
+    my ($self, $params) = validate(@_, 'ids');
     
     # Only products that are in the users accessible products, 
     # can be allowed to be returned
diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm
index 6283f55a1..790a9da7c 100755
--- a/Bugzilla/WebService/User.pm
+++ b/Bugzilla/WebService/User.pm
@@ -28,7 +28,7 @@ use Bugzilla::Error;
 use Bugzilla::User;
 use Bugzilla::Util qw(trim);
 use Bugzilla::Token;
-use Bugzilla::WebService::Util qw(filter);
+use Bugzilla::WebService::Util qw(filter validate);
 
 # Don't need auth to login
 use constant LOGIN_EXEMPT => {
@@ -131,7 +131,7 @@ sub create {
 # $call = $rpc->call( 'User.get', { ids => [1,2,3], 
 #         names => ['testusera@redhat.com', 'testuserb@redhat.com'] });
 sub get {
-    my ($self, $params) = @_;
+    my ($self, $params) = validate(@_, 'names', 'ids');
 
     my @user_objects;
     @user_objects = map { Bugzilla::User->check($_) } @{ $params->{names} }
diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm
index cd75bee8c..74c1f2f02 100644
--- a/Bugzilla/WebService/Util.pm
+++ b/Bugzilla/WebService/Util.pm
@@ -24,7 +24,7 @@ use strict;
 
 use base qw(Exporter);
 
-our @EXPORT_OK = qw(filter);
+our @EXPORT_OK = qw(filter validate);
 
 sub filter ($$) {
     my ($params, $hash) = @_;
@@ -44,6 +44,23 @@ sub filter ($$) {
     return \%newhash;
 }
 
+sub validate  {
+    my ($self, $params, @keys) = @_;
+    
+    # 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} ];
+        }
+    }
+
+    return ($self, $params);
+}
+
 __END__
 
 =head1 NAME
@@ -61,6 +78,8 @@ internally in the WebService code.
  filter({ include_fields => ['id', 'name'], 
           exclude_fields => ['name'] }, $hash);
 
+ validate(@_, 'ids');
+
 =head1 METHODS
 
 =over
@@ -72,4 +91,11 @@ of WebService methods. Given a hash (the second argument to this subroutine),
 this will remove any keys that are I<not> in C<include_fields> and then remove
 any keys that I<are> in C<exclude_fields>.
 
+=item C<validate>
+
+This helps in the validation of parameters passed into the WebSerice
+methods. Currently it converts listed parameters into an array reference
+if the client only passed a single scalar value. It modifies the parameters
+hash in place so other parameters should be unaltered.
+
 =back