Bug 471942: Make the WebService validate and properly convert input parameters

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=dkl, a=mkanat

diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm
index 615abf68cd4266635996aee47bc55052c04a9ced..3152ef04f5bffb1376995b8df475cd092d7f4884 100755 (executable)
--- a/Bugzilla/WebService.pm
+++ b/Bugzilla/WebService.pm
@@ -97,6 +97,7 @@ sub initialize {
     my $self = shift;
     my %retval = $self->SUPER::initialize(@_);
     $retval{'serializer'} = Bugzilla::WebService::XMLRPC::Serializer->new;
+    $retval{'deserializer'} = Bugzilla::WebService::XMLRPC::Deserializer->new;
     return %retval;
 }
 
@@ -114,6 +115,74 @@ sub make_response {
 
 1;
 
+# This exists to validate input parameters (which XMLRPC::Lite doesn't do)
+# and also, in some cases, to more-usefully decode them.
+package Bugzilla::WebService::XMLRPC::Deserializer;
+use strict;
+# We can't use "use base" because XMLRPC::Serializer doesn't return
+# a true value.
+eval { require XMLRPC::Lite; };
+our @ISA = qw(XMLRPC::Deserializer);
+
+use Bugzilla::Error;
+
+# Some method arguments need to be converted in some way, when they are input.
+sub decode_value {
+    my $self = shift;
+    my ($type) = @{ $_[0] };
+    my $value = $self->SUPER::decode_value(@_);
+    
+    # We only validate/convert certain types here.
+    return $value if $type !~ /^(?:int|i4|boolean|double|dateTime\.iso8601)$/;
+    
+    # Though the XML-RPC standard doesn't allow an empty <int>,
+    # <double>,or <dateTime.iso8601>,  we do, and we just say
+    # "that's undef".
+    if (grep($type eq $_, qw(int double dateTime))) {
+        return undef if $value eq '';
+    }
+    
+    my $validator = $self->_validation_subs->{$type};
+    if (!$validator->($value)) {
+        ThrowUserError('xmlrpc_invalid_value',
+                       { type => $type, value => $value });
+    }
+    
+    # We convert dateTimes to a DB-friendly date format.
+    if ($type eq 'dateTime.iso8601') {
+        # We leave off the $ from the end of this regex to allow for possible
+        # extensions to the XML-RPC date standard.
+        $value =~ /^(\d{4})(\d{2})(\d{2})T(\d{2}):(\d{2}):(\d{2})/;
+        $value = "$1-$2-$3 $4:$5:$6";
+    }
+
+    return $value;
+}
+
+sub _validation_subs {
+    my $self = shift;
+    return $self->{_validation_subs} if $self->{_validation_subs};
+    # The only place that XMLRPC::Lite stores any sort of validation
+    # regex is in XMLRPC::Serializer. We want to re-use those regexes here.
+    my $lookup = Bugzilla::WebService::XMLRPC::Serializer->new->typelookup;
+    
+    # $lookup is a hash whose values are arrayrefs, and whose keys are the
+    # names of types. The second item of each arrayref is a subroutine
+    # that will do our validation for us.
+    my %validators = map { $_ => $lookup->{$_}->[1] } (keys %$lookup);
+    # Add a boolean validator
+    $validators{'boolean'} = sub {$_[0] =~ /^[01]$/};
+    # Some types have multiple names, or have a different name in
+    # XMLRPC::Serializer than their standard XML-RPC name.
+    $validators{'dateTime.iso8601'} = $validators{'dateTime'};
+    $validators{'i4'} = $validators{'int'};
+    
+    $self->{_validation_subs} = \%validators;
+    return \%validators;
+}
+
+1;
+
 # This package exists to fix a UTF-8 bug in SOAP::Lite.
 # See http://rt.cpan.org/Public/Bug/Display.html?id=32952.
 package Bugzilla::WebService::XMLRPC::Serializer;
@@ -356,3 +425,20 @@ would return something like:
 
 =back
 
+
+=head1 EXTENSIONS TO THE XML-RPC STANDARD
+
+=head2 Undefined Values
+
+Normally, XML-RPC does not allow empty values for C<int>, C<double>, or
+C<dateTime.iso8601> fields. Bugzilla does--it treats empty values as
+C<undef> (called C<NULL> or C<None> in some programming languages).
+
+Bugzilla also accepts a type called C<< <nil> >>, which is always considered
+to be C<undef>, no matter what it contains.
+
+=begin private
+
+nil is implemented by XMLRPC::Lite, in XMLRPC::Deserializer::decode_value.
+
+=end private

diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index 07b51e5f1bfe68ecd0cab276b760e4b6c6e01ebd..4f98cd11af760e37d955352ea3964d2a289cb1cb 100755 (executable)
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -48,11 +48,12 @@ use base qw(Exporter);
 # comment that it was retired. Also, if an error changes its name, you'll
 # have to fix it here.
 use constant WS_ERROR_CODE => {
-    # Generic Bugzilla::Object errors are 50-99.
+    # Generic errors (Bugzilla::Object and others) are 50-99.    
     object_not_specified        => 50,
     param_required              => 50,
     params_required             => 50,
     object_does_not_exist       => 51,
+    xmlrpc_invalid_value        => 52,
     # Bug errors usually occupy the 100-200 range.
     improper_bug_id_field_value => 100,
     bug_id_does_not_exist       => 101,

diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index d92b046227a7eea724c223ae8fc52b071ba0ed37..39077c542ec78dc2bdabae9e2f13017ad5310ad2 100644 (file)
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1618,6 +1618,11 @@
     [% title = "Wrong Token" %]
     That token cannot be used to create a user account.
 
+  [% ELSIF error == "xmlrpc_invalid_value" %]
+    "[% value FILTER html %]" is not a valid value for a
+    <[% type FILTER html %]> field. (See the XML-RPC specification
+    for details.)
+
   [% ELSIF error == "zero_length_file" %]
     [% title = "File Is Empty" %]
     The file you are trying to attach is empty, does not exist, or you don't