Bug 2072313 - Accept query-string params on Reminders add

Bugzilla/API/V1/Reminders.pm

@@ -9,10 +9,10 @@ package Bugzilla::API::V1::Reminders;
 
 use 5.10.1;
 use Mojo::Base qw( Mojolicious::Controller );
-use Mojo::JSON qw( decode_json );
 
 use Bugzilla::Constants;
 use Bugzilla::Reminder;
+use Bugzilla::WebService::Util qw(merge_request_params);
 
 use Try::Tiny;
 
@@ -63,13 +63,8 @@ sub add {
   return $self->render(json => {})
     if !$user->in_group(Bugzilla->params->{reminders_group});
 
-  my $params = {};
-  try {
-    $params = decode_json($self->req->body);
-  }
-  catch {
-    return $self->user_error('rest_malformed_json');
-  };
+  my ($params, $error) = merge_request_params($self);
+  return $self->user_error($error) if $error;
 
   my $bug_id      = $params->{bug_id};
   my $note        = $params->{note};

Bugzilla/WebService/Util.pm

@@ -21,6 +21,8 @@ use Storable qw(dclone);
 use URI::Escape qw(uri_unescape);
 use Type::Params qw( compile );
 use Types::Standard -all;
+use Mojo::JSON qw(decode_json);
+use Try::Tiny;
 
 use base qw(Exporter);
 
@@ -38,6 +40,7 @@ our @EXPORT_OK = qw(
   params_to_objects
   fix_credentials
   set_rest_cors_headers
+  merge_request_params
 );
 
 sub set_rest_cors_headers {
@@ -297,6 +300,38 @@ sub params_to_objects {
   return \@objects;
 }
 
+sub merge_request_params {
+  my ($c) = @_;
+
+  # $c->req->params already covers the query string plus, for POST/PUT, an
+  # application/x-www-form-urlencoded or multipart body. Layer a JSON body
+  # underneath that, so params work from either the query string or a JSON
+  # request body. Query-string values win on a key collision, matching the
+  # legacy REST layer (see fix_credentials/_retrieve_json_params in
+  # Bugzilla::WebService::Server::REST) and the documented behavior in
+  # docs/en/rst/api/core/v1/general.rst.
+  #
+  # ->to_hash would turn a repeated key (e.g. ?note=a&note=b) into an
+  # arrayref, which validators don't expect, so collapse to scalars instead.
+  my $params = {};
+  $params->{$_} = $c->req->param($_) for @{$c->req->params->names};
+
+  # Only decode a body that wasn't already parsed as form params, otherwise a
+  # form-urlencoded or multipart request would be rejected as malformed JSON.
+  # The legacy REST layer gets this for free: CGI.pm only populates
+  # POSTDATA/PUTDATA for non-form content types.
+  if (length $c->req->body && !@{$c->req->body_params->names}) {
+    my $body_params;
+    my $error;
+    try { $body_params = decode_json($c->req->body); }
+    catch { $error = 'rest_malformed_json'; };
+    return (undef, $error) if $error;
+    $params = {%$body_params, %$params} if ref $body_params eq 'HASH';
+  }
+
+  return ($params, undef);
+}
+
 sub fix_credentials {
   my ($params, $cgi) = @_;
 
@@ -399,6 +434,19 @@ Helps make life simpler for WebService methods that internally create objects
 via both "ids" and "names" fields. Also de-duplicates objects that were loaded
 by both "ids" and "names". Returns an arrayref of objects.
 
+=head2 merge_request_params
+
+Takes a Mojolicious controller and returns a two-element list
+C<($params, $error)>, merging its query string/form-body params with a
+decoded JSON request body, if any. Query-string/form-body values win on a
+key collision. For use by native Mojo REST controllers that need to accept
+parameters from either the query string or a JSON body on non-GET
+requests.
+
+If the request has a non-empty body that fails to decode as JSON, C<$params>
+is C<undef> and C<$error> is set to C<rest_malformed_json>; callers should
+pass it to C<user_error>. Otherwise C<$error> is C<undef>.
+
 =head2 fix_credentials
 
 Allows for certain parameters related to authentication such as Bugzilla_login,

qa/t/rest_reminders.t

@@ -75,6 +75,40 @@ $t->delete_ok($url . "rest/reminder/invalid" => {'X-Bugzilla-API-Key' => $api_ke
 $t->delete_ok($url . "rest/reminder/$id" => {'X-Bugzilla-API-Key' => $api_key})
   ->status_is(200)->json_is('/success' => 1);
 
+### Section 3b: Fields may also be passed entirely via the query string,
+### with no JSON body
+
+$t->post_ok($url
+    . "rest/reminder?bug_id=$bug_id&note=Query%20String%20Reminder"
+    . '&reminder_ts=2024-06-08' => {'X-Bugzilla-API-Key' => $api_key})
+  ->status_is(200)->json_is('/note' => 'Query String Reminder');
+
+my $qs_id = $t->tx->res->json->{id};
+
+$t->delete_ok(
+  $url . "rest/reminder/$qs_id" => {'X-Bugzilla-API-Key' => $api_key})
+  ->status_is(200)->json_is('/success' => 1);
+
+### Section 3b-bis: A form-urlencoded body is accepted, not treated as JSON
+
+$t->post_ok($url
+    . 'rest/reminder' => {'X-Bugzilla-API-Key' => $api_key} => form =>
+    {bug_id => $bug_id, note => 'Form Reminder', reminder_ts => '2024-06-08'})
+  ->status_is(200)->json_is('/note' => 'Form Reminder');
+
+my $form_id = $t->tx->res->json->{id};
+
+$t->delete_ok(
+  $url . "rest/reminder/$form_id" => {'X-Bugzilla-API-Key' => $api_key})
+  ->status_is(200)->json_is('/success' => 1);
+
+### Section 3c: A malformed JSON body is rejected
+
+$t->post_ok($url
+    . 'rest/reminder' => {'X-Bugzilla-API-Key' => $api_key} => '{"bug_id": ')
+  ->status_is(400)->json_is('/code' => 32000)
+  ->json_like('/message' => qr/JSON data used for the request was malformed/);
+
 ### Section 4: Another user cannot delete someone else's reminder
 
 # Create a new reminder as userA (editbugs_user)