From 05091f9d7bbb7ce6ce40e7646aef134d0551b18c Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 22:32:47 +0200 Subject: [PATCH 1/7] Bug 2072313 - Accept query-string params on Reminders add add() only read from the JSON body. Reuse the same merge_request_params helper as bugs 2065171/2065173/2072305 instead of writing another copy. No whitelist needed here: create() only reads three named keys, not the whole params hash. --- Bugzilla/API/V1/Reminders.pm | 10 ++-------- Bugzilla/WebService/Util.pm | 35 +++++++++++++++++++++++++++++++++++ qa/t/rest_reminders.t | 14 ++++++++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/Bugzilla/API/V1/Reminders.pm b/Bugzilla/API/V1/Reminders.pm index d5279089ff..82669d66c3 100644 --- a/Bugzilla/API/V1/Reminders.pm +++ b/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,7 @@ 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 = merge_request_params($self); my $bug_id = $params->{bug_id}; my $note = $params->{note}; diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 32b34a4bb0..f7ade2aecb 100644 --- a/Bugzilla/WebService/Util.pm +++ b/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,29 @@ 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 (silently ignored if absent or not valid JSON), 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. + my $params = $c->req->params->to_hash; + + if (length $c->req->body) { + my $body_params; + try { $body_params = decode_json($c->req->body); } + catch { $body_params = undef; }; + $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; + } + + return $params; +} + sub fix_credentials { my ($params, $cgi) = @_; @@ -399,6 +425,15 @@ 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 hashref merging its query +string/form-body params (C<< $c->req->params->to_hash >>) 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. + =head2 fix_credentials Allows for certain parameters related to authentication such as Bugzilla_login, diff --git a/qa/t/rest_reminders.t b/qa/t/rest_reminders.t index 9ac2de7cf5..e824bc4bc4 100644 --- a/qa/t/rest_reminders.t +++ b/qa/t/rest_reminders.t @@ -75,6 +75,20 @@ $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¬e=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 4: Another user cannot delete someone else's reminder # Create a new reminder as userA (editbugs_user) From 8cfa8880dba8b3f09f70ae9c1bc8b3533ea90b1e Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 21 Sep 2026 16:39:52 +0200 Subject: [PATCH 2/7] Bug 2072313 - Collapse repeated query params to scalars and signal malformed JSON from merge_request_params --- Bugzilla/WebService/Util.pm | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index f7ade2aecb..09cfa2a425 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -305,22 +305,27 @@ sub merge_request_params { # $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 (silently ignored if absent or not valid JSON), 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 + # 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. - my $params = $c->req->params->to_hash; + # + # ->to_hash would turn a repeated key (e.g. ?note=a¬e=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}; if (length $c->req->body) { my $body_params; + my $error; try { $body_params = decode_json($c->req->body); } - catch { $body_params = undef; }; + catch { $error = 'rest_malformed_json'; }; + return (undef, $error) if $error; $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; } - return $params; + return ($params, undef); } sub fix_credentials { @@ -427,13 +432,17 @@ by both "ids" and "names". Returns an arrayref of objects. =head2 merge_request_params -Takes a Mojolicious controller and returns a hashref merging its query -string/form-body params (C<< $c->req->params->to_hash >>) 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 +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 and C<$error> is set to C; callers should +pass it to C. Otherwise C<$error> is C. + =head2 fix_credentials Allows for certain parameters related to authentication such as Bugzilla_login, From d394b244ffdb98dc7ba5b9e49c639e4c0ad082cc Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 21 Sep 2026 16:40:15 +0200 Subject: [PATCH 3/7] Bug 2072313 - Return rest_malformed_json from Reminders add() on decode failure --- Bugzilla/API/V1/Reminders.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Bugzilla/API/V1/Reminders.pm b/Bugzilla/API/V1/Reminders.pm index 82669d66c3..0f86e3c59d 100644 --- a/Bugzilla/API/V1/Reminders.pm +++ b/Bugzilla/API/V1/Reminders.pm @@ -63,7 +63,8 @@ sub add { return $self->render(json => {}) if !$user->in_group(Bugzilla->params->{reminders_group}); - my $params = merge_request_params($self); + my ($params, $error) = merge_request_params($self); + return $self->user_error($error) if $error; my $bug_id = $params->{bug_id}; my $note = $params->{note}; From cf76e16595cb45f2e22d35a04827343374625552 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 21 Sep 2026 16:40:31 +0200 Subject: [PATCH 4/7] Bug 2072313 - Add test coverage for malformed JSON reminder body --- qa/t/rest_reminders.t | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/qa/t/rest_reminders.t b/qa/t/rest_reminders.t index e824bc4bc4..8af33275e7 100644 --- a/qa/t/rest_reminders.t +++ b/qa/t/rest_reminders.t @@ -89,6 +89,14 @@ $t->delete_ok( $url . "rest/reminder/$qs_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_is('/message' => + 'The JSON data used for the request was malformed. Please update your request and try again.' + ); + ### Section 4: Another user cannot delete someone else's reminder # Create a new reminder as userA (editbugs_user) From 07d2acb69ef232eef561a8b52c5427006eeb5ffb Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 21 Sep 2026 17:14:35 +0200 Subject: [PATCH 5/7] Bug 2072313 - Recognize USAGE_MODE_MOJO_REST as a webservice mode in i_am_webservice --- Bugzilla/Util.pm | 4 +++- qa/t/rest_reminders.t | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 7c3e4f8928..d9570aba05 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -298,7 +298,9 @@ sub i_am_cgi { sub i_am_webservice { my $usage_mode = Bugzilla->usage_mode; - return $usage_mode == USAGE_MODE_JSON || $usage_mode == USAGE_MODE_REST; + return $usage_mode == USAGE_MODE_JSON + || $usage_mode == USAGE_MODE_REST + || $usage_mode == USAGE_MODE_MOJO_REST; } sub is_webserver_group { diff --git a/qa/t/rest_reminders.t b/qa/t/rest_reminders.t index 8af33275e7..aa3bea7091 100644 --- a/qa/t/rest_reminders.t +++ b/qa/t/rest_reminders.t @@ -90,6 +90,9 @@ $t->delete_ok( ->status_is(200)->json_is('/success' => 1); ### Section 3c: A malformed JSON body is rejected +### +### Also guards i_am_webservice() recognizing USAGE_MODE_MOJO_REST: without +### that, this message gets word-wrapped to 72 columns with embedded \n's. $t->post_ok($url . 'rest/reminder' => {'X-Bugzilla-API-Key' => $api_key} => '{"bug_id": ') From 76ede6f4b568617b05067074e7244008518ff2dc Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 15:16:55 +0200 Subject: [PATCH 6/7] Bug 2072313 - Only decode a JSON request body when it was not parsed as form params --- Bugzilla/WebService/Util.pm | 6 +++++- qa/t/rest_reminders.t | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 09cfa2a425..0a5e94a98c 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -316,7 +316,11 @@ sub merge_request_params { my $params = {}; $params->{$_} = $c->req->param($_) for @{$c->req->params->names}; - if (length $c->req->body) { + # 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); } diff --git a/qa/t/rest_reminders.t b/qa/t/rest_reminders.t index aa3bea7091..8f5e9aedcf 100644 --- a/qa/t/rest_reminders.t +++ b/qa/t/rest_reminders.t @@ -89,6 +89,19 @@ $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 ### ### Also guards i_am_webservice() recognizing USAGE_MODE_MOJO_REST: without From 0f85fa29d785d063863a653ee6f836e6ebe6041d Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 15:17:42 +0200 Subject: [PATCH 7/7] Bug 2072313 - Drop the i_am_webservice change and relax the malformed JSON assertion --- Bugzilla/Util.pm | 4 +--- qa/t/rest_reminders.t | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index d9570aba05..7c3e4f8928 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -298,9 +298,7 @@ sub i_am_cgi { sub i_am_webservice { my $usage_mode = Bugzilla->usage_mode; - return $usage_mode == USAGE_MODE_JSON - || $usage_mode == USAGE_MODE_REST - || $usage_mode == USAGE_MODE_MOJO_REST; + return $usage_mode == USAGE_MODE_JSON || $usage_mode == USAGE_MODE_REST; } sub is_webserver_group { diff --git a/qa/t/rest_reminders.t b/qa/t/rest_reminders.t index 8f5e9aedcf..227df868db 100644 --- a/qa/t/rest_reminders.t +++ b/qa/t/rest_reminders.t @@ -103,15 +103,11 @@ $t->delete_ok( ->status_is(200)->json_is('/success' => 1); ### Section 3c: A malformed JSON body is rejected -### -### Also guards i_am_webservice() recognizing USAGE_MODE_MOJO_REST: without -### that, this message gets word-wrapped to 72 columns with embedded \n's. $t->post_ok($url . 'rest/reminder' => {'X-Bugzilla-API-Key' => $api_key} => '{"bug_id": ') - ->status_is(400)->json_is('/code' => 32000)->json_is('/message' => - 'The JSON data used for the request was malformed. Please update your request and try again.' - ); + ->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