Conversation
create/update only read from the JSON body. Adds the query-string merge the docs promise, reusing the same merge_request_params helper as bugs 2065171/2065173. update() now whitelists fields before set_all(), since form-urlencoded cookie-auth requests (which include Bugzilla_api_token) previously failed JSON parsing before reaching it and no longer will.
| # 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; |
There was a problem hiding this comment.
req->params is body_params->clone->append(query_params), and Mojo::Parameters::to_hash turns a repeated key into an arrayref instead of letting one side win. so a form-urlencoded body description=B plus ?description=A gives description => ['B','A'], which set_description stringifies to ARRAY(0x...) in the db -- the "query string wins" claim in the comment above doesn't hold, and the same applies to any repeated query param. the legacy _retrieve_json_params avoids this by assigning url_param values one at a time. suggest layering each source explicitly, e.g. json body, then $c->req->body_params->to_hash, then $c->req->query_params->to_hash
| if (length $c->req->body) { | ||
| my $body_params; | ||
| try { $body_params = decode_json($c->req->body); } | ||
| catch { $body_params = undef; }; |
There was a problem hiding this comment.
swallowing the decode error drops the rest_malformed_json response _get_params used to return. a PUT with a broken json body now falls through to set_all({}) and returns 200 with an unchanged component, which reads as success. Reminders.pm still throws rest_malformed_json for the same input, and the legacy layer cited above throws json_rpc_invalid_params, so this is a behaviour regression rather than a match. consider signalling the parse failure to the caller
| my %values = map { $_ => $params->{$_} } | ||
| grep { exists $params->{$_} } | ||
| qw(name description default_assignee default_qa_contact default_bug_type | ||
| is_active triage_owner team_name bug_description_template); |
There was a problem hiding this comment.
is_active is only a real boolean when it comes from a json body. from the query string it is the string "false", and isactive validates through Bugzilla::Object::check_boolean, which is $_[1] ? 1 : 0 -- so ?is_active=false activates the component, the opposite of the documented value in component.rst. the query-string path needs an explicit string-to-boolean coercion
This branch has not been deployed
2 participants
Summary
Component.pm'screate/updateonly read parameters from the JSON POST/PUT body. Adds the query-string merge that the REST docs promise (query string overrides the body on a key collision), reusing the samemerge_request_paramshelper as bugs 2065171/2065173, and fixes a stray-key crash inupdate()'sset_all()call that this change would otherwise expose.Changes
Bugzilla::WebService::Util::merge_request_params(third copy of this helper across open PRs -- will collapse to one once 2065171 or 2065173 merges)Component.pmcreate/update: query string + JSON body merge instead of JSON-body-only; drop the now-unused_get_paramsComponent.pmupdate(): whitelist the 8 documented update fields beforeset_all(), since form-urlencoded cookie-auth requests (which includeBugzilla_api_token) will now actually reach it instead of failing JSON parsing firstqa/t/rest_components.t: cover query-string-only create and query-string-overrides-body updateTest plan
References