From 92355bb7035ae5c58315e7ab87f9df5b3a28fbd8 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 14 Sep 2026 15:37:01 +0200 Subject: [PATCH 01/16] Bug 2065171 - Migrate BugUserLastVisit REST resource to native Mojo API --- Bugzilla/API/V1/BugUserLastVisit.pm | 174 +++++++++++++++ Bugzilla/WebService/BugUserLastVisit.pm | 206 ------------------ Bugzilla/WebService/Constants.pm | 1 - Bugzilla/WebService/Server/REST.pm | 1 - .../Server/REST/Resources/BugUserLastVisit.pm | 57 ----- 5 files changed, 174 insertions(+), 265 deletions(-) create mode 100644 Bugzilla/API/V1/BugUserLastVisit.pm delete mode 100644 Bugzilla/WebService/BugUserLastVisit.pm delete mode 100644 Bugzilla/WebService/Server/REST/Resources/BugUserLastVisit.pm diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm new file mode 100644 index 0000000000..6483561d3b --- /dev/null +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -0,0 +1,174 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::API::V1::BugUserLastVisit; + +use 5.10.1; +use Mojo::Base qw( Mojolicious::Controller ); + +use Mojo::JSON qw(decode_json); +use Try::Tiny; + +use Bugzilla::Bug; +use Bugzilla::Constants; +use Bugzilla::Util qw(datetime_from); +use Bugzilla::WebService::Util qw(filter); + +sub setup_routes { + my ($class, $r) = @_; + my $routes = $r->under( + '/bug_user_last_visit' => sub { Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); }); + $routes->get('/')->to('V1::BugUserLastVisit#get'); + $routes->get('/:id')->to('V1::BugUserLastVisit#get'); + $routes->post('/')->to('V1::BugUserLastVisit#update'); + $routes->post('/:id')->to('V1::BugUserLastVisit#update'); + + foreach my $path ('/', '/:id') { + $routes->options($path)->to('V1::BugUserLastVisit#options'); + } +} + +sub options { + my ($self) = @_; + + $self->res->headers->header('Allow' => 'GET, POST'); + $self->res->headers->header('Access-Control-Allow-Methods' => 'GET, POST'); + + return $self->rendered(200); +} + +sub get { + my ($self) = @_; + + my $user = $self->bugzilla->login; + $user->id || return $self->user_error('login_required'); + + my ($ids) = $self->_ids_from_request; + + if ($ids) { + + # Cache permissions for bugs. This highly reduces the number of calls to + # the DB. visible_bugs() is only able to handle bug IDs, so we have to + # skip aliases. + $user->visible_bugs([grep {/^[0-9]+$/} @$ids]); + } + + my @last_visits = @{$user->last_visited}; + + if ($ids) { + + # remove bugs that we are not interested in if ids is passed in. + my %id_set = map { ($_ => 1) } @$ids; + @last_visits = grep { $id_set{$_->bug_id} } @last_visits; + } + + my $params = $self->_filter_params; + + return $self->render( + json => [ + map { + $self->_bug_user_last_visit_to_hash($_->bug_id, $_->last_visit_ts, $params) + } @last_visits + ] + ); +} + +sub update { + my ($self) = @_; + + my $user = $self->bugzilla->login; + $user->id || return $self->user_error('login_required'); + + my ($ids, $error) = $self->_ids_from_request; + return $self->user_error($error) if $error; + return $self->code_error('param_required', {param => 'ids'}) + unless $ids && @$ids; + + # Cache permissions for bugs. This highly reduces the number of calls to the + # DB. visible_bugs() is only able to handle bug IDs, so we have to skip + # aliases. + $user->visible_bugs([grep {/^[0-9]+$/} @$ids]); + + my $params = $self->_filter_params; + my $dbh = Bugzilla->dbh; + + $dbh->bz_start_transaction(); + my @results; + my $last_visit_ts = $dbh->selectrow_array('SELECT NOW()'); + foreach my $bug_id (@$ids) { + my $bug = Bugzilla::Bug->check({id => $bug_id, cache => 1}); + + next unless $user->can_see_bug($bug->id); + + $bug->update_user_last_visit($user, $last_visit_ts); + + push(@results, + $self->_bug_user_last_visit_to_hash($bug_id, $last_visit_ts, $params)); + } + $dbh->bz_commit_transaction(); + + return $self->render(json => \@results); +} + +sub _ids_from_request { + my ($self) = @_; + + if (my $id = $self->param('id')) { + return [$id]; + } + + if ($self->req->method eq 'POST') { + my $params; + my $error; + try { $params = decode_json($self->req->body || '{}'); } + catch { $error = 'rest_malformed_json'; }; + return (undef, $error) if $error; + my $ids = $params->{ids} // []; + return ref $ids ? $ids : [$ids]; + } + + my $ids = $self->every_param('ids'); + return @$ids ? $ids : undef; +} + +sub _filter_params { + my ($self) = @_; + + my $params = $self->req->params->to_hash; + for my $field (qw(include_fields exclude_fields)) { + $params->{$field} = [split(/[\s,]+/, $params->{$field})] + if exists $params->{$field} && !ref $params->{$field}; + } + + return $params; +} + +sub _bug_user_last_visit_to_hash { + my ($self, $bug_id, $last_visit_ts, $params) = @_; + + return filter( + $params, + { + id => 0 + $bug_id, + last_visit_ts => datetime_from($last_visit_ts, 'UTC')->iso8601() . 'Z', + } + ); +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::API::V1::BugUserLastVisit - Find and Store the last time a user +visited a bug. + +=head1 DESCRIPTION + +This part of the Bugzilla REST API allows you to lookup and update the last +time a user visited a bug. diff --git a/Bugzilla/WebService/BugUserLastVisit.pm b/Bugzilla/WebService/BugUserLastVisit.pm deleted file mode 100644 index 1b07dbf102..0000000000 --- a/Bugzilla/WebService/BugUserLastVisit.pm +++ /dev/null @@ -1,206 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -# This Source Code Form is "Incompatible With Secondary Licenses", as -# defined by the Mozilla Public License, v. 2.0. - -package Bugzilla::WebService::BugUserLastVisit; - -use 5.10.1; -use strict; -use warnings; - -use base qw(Bugzilla::WebService); - -use Bugzilla::Bug; -use Bugzilla::Error; -use Bugzilla::WebService::Util qw( validate filter ); -use Bugzilla::Constants; - -use constant PUBLIC_METHODS => qw( - get - update -); - -sub update { - my ($self, $params) = validate(@_, 'ids'); - my $user = Bugzilla->user; - my $dbh = Bugzilla->dbh; - - $user->login(LOGIN_REQUIRED); - - my $ids = $params->{ids} // []; - ThrowCodeError('param_required', {param => 'ids'}) unless @$ids; - - # Cache permissions for bugs. This highly reduces the number of calls to the - # DB. visible_bugs() is only able to handle bug IDs, so we have to skip - # aliases. - $user->visible_bugs([grep /^[0-9]+$/, @$ids]); - - $dbh->bz_start_transaction(); - my @results; - my $last_visit_ts = $dbh->selectrow_array('SELECT NOW()'); - foreach my $bug_id (@$ids) { - my $bug = Bugzilla::Bug->check({id => $bug_id, cache => 1}); - - next unless $user->can_see_bug($bug->id); - - $bug->update_user_last_visit($user, $last_visit_ts); - - push(@results, - $self->_bug_user_last_visit_to_hash($bug_id, $last_visit_ts, $params)); - } - $dbh->bz_commit_transaction(); - - return \@results; -} - -sub get { - my ($self, $params) = validate(@_, 'ids'); - my $user = Bugzilla->user; - my $ids = $params->{ids}; - - $user->login(LOGIN_REQUIRED); - - if ($ids) { - - # Cache permissions for bugs. This highly reduces the number of calls to - # the DB. visible_bugs() is only able to handle bug IDs, so we have to - # skip aliases. - $user->visible_bugs([grep /^[0-9]+$/, @$ids]); - } - - my @last_visits = @{$user->last_visited}; - - if ($ids) { - - # remove bugs that we are not interested in if ids is passed in. - my %id_set = map { ($_ => 1) } @$ids; - @last_visits = grep { $id_set{$_->bug_id} } @last_visits; - } - - return [ - map { - $self->_bug_user_last_visit_to_hash($_->bug_id, $_->last_visit_ts, $params) - } @last_visits - ]; -} - -sub _bug_user_last_visit_to_hash { - my ($self, $bug_id, $last_visit_ts, $params) = @_; - - my %result = ( - id => $self->type('int', $bug_id), - last_visit_ts => $self->type('dateTime', $last_visit_ts) - ); - - return filter($params, \%result); -} - -1; - -__END__ -=head1 NAME - -Bugzilla::WebService::BugUserLastVisit - Find and Store the last time a user -visited a bug. - -=head1 METHODS - -See L for a description of how parameters are passed, -and what B, B, and B mean. - -Although the data input and output is the same for JSON-RPC and REST, -the directions for how to access the data via REST is noted in each method -where applicable. - -=head2 update - -B - -=over - -=item B - -Update the last visit time for the specified bug and current user. - -=item B - -To add a single bug id: - - POST /rest/bug_user_last_visit/ - -Tp add one or more bug ids at once: - - POST /rest/bug_user_last_visit - -The returned data format is the same as below. - -=item B - -=over - -=item C (array) - One or more bug ids to add. - -=back - -=item B - -=over - -=item C - An array of hashes containing the following: - -=over - -=item C - (int) The bug id. - -=item C - (string) The timestamp the user last visited the bug. - -=back - -=back - -=back - -=head2 get - -B - -=over - -=item B - -Get the last visited timestamp for one or more specified bug ids. - -=item B - -To return the last visited timestamp for a single bug id: - - GET /rest/bug_user_last_visit/ - -=item B - -=over - -=item C (integer) - One or more optional bug ids to get. - -=back - -=item B - -=over - -=item C - An array of hashes containing the following: - -=over - -=item C - (int) The bug id. - -=item C - (string) The timestamp the user last visited the bug. - -=back - -=back - -=back diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 1986c9a479..c0f22d8f6f 100644 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -318,7 +318,6 @@ sub WS_DISPATCH { 'User' => 'Bugzilla::WebService::User', 'Product' => 'Bugzilla::WebService::Product', 'Group' => 'Bugzilla::WebService::Group', - 'BugUserLastVisit' => 'Bugzilla::WebService::BugUserLastVisit', %hook_dispatch }; return $dispatch; diff --git a/Bugzilla/WebService/Server/REST.pm b/Bugzilla/WebService/Server/REST.pm index b9ae7b9a3e..5ad30efe09 100644 --- a/Bugzilla/WebService/Server/REST.pm +++ b/Bugzilla/WebService/Server/REST.pm @@ -27,7 +27,6 @@ use Bugzilla::WebService::Server::REST::Resources::Bugzilla; use Bugzilla::WebService::Server::REST::Resources::Group; use Bugzilla::WebService::Server::REST::Resources::Product; use Bugzilla::WebService::Server::REST::Resources::User; -use Bugzilla::WebService::Server::REST::Resources::BugUserLastVisit; use List::MoreUtils qw(uniq); use Scalar::Util qw(blessed reftype); diff --git a/Bugzilla/WebService/Server/REST/Resources/BugUserLastVisit.pm b/Bugzilla/WebService/Server/REST/Resources/BugUserLastVisit.pm deleted file mode 100644 index 72aa0d40f0..0000000000 --- a/Bugzilla/WebService/Server/REST/Resources/BugUserLastVisit.pm +++ /dev/null @@ -1,57 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -# This Source Code Form is "Incompatible With Secondary Licenses", as -# defined by the Mozilla Public License, v. 2.0. - -package Bugzilla::WebService::Server::REST::Resources::BugUserLastVisit; - -use 5.10.1; -use strict; -use warnings; - -BEGIN { - *Bugzilla::WebService::BugUserLastVisit::rest_resources = \&_rest_resources; -} - -sub _rest_resources { - return [ - # bug-id - qr{^/bug_user_last_visit/(\d+)$}, - { - GET => { - method => 'get', - params => sub { - return {ids => $_[0]}; - }, - }, - POST => { - method => 'update', - params => sub { - return {ids => $_[0]}; - }, - }, - }, - - # no bug-id - qr{^/bug_user_last_visit$}, - {GET => {method => 'get',}, POST => {method => 'update',},}, - ]; -} - -1; -__END__ - -=head1 NAME - -Bugzilla::Webservice::Server::REST::Resources::BugUserLastVisit - The -BugUserLastVisit REST API - -=head1 DESCRIPTION - -This part of the Bugzilla REST API allows you to lookup and update the last time -a user visited a bug. - -See L for more details on how to use -this part of the REST API. From 91205fec8d651efa7e6a4f530235c26aab3dc542 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 14 Sep 2026 19:49:46 +0200 Subject: [PATCH 02/16] Bug 2065171 - Merge query-string and JSON body params for ids/include_fields --- Bugzilla/API/V1/BugUserLastVisit.pm | 33 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index 6483561d3b..533a909bb9 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -66,7 +66,7 @@ sub get { @last_visits = grep { $id_set{$_->bug_id} } @last_visits; } - my $params = $self->_filter_params; + my $params = $self->_request_params; return $self->render( json => [ @@ -93,7 +93,7 @@ sub update { # aliases. $user->visible_bugs([grep {/^[0-9]+$/} @$ids]); - my $params = $self->_filter_params; + my $params = $self->_request_params; my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction(); @@ -121,24 +121,27 @@ sub _ids_from_request { return [$id]; } - if ($self->req->method eq 'POST') { - my $params; - my $error; - try { $params = decode_json($self->req->body || '{}'); } - catch { $error = 'rest_malformed_json'; }; - return (undef, $error) if $error; - my $ids = $params->{ids} // []; - return ref $ids ? $ids : [$ids]; - } - - my $ids = $self->every_param('ids'); - return @$ids ? $ids : undef; + my $ids = $self->_request_params->{ids} // []; + return ref $ids ? $ids : [$ids]; } -sub _filter_params { +sub _request_params { my ($self) = @_; + # $self->req->params already covers the query string plus, for POST, an + # application/x-www-form-urlencoded or multipart body. Layer a JSON body + # on top of that (silently ignored if absent or not valid JSON) so ids and + # include_fields/exclude_fields work from either the query string or a + # JSON POST body, matching the legacy REST layer's merging behavior. my $params = $self->req->params->to_hash; + + if ($self->req->method eq 'POST' && length $self->req->body) { + my $body_params; + try { $body_params = decode_json($self->req->body); } + catch { $body_params = undef; }; + $params = {%$params, %$body_params} if ref $body_params eq 'HASH'; + } + for my $field (qw(include_fields exclude_fields)) { $params->{$field} = [split(/[\s,]+/, $params->{$field})] if exists $params->{$field} && !ref $params->{$field}; From 1957cb4f691ba9332da11f2afa7c8a5b18138f90 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 14 Sep 2026 20:04:45 +0200 Subject: [PATCH 03/16] Bug 2065171 - Reject non-array ids with invalid_params --- Bugzilla/API/V1/BugUserLastVisit.pm | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index 533a909bb9..db5248a0cf 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -47,7 +47,8 @@ sub get { my $user = $self->bugzilla->login; $user->id || return $self->user_error('login_required'); - my ($ids) = $self->_ids_from_request; + my ($ids, $error, $vars) = $self->_ids_from_request; + return $self->user_error($error, $vars) if $error; if ($ids) { @@ -83,8 +84,8 @@ sub update { my $user = $self->bugzilla->login; $user->id || return $self->user_error('login_required'); - my ($ids, $error) = $self->_ids_from_request; - return $self->user_error($error) if $error; + my ($ids, $error, $vars) = $self->_ids_from_request; + return $self->user_error($error, $vars) if $error; return $self->code_error('param_required', {param => 'ids'}) unless $ids && @$ids; @@ -122,7 +123,9 @@ sub _ids_from_request { } my $ids = $self->_request_params->{ids} // []; - return ref $ids ? $ids : [$ids]; + return (undef, 'invalid_params', {type_error => 'ids must be an array'}) + if ref $ids && ref $ids ne 'ARRAY'; + return ref $ids eq 'ARRAY' ? $ids : [$ids]; } sub _request_params { From e2dbfdcbb9883ba47d81757d196501d6b2c1f41f Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 14 Sep 2026 20:31:08 +0200 Subject: [PATCH 04/16] Bug 2065171 - Use resolved bug id, restrict :id route to digits --- Bugzilla/API/V1/BugUserLastVisit.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index db5248a0cf..921905216e 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -23,9 +23,9 @@ sub setup_routes { my $routes = $r->under( '/bug_user_last_visit' => sub { Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); }); $routes->get('/')->to('V1::BugUserLastVisit#get'); - $routes->get('/:id')->to('V1::BugUserLastVisit#get'); + $routes->get('/:id' => [id => qr/\d+/])->to('V1::BugUserLastVisit#get'); $routes->post('/')->to('V1::BugUserLastVisit#update'); - $routes->post('/:id')->to('V1::BugUserLastVisit#update'); + $routes->post('/:id' => [id => qr/\d+/])->to('V1::BugUserLastVisit#update'); foreach my $path ('/', '/:id') { $routes->options($path)->to('V1::BugUserLastVisit#options'); @@ -108,7 +108,7 @@ sub update { $bug->update_user_last_visit($user, $last_visit_ts); push(@results, - $self->_bug_user_last_visit_to_hash($bug_id, $last_visit_ts, $params)); + $self->_bug_user_last_visit_to_hash($bug->id, $last_visit_ts, $params)); } $dbh->bz_commit_transaction(); From 708e50b4b4a1fe3d7e980169b9e39de4d407db42 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Mon, 14 Sep 2026 21:59:27 +0200 Subject: [PATCH 05/16] Bug 2065171 - Fix ids/include_fields precedence: query string wins over body --- Bugzilla/API/V1/BugUserLastVisit.pm | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index 921905216e..7004e8cf1b 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -133,16 +133,19 @@ sub _request_params { # $self->req->params already covers the query string plus, for POST, an # application/x-www-form-urlencoded or multipart body. Layer a JSON body - # on top of that (silently ignored if absent or not valid JSON) so ids and - # include_fields/exclude_fields work from either the query string or a - # JSON POST body, matching the legacy REST layer's merging behavior. + # underneath that (silently ignored if absent or not valid JSON) so ids + # and include_fields/exclude_fields work from either the query string or + # a JSON POST body. Query-string values win on a key collision, matching + # the legacy REST layer (see _retrieve_json_params in + # Bugzilla::WebService::Server::REST) and the documented behavior in + # docs/en/rst/api/core/v1/general.rst. my $params = $self->req->params->to_hash; if ($self->req->method eq 'POST' && length $self->req->body) { my $body_params; try { $body_params = decode_json($self->req->body); } catch { $body_params = undef; }; - $params = {%$params, %$body_params} if ref $body_params eq 'HASH'; + $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; } for my $field (qw(include_fields exclude_fields)) { From 30048ecd8cfb45807a4d9139ff8b7584e0718890 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Tue, 15 Sep 2026 15:42:40 +0200 Subject: [PATCH 06/16] Bug 2065171 - Return undef when ids is absent `_request_params->{ids} // []` made a missing ids param filter to nothing instead of returning every visited bug, since an empty arrayref is truthy. Legacy left $ids undef when the param is absent, skipping filter entirely. Return undef in that case matches legacy behavior. An empty array still filters to nothing. --- Bugzilla/API/V1/BugUserLastVisit.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index 7004e8cf1b..49f4bc9bd2 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -122,7 +122,8 @@ sub _ids_from_request { return [$id]; } - my $ids = $self->_request_params->{ids} // []; + my $ids = $self->_request_params->{ids}; + return undef unless defined $ids; return (undef, 'invalid_params', {type_error => 'ids must be an array'}) if ref $ids && ref $ids ne 'ARRAY'; return ref $ids eq 'ARRAY' ? $ids : [$ids]; From de80829b8366abd5d26dc68b29181b56024745d8 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 14:44:15 +0200 Subject: [PATCH 07/16] Bug 2065171 - Use shared merge_request_params helper _request_params duplicated the query-string/JSON-body merge logic. Now call a single shared Bugzilla::WebService::Util::merge_request_params helper instead, so it's a one-place change to drop later if query-string-on-POST support is ever removed. --- Bugzilla/API/V1/BugUserLastVisit.pm | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index 49f4bc9bd2..d0c314b4d5 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -10,13 +10,10 @@ package Bugzilla::API::V1::BugUserLastVisit; use 5.10.1; use Mojo::Base qw( Mojolicious::Controller ); -use Mojo::JSON qw(decode_json); -use Try::Tiny; - use Bugzilla::Bug; use Bugzilla::Constants; use Bugzilla::Util qw(datetime_from); -use Bugzilla::WebService::Util qw(filter); +use Bugzilla::WebService::Util qw(filter merge_request_params); sub setup_routes { my ($class, $r) = @_; @@ -132,22 +129,7 @@ sub _ids_from_request { sub _request_params { my ($self) = @_; - # $self->req->params already covers the query string plus, for POST, an - # application/x-www-form-urlencoded or multipart body. Layer a JSON body - # underneath that (silently ignored if absent or not valid JSON) so ids - # and include_fields/exclude_fields work from either the query string or - # a JSON POST body. Query-string values win on a key collision, matching - # the legacy REST layer (see _retrieve_json_params in - # Bugzilla::WebService::Server::REST) and the documented behavior in - # docs/en/rst/api/core/v1/general.rst. - my $params = $self->req->params->to_hash; - - if ($self->req->method eq 'POST' && length $self->req->body) { - my $body_params; - try { $body_params = decode_json($self->req->body); } - catch { $body_params = undef; }; - $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; - } + my $params = merge_request_params($self); for my $field (qw(include_fields exclude_fields)) { $params->{$field} = [split(/[\s,]+/, $params->{$field})] From f2bd2121f7afbd4581f27c164a6ed92760ba0c81 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 16:02:53 +0200 Subject: [PATCH 08/16] Bug 2065171 - Fix ids precedence: body overrides path id on POST _ids_from_request short-circuited to the path id whenever present, never consulting the merged query-string/body params. Legacy's _retrieve_json_params merges non-GET request-body/query params in *after* the path-derived params, so those win for POST. For GET, the path id still wins (legacy's override step only ran for non-GET requests), so that precedence is unchanged. Also switch from $self->param('id') (a truthiness check that also falls back to a same-named query param) to $self->stash('id') (defined check, route-placeholder only). This fixes two more bugs: - /bug_user_last_visit/0 was falling through to the no-ids branch since "0" is falsy - a stray ?id=5 query parameter (distinct from ids) was being treated as if it were a path id --- Bugzilla/API/V1/BugUserLastVisit.pm | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index d0c314b4d5..cb3e87e638 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -115,12 +115,21 @@ sub update { sub _ids_from_request { my ($self) = @_; - if (my $id = $self->param('id')) { - return [$id]; + my $path_id = $self->stash('id'); + + # Legacy REST layer (_retrieve_json_params in + # Bugzilla::WebService::Server::REST): for GET, the path id wins over any + # query-string ids. For POST, request-body/query-string params are merged + # in *after* the path-derived params, so they win instead. + if (defined $path_id && $self->req->method ne 'POST') { + return [$path_id]; } my $ids = $self->_request_params->{ids}; - return undef unless defined $ids; + if (!defined $ids) { + return defined $path_id ? [$path_id] : undef; + } + return (undef, 'invalid_params', {type_error => 'ids must be an array'}) if ref $ids && ref $ids ne 'ARRAY'; return ref $ids eq 'ARRAY' ? $ids : [$ids]; From 229e0465f0b525752050ce00c8f2a3ce48c9db36 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Thu, 17 Sep 2026 16:20:55 +0200 Subject: [PATCH 09/16] Bug 2065171 - Add qa/t/rest_bug_user_last_visit.t Covers: anonymous access requiring login, OPTIONS, POST via path id, POST via a JSON ids body, POST with a JSON body posted with no Content-Type header, GET via path id vs query-string ids precedence, GET via query-string ids, and GET with no ids returning every visited bug --- qa/t/rest_bug_user_last_visit.t | 138 ++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 qa/t/rest_bug_user_last_visit.t diff --git a/qa/t/rest_bug_user_last_visit.t b/qa/t/rest_bug_user_last_visit.t new file mode 100644 index 0000000000..7615ad2af0 --- /dev/null +++ b/qa/t/rest_bug_user_last_visit.t @@ -0,0 +1,138 @@ +#!/usr/bin/env perl +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. +use strict; +use warnings; +use 5.10.1; +use lib qw(lib ../../lib ../../local/lib/perl5); + +use Bugzilla; +use QA::Util qw(get_config); + +use Mojo::JSON qw(encode_json); +use Test::Mojo; +use Test::More; + +my $config = get_config(); +my $api_key = $config->{editbugs_user_api_key}; +my $url = Bugzilla->localconfig->urlbase; + +my $t = Test::Mojo->new(); +$t->ua->max_redirects(1); + +### Setup: create two bugs to record visits against + +sub create_bug { + my ($summary) = @_; + $t->post_ok($url + . 'rest/bug' => {'X-Bugzilla-API-Key' => $api_key} => json => { + product => 'Firefox', + component => 'General', + summary => $summary, + type => 'defect', + version => 'unspecified', + severity => 'blocker', + description => $summary, + })->status_is(200)->json_has('/id'); + return $t->tx->res->json->{id}; +} + +my $bug_id_1 = create_bug('bug_user_last_visit test bug 1'); +my $bug_id_2 = create_bug('bug_user_last_visit test bug 2'); + +### Section 1: Anonymous access requires login + +$t->get_ok($url . 'rest/bug_user_last_visit')->status_is(401) + ->json_is( + '/message' => 'You must log in before using this part of Bugzilla.'); + +### Section 2: OPTIONS + +$t->options_ok($url . 'rest/bug_user_last_visit')->status_is(200) + ->header_is('Allow' => 'GET, POST'); +$t->options_ok($url . "rest/bug_user_last_visit/$bug_id_1")->status_is(200) + ->header_is('Allow' => 'GET, POST'); + +### Section 3: POST /rest/bug_user_last_visit/ records a visit via the path + +$t->post_ok($url + . "rest/bug_user_last_visit/$bug_id_1" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200) + ->json_is('/0/id' => $bug_id_1)->json_has('/0/last_visit_ts'); + +like($t->tx->res->json->[0]->{last_visit_ts}, qr/Z$/, 'last_visit_ts ends in Z'); + +### Section 4: POST /rest/bug_user_last_visit with a JSON ids body records +### visits for multiple bugs at once + +$t->post_ok($url + . 'rest/bug_user_last_visit' => {'X-Bugzilla-API-Key' => $api_key} => + json => {ids => [$bug_id_1, $bug_id_2]})->status_is(200); + +my @posted_ids = sort { $a <=> $b } map { $_->{id} } @{$t->tx->res->json}; +is_deeply(\@posted_ids, [sort { $a <=> $b } ($bug_id_1, $bug_id_2)], + 'both bugs recorded from a JSON body ids array'); + +### Section 5: a JSON body ids overrides a path id on POST (matches the +### legacy REST layer, where non-GET body/query params are merged in after, +### and so win over, path-derived params) + +$t->post_ok($url + . "rest/bug_user_last_visit/$bug_id_1" => + {'X-Bugzilla-API-Key' => $api_key} => json => {ids => [$bug_id_2]}) + ->status_is(200); + +my @body_override_ids = map { $_->{id} } @{$t->tx->res->json}; +is_deeply(\@body_override_ids, [$bug_id_2], + 'a JSON body ids overrides the path id on POST'); + +### Section 6: a JSON body with no Content-Type header still works (real +### frontend callers post this way) + +my $raw_json = encode_json({ids => [$bug_id_1]}); +$t->post_ok($url + . 'rest/bug_user_last_visit' => {'X-Bugzilla-API-Key' => $api_key} => + $raw_json)->status_is(200); + +my @no_content_type_ids = map { $_->{id} } @{$t->tx->res->json}; +is_deeply(\@no_content_type_ids, [$bug_id_1], + 'a JSON body with no Content-Type header is still parsed'); + +### Section 7: GET /rest/bug_user_last_visit/ -- the path id wins over a +### query-string ids on GET (unchanged from before the POST precedence fix) + +$t->get_ok($url + . "rest/bug_user_last_visit/$bug_id_1?ids=$bug_id_2" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200); + +my @get_path_wins_ids = map { $_->{id} } @{$t->tx->res->json}; +is_deeply(\@get_path_wins_ids, [$bug_id_1], + 'the path id wins over a query-string ids on GET'); + +### Section 8: GET /rest/bug_user_last_visit?ids=...&ids=... filters to the +### requested bugs + +$t->get_ok($url + . "rest/bug_user_last_visit?ids=$bug_id_1&ids=$bug_id_2" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200); + +my @get_query_ids = sort { $a <=> $b } map { $_->{id} } @{$t->tx->res->json}; +is_deeply(\@get_query_ids, [sort { $a <=> $b } ($bug_id_1, $bug_id_2)], + 'query-string ids filters to the requested bugs'); + +### Section 9: GET /rest/bug_user_last_visit with no ids at all returns +### every visited bug, not an empty list + +$t->get_ok($url . 'rest/bug_user_last_visit' => {'X-Bugzilla-API-Key' => $api_key}) + ->status_is(200); + +my @get_all_ids = sort { $a <=> $b } map { $_->{id} } @{$t->tx->res->json}; +ok((grep { $_ == $bug_id_1 } @get_all_ids) + && (grep { $_ == $bug_id_2 } @get_all_ids), + 'GET with no ids returns every visited bug'); + +done_testing(); From 7b6a8d0bdbf2a341311310db31cb869c7942e614 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 19:13:25 +0200 Subject: [PATCH 10/16] Bug 2065171 - Report a malformed JSON body instead of silently ignoring it --- Bugzilla/API/V1/BugUserLastVisit.pm | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index cb3e87e638..f7a0445dd0 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -64,7 +64,8 @@ sub get { @last_visits = grep { $id_set{$_->bug_id} } @last_visits; } - my $params = $self->_request_params; + my ($params, $params_error) = $self->_request_params; + return $self->user_error($params_error) if $params_error; return $self->render( json => [ @@ -91,8 +92,10 @@ sub update { # aliases. $user->visible_bugs([grep {/^[0-9]+$/} @$ids]); - my $params = $self->_request_params; - my $dbh = Bugzilla->dbh; + my ($params, $params_error) = $self->_request_params; + return $self->user_error($params_error) if $params_error; + + my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction(); my @results; @@ -125,7 +128,10 @@ sub _ids_from_request { return [$path_id]; } - my $ids = $self->_request_params->{ids}; + my ($params, $error) = $self->_request_params; + return (undef, $error) if $error; + + my $ids = $params->{ids}; if (!defined $ids) { return defined $path_id ? [$path_id] : undef; } @@ -138,14 +144,15 @@ sub _ids_from_request { sub _request_params { my ($self) = @_; - my $params = merge_request_params($self); + my ($params, $error) = merge_request_params($self); + return (undef, $error) if $error; for my $field (qw(include_fields exclude_fields)) { $params->{$field} = [split(/[\s,]+/, $params->{$field})] if exists $params->{$field} && !ref $params->{$field}; } - return $params; + return ($params, undef); } sub _bug_user_last_visit_to_hash { From f3d1df30ed5bd6fb114d4ff041b680b9ccde1287 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 19:15:09 +0200 Subject: [PATCH 11/16] Bug 2065171 - Let callers declare which request params are lists merge_request_params collapsed every param to a scalar, which dropped all but the last value of a repeated key such as ?ids=1&ids=2. Callers now declare their list params, so a param's type no longer depends on how many times it was sent. --- Bugzilla/API/V1/BugUserLastVisit.pm | 2 +- Bugzilla/WebService/Util.pm | 28 ++++++++++++++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index f7a0445dd0..4a05f30253 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -144,7 +144,7 @@ sub _ids_from_request { sub _request_params { my ($self) = @_; - my ($params, $error) = merge_request_params($self); + my ($params, $error) = merge_request_params($self, ['ids']); return (undef, $error) if $error; for my $field (qw(include_fields exclude_fields)) { diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 0a5e94a98c..8c2ca8c02d 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -301,20 +301,26 @@ sub params_to_objects { } sub merge_request_params { - my ($c) = @_; + my ($c, $list_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, 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 + # request body. Query-string/form-body 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¬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}; + # A param's type must not depend on how many times it was sent: ->to_hash + # returns a scalar for one occurrence and an arrayref for two. Callers + # therefore declare which params are lists; those always come back as + # arrayrefs, everything else always as a scalar. + my %is_list = map { $_ => 1 } @{$list_params || []}; + my $params = {}; + for my $name (@{$c->req->params->names}) { + $params->{$name} + = $is_list{$name} ? $c->req->every_param($name) : $c->req->param($name); + } # 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. @@ -443,6 +449,12 @@ 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. +An optional second argument is an arrayref of parameter names that are +lists, e.g. C. Those are always returned +as arrayrefs, however many times they appear in the request; every other +parameter is always returned as a scalar. Declaring them is required because +a parameter's type must not depend on the number of occurrences sent. + 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. From 0ea15f626b44f1e91e8e78d2824ef028b1aed46e Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 19:19:24 +0200 Subject: [PATCH 12/16] Bug 2065171 - Compute the merged request params once per request _request_params ran twice per request and merge_request_params read the body twice, so one POST slurped a file-backed request asset four times and decoded the JSON twice. get/update now compute the params once and pass them into _ids_from_request, and the helper reads the body into a variable. --- Bugzilla/API/V1/BugUserLastVisit.pm | 27 ++++++++++++--------------- Bugzilla/WebService/Util.pm | 7 +++++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Bugzilla/API/V1/BugUserLastVisit.pm b/Bugzilla/API/V1/BugUserLastVisit.pm index 4a05f30253..1f0a291f2c 100644 --- a/Bugzilla/API/V1/BugUserLastVisit.pm +++ b/Bugzilla/API/V1/BugUserLastVisit.pm @@ -24,9 +24,9 @@ sub setup_routes { $routes->post('/')->to('V1::BugUserLastVisit#update'); $routes->post('/:id' => [id => qr/\d+/])->to('V1::BugUserLastVisit#update'); - foreach my $path ('/', '/:id') { - $routes->options($path)->to('V1::BugUserLastVisit#options'); - } + $routes->options('/')->to('V1::BugUserLastVisit#options'); + $routes->options('/:id' => [id => qr/\d+/]) + ->to('V1::BugUserLastVisit#options'); } sub options { @@ -44,7 +44,10 @@ sub get { my $user = $self->bugzilla->login; $user->id || return $self->user_error('login_required'); - my ($ids, $error, $vars) = $self->_ids_from_request; + my ($params, $params_error) = $self->_request_params; + return $self->user_error($params_error) if $params_error; + + my ($ids, $error, $vars) = $self->_ids_from_request($params); return $self->user_error($error, $vars) if $error; if ($ids) { @@ -64,9 +67,6 @@ sub get { @last_visits = grep { $id_set{$_->bug_id} } @last_visits; } - my ($params, $params_error) = $self->_request_params; - return $self->user_error($params_error) if $params_error; - return $self->render( json => [ map { @@ -82,7 +82,10 @@ sub update { my $user = $self->bugzilla->login; $user->id || return $self->user_error('login_required'); - my ($ids, $error, $vars) = $self->_ids_from_request; + my ($params, $params_error) = $self->_request_params; + return $self->user_error($params_error) if $params_error; + + my ($ids, $error, $vars) = $self->_ids_from_request($params); return $self->user_error($error, $vars) if $error; return $self->code_error('param_required', {param => 'ids'}) unless $ids && @$ids; @@ -92,9 +95,6 @@ sub update { # aliases. $user->visible_bugs([grep {/^[0-9]+$/} @$ids]); - my ($params, $params_error) = $self->_request_params; - return $self->user_error($params_error) if $params_error; - my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction(); @@ -116,7 +116,7 @@ sub update { } sub _ids_from_request { - my ($self) = @_; + my ($self, $params) = @_; my $path_id = $self->stash('id'); @@ -128,9 +128,6 @@ sub _ids_from_request { return [$path_id]; } - my ($params, $error) = $self->_request_params; - return (undef, $error) if $error; - my $ids = $params->{ids}; if (!defined $ids) { return defined $path_id ? [$path_id] : undef; diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 8c2ca8c02d..d282700a5d 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -326,10 +326,13 @@ sub merge_request_params { # 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}) { + # Read the body once: for a file-backed request asset each ->body call + # re-slurps it from disk. + my $body = $c->req->body; + if (length $body && !@{$c->req->body_params->names}) { my $body_params; my $error; - try { $body_params = decode_json($c->req->body); } + try { $body_params = decode_json($body); } catch { $error = 'rest_malformed_json'; }; return (undef, $error) if $error; $params = {%$body_params, %$params} if ref $body_params eq 'HASH'; From dcc2ba1aea698e158197ea8b50c879139191bead Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 19:20:52 +0200 Subject: [PATCH 13/16] Bug 2065171 - Constrain the OPTIONS :id route to digits OPTIONS /rest/bug_user_last_visit/abc answered 200 Allow: GET, POST while GET and POST on that path 404, advertising methods that do not exist there. --- qa/t/rest_bug_user_last_visit.t | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qa/t/rest_bug_user_last_visit.t b/qa/t/rest_bug_user_last_visit.t index 7615ad2af0..d3031f5472 100644 --- a/qa/t/rest_bug_user_last_visit.t +++ b/qa/t/rest_bug_user_last_visit.t @@ -57,6 +57,10 @@ $t->options_ok($url . 'rest/bug_user_last_visit')->status_is(200) $t->options_ok($url . "rest/bug_user_last_visit/$bug_id_1")->status_is(200) ->header_is('Allow' => 'GET, POST'); +# A non-numeric id matches no route, so OPTIONS must not advertise methods +# that would 404 on that path. +$t->options_ok($url . 'rest/bug_user_last_visit/abc')->status_is(404); + ### Section 3: POST /rest/bug_user_last_visit/ records a visit via the path $t->post_ok($url From bfaa0af1c2c49c1678968c1bd3b1f3718be99b3e Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 19:53:31 +0200 Subject: [PATCH 14/16] Bug 2065171 - Cover the negative cases in rest_bug_user_last_visit.t Adds the failure paths that prove parity with the legacy endpoint: a bug in a group the user is not in, anonymous POST, a nonexistent bug id and the mid-loop rollback it triggers, POST with no ids anywhere, and include_fields/exclude_fields. --- qa/t/rest_bug_user_last_visit.t | 81 +++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/qa/t/rest_bug_user_last_visit.t b/qa/t/rest_bug_user_last_visit.t index d3031f5472..5bb33243db 100644 --- a/qa/t/rest_bug_user_last_visit.t +++ b/qa/t/rest_bug_user_last_visit.t @@ -139,4 +139,85 @@ ok((grep { $_ == $bug_id_1 } @get_all_ids) && (grep { $_ == $bug_id_2 } @get_all_ids), 'GET with no ids returns every visited bug'); +### Section 10: a bug in a group the user is not a member of is not +### accessible, and does not leak through the GET filter + +my $private_api_key = $config->{QA_Selenium_TEST_user_api_key}; + +$t->post_ok($url + . 'rest/bug' => {'X-Bugzilla-API-Key' => $private_api_key} => json => { + product => 'QA-Selenium-TEST', + component => 'QA-Selenium-TEST', + summary => 'bug_user_last_visit private test bug', + type => 'defect', + version => 'QAVersion', + target_milestone => 'QAMilestone', + severity => 'blocker', + description => 'bug_user_last_visit private test bug', + groups => ['QA-Selenium-TEST'], + })->status_is(200)->json_has('/id'); + +my $private_bug_id = $t->tx->res->json->{id}; + +$t->post_ok($url + . "rest/bug_user_last_visit/$private_bug_id" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(401) + ->json_is('/code' => 102) + ->json_like('/message' => qr/not authorized to access/); + +$t->get_ok($url + . "rest/bug_user_last_visit?ids=$private_bug_id" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200); +is_deeply($t->tx->res->json, [], + 'a bug the user cannot see is not returned by GET'); + +### Section 11: anonymous POST requires login (anonymous GET is section 1) + +$t->post_ok($url . 'rest/bug_user_last_visit' => json => {ids => [$bug_id_1]}) + ->status_is(401) + ->json_is( + '/message' => 'You must log in before using this part of Bugzilla.'); + +### Section 12: a nonexistent bug id fails the whole request, and the visit +### recorded earlier in the same loop is rolled back + +$t->post_ok($url + . "rest/bug_user_last_visit/$bug_id_1" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200); +my $ts_before = $t->tx->res->json->[0]->{last_visit_ts}; + +# last_visit_ts has second granularity, so without this the rolled-back and +# the would-be-new timestamp could be identical and the test pass spuriously. +sleep 1; + +$t->post_ok($url + . 'rest/bug_user_last_visit' => {'X-Bugzilla-API-Key' => $api_key} => + json => {ids => [$bug_id_1, 99999999]})->status_is(404) + ->json_is('/code' => 101)->json_like('/message' => qr/does not exist/); + +$t->get_ok($url + . "rest/bug_user_last_visit/$bug_id_1" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200); +is($t->tx->res->json->[0]->{last_visit_ts}, + $ts_before, 'the visit recorded before the bad id was rolled back'); + +### Section 13: POST with no ids in the path, query string or body + +$t->post_ok($url + . 'rest/bug_user_last_visit' => {'X-Bugzilla-API-Key' => $api_key} => + json => {})->status_is(400)->json_is('/code' => 50) + ->json_like('/message' => qr/argument was not set/); + +### Section 14: include_fields / exclude_fields + +$t->get_ok($url + . "rest/bug_user_last_visit/$bug_id_1?include_fields=id" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200)->json_has('/0/id') + ->json_hasnt('/0/last_visit_ts'); + +$t->get_ok($url + . "rest/bug_user_last_visit/$bug_id_1?exclude_fields=last_visit_ts" => + {'X-Bugzilla-API-Key' => $api_key})->status_is(200)->json_has('/0/id') + ->json_hasnt('/0/last_visit_ts'); + done_testing(); From 1fe8de502f4ede51d75b2dfc9b525058536b8c28 Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 20:26:50 +0200 Subject: [PATCH 15/16] Bug 2065171 - Fix the negative-case expectations in rest_bug_user_last_visit.t A nonexistent bug id reports improper_bug_id_field_value, not bug_id_does_not_exist, because Bug->new only sets NotFound for a bare scalar. The private bug needs no groups or milestone, since its product carries the group as mandatory. Drops the OPTIONS assertion on a non-numeric id: the route constraint sends it to the legacy rest.cgi catch-all instead of a 404. --- qa/t/rest_bug_user_last_visit.t | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/qa/t/rest_bug_user_last_visit.t b/qa/t/rest_bug_user_last_visit.t index 5bb33243db..91e378ca9c 100644 --- a/qa/t/rest_bug_user_last_visit.t +++ b/qa/t/rest_bug_user_last_visit.t @@ -57,10 +57,6 @@ $t->options_ok($url . 'rest/bug_user_last_visit')->status_is(200) $t->options_ok($url . "rest/bug_user_last_visit/$bug_id_1")->status_is(200) ->header_is('Allow' => 'GET, POST'); -# A non-numeric id matches no route, so OPTIONS must not advertise methods -# that would 404 on that path. -$t->options_ok($url . 'rest/bug_user_last_visit/abc')->status_is(404); - ### Section 3: POST /rest/bug_user_last_visit/ records a visit via the path $t->post_ok($url @@ -144,17 +140,18 @@ ok((grep { $_ == $bug_id_1 } @get_all_ids) my $private_api_key = $config->{QA_Selenium_TEST_user_api_key}; +# No "groups" needed: the QA-Selenium-TEST group is CONTROLMAPMANDATORY on the +# product of the same name, so every bug filed there gets it automatically. + $t->post_ok($url . 'rest/bug' => {'X-Bugzilla-API-Key' => $private_api_key} => json => { - product => 'QA-Selenium-TEST', - component => 'QA-Selenium-TEST', - summary => 'bug_user_last_visit private test bug', - type => 'defect', - version => 'QAVersion', - target_milestone => 'QAMilestone', - severity => 'blocker', - description => 'bug_user_last_visit private test bug', - groups => ['QA-Selenium-TEST'], + product => 'QA-Selenium-TEST', + component => 'QA-Selenium-TEST', + summary => 'bug_user_last_visit private test bug', + type => 'defect', + version => 'unspecified', + severity => 'blocker', + description => 'bug_user_last_visit private test bug', })->status_is(200)->json_has('/id'); my $private_bug_id = $t->tx->res->json->{id}; @@ -190,10 +187,14 @@ my $ts_before = $t->tx->res->json->[0]->{last_visit_ts}; # the would-be-new timestamp could be identical and the test pass spuriously. sleep 1; +# Bugzilla::Bug->new sets error => 'InvalidBugId' rather than 'NotFound' when +# handed a hashref, so check() reports improper_bug_id_field_value with no bug +# id rather than bug_id_does_not_exist. The legacy endpoint calls check() the +# same way and behaves identically. $t->post_ok($url . 'rest/bug_user_last_visit' => {'X-Bugzilla-API-Key' => $api_key} => - json => {ids => [$bug_id_1, 99999999]})->status_is(404) - ->json_is('/code' => 101)->json_like('/message' => qr/does not exist/); + json => {ids => [$bug_id_1, 99999999]})->status_is(400) + ->json_is('/code' => 100)->json_like('/message' => qr/valid bug number/); $t->get_ok($url . "rest/bug_user_last_visit/$bug_id_1" => From b43a120094dd8565a79c2408e40e65bdfeeef60a Mon Sep 17 00:00:00 2001 From: Xavier L'Hour Date: Wed, 23 Sep 2026 20:47:06 +0200 Subject: [PATCH 16/16] Bug 2065171 - Build the private test bug the way rest_relationship_trees.t does Filing directly into the QA-Selenium-TEST product returned 400. Create the bug in Another Product with create_bug_fields() and restrict it afterwards, which is the setup already proven in CI. It is filed by the private user so the editbugs user is not its reporter and cannot see it that way. --- qa/t/rest_bug_user_last_visit.t | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/qa/t/rest_bug_user_last_visit.t b/qa/t/rest_bug_user_last_visit.t index 91e378ca9c..19cbb0657e 100644 --- a/qa/t/rest_bug_user_last_visit.t +++ b/qa/t/rest_bug_user_last_visit.t @@ -11,7 +11,8 @@ use 5.10.1; use lib qw(lib ../../lib ../../local/lib/perl5); use Bugzilla; -use QA::Util qw(get_config); +use QA::Util qw(get_config); +use QA::Tests qw(create_bug_fields PRIVATE_BUG_USER); use Mojo::JSON qw(encode_json); use Test::Mojo; @@ -138,24 +139,26 @@ ok((grep { $_ == $bug_id_1 } @get_all_ids) ### Section 10: a bug in a group the user is not a member of is not ### accessible, and does not leak through the GET filter -my $private_api_key = $config->{QA_Selenium_TEST_user_api_key}; +# File it as, and restrict it to, a group the editbugs user is not in. Created +# by the private user so that the editbugs user is not its reporter either. +# Same setup as qa/t/rest_relationship_trees.t. +my $private_api_key = $config->{PRIVATE_BUG_USER . '_user_api_key'}; -# No "groups" needed: the QA-Selenium-TEST group is CONTROLMAPMANDATORY on the -# product of the same name, so every bug filed there gets it automatically. +my $private_bug_data = create_bug_fields($config); +delete $private_bug_data->{cc}; +$private_bug_data->{summary} = 'bug_user_last_visit private test bug'; +$private_bug_data->{description} = 'bug_user_last_visit private test bug'; $t->post_ok($url - . 'rest/bug' => {'X-Bugzilla-API-Key' => $private_api_key} => json => { - product => 'QA-Selenium-TEST', - component => 'QA-Selenium-TEST', - summary => 'bug_user_last_visit private test bug', - type => 'defect', - version => 'unspecified', - severity => 'blocker', - description => 'bug_user_last_visit private test bug', - })->status_is(200)->json_has('/id'); + . 'rest/bug' => {'X-Bugzilla-API-Key' => $private_api_key} => json => + $private_bug_data)->status_is(200)->json_has('/id'); my $private_bug_id = $t->tx->res->json->{id}; +$t->put_ok($url + . "rest/bug/$private_bug_id" => {'X-Bugzilla-API-Key' => $private_api_key} + => json => {groups => {add => ['QA-Selenium-TEST']}})->status_is(200); + $t->post_ok($url . "rest/bug_user_last_visit/$private_bug_id" => {'X-Bugzilla-API-Key' => $api_key})->status_is(401)