1232203 - Add extensive parameter validation to all external APIs

Status: NEW

(Bugzilla :: WebService, defect)

Description

[Dylan Hardison [:dylan] (he/him)]

Bug #1230932 comment 20, Frédéric Buclin notes:
> All the WS methods are very lenient about input data. They simply make sure 
> that mandatory parameters are defined, but they do not check that parameters 
> (both mandatory and optional) are of the expected format (i.e. strings or 
> integers, most of the time). This is out of the scope of this bug, but we 
> should really improve validators in Bugzilla/WebService/*.pm. That's IMO not 
> the job of Bugzilla/*.pm to do these checks as they are supposed to be used 
> internally only, where the data format is not controlled by users. So it's the 
> job of the API to pass correctly formatted data to internal methods.

As those changes were outside the scope of that bug, they are within the scope of this bug.

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Additional problems with param validation: rest/bug/update takes a comment: { body: "foo" } 
and if you pass a comment: "foo" it dies with a HASH error.

Comment 2

[Frédéric Buclin]

For instance, this call works, despite 'names' should be an array of strings only:

  my $call = rpc_call('Bug.fields', { names => [{ id => 8 }] });

because Bug.fields does:

  my $names = $params->{names};
  foreach my $field_name (@$names) {
      my $loop_field = Bugzilla::Field->check($field_name);

and so you can pass { id => 8 } directly to check(). Fortunately, passing 'id' is safe, because Bugzilla::Object->check will make sure that this parameter is an integer.


If you try to pass:

  my $call = rpc_call('Bug.fields', { names => [{ conditions => ..., values => ... }] });

then Bug.fields happily accepts it (oops!), but fortunately Bugzilla::Object->check() will refuse it because it wants either 'id' or 'name' to be defined, and so the conditions/values code is never reached. But this is a weak and dangerous protection. The API should have caught this illegal input earlier.