From add549d6c28d0ab4ae45d7e9344f9c4cf406c1b0 Mon Sep 17 00:00:00 2001 From: Logan Rosen Date: Fri, 2 Oct 2026 14:35:42 -0400 Subject: [PATCH 1/2] Bug 2059957 - Reject truncated request bodies before native endpoint dispatch Reject body-related and unknown parser limits before native endpoint actions run, preserving response formats, REST CORS, and existing CGI handling. Document the REST 413 response and add focused regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Bugzilla/App.pm | 1 + Bugzilla/App/Controller/API.pm | 10 +-- Bugzilla/App/Controller/CSPReport.pm | 3 +- Bugzilla/App/Controller/Main.pm | 3 +- Bugzilla/App/Plugin/RequestLimit.pm | 82 ++++++++++++++++++++++++ docs/en/rst/api/core/v1/general.rst | 6 ++ t/app-cgi-request-limit.t | 95 ++++++++++++++++++++++++++++ 7 files changed, 189 insertions(+), 11 deletions(-) create mode 100644 Bugzilla/App/Plugin/RequestLimit.pm diff --git a/Bugzilla/App.pm b/Bugzilla/App.pm index 4a3eac3084..e6f52890d4 100644 --- a/Bugzilla/App.pm +++ b/Bugzilla/App.pm @@ -42,6 +42,7 @@ sub startup { $self->plugin('Bugzilla::App::Plugin::Glue'); $self->plugin('Bugzilla::App::Plugin::Login'); $self->plugin('Bugzilla::App::Plugin::Error'); + $self->plugin('Bugzilla::App::Plugin::RequestLimit'); $self->plugin('Bugzilla::App::Plugin::Hostage') unless $ENV{BUGZILLA_DISABLE_HOSTAGE}; $self->plugin('Bugzilla::App::Plugin::SizeLimit') diff --git a/Bugzilla/App/Controller/API.pm b/Bugzilla/App/Controller/API.pm index 767a66191d..3dba5891ad 100644 --- a/Bugzilla/App/Controller/API.pm +++ b/Bugzilla/App/Controller/API.pm @@ -20,7 +20,6 @@ use Bugzilla::Logging; use Bugzilla::WebService::Util qw(set_rest_cors_headers); use constant SUPPORTED_VERSIONS => qw(V1); -use constant REQUEST_TOO_LARGE_ERROR => 'request_too_large'; sub setup_routes { my ($class, $r) = @_; @@ -73,14 +72,7 @@ sub setup_routes { sub _prepare_rest_request { my ($c) = @_; _insert_rest_headers($c); - - if ($c->req->is_limit_exceeded) { - my $reason = $c->req->error->{message}; - WARN("Rejected oversized request for native REST API: $reason"); - Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); - return $c->user_error(REQUEST_TOO_LARGE_ERROR); - } - + $c->stash->{request_limit_format} = 'rest'; Bugzilla->usage_mode(USAGE_MODE_REST); return 1; } diff --git a/Bugzilla/App/Controller/CSPReport.pm b/Bugzilla/App/Controller/CSPReport.pm index f39721c826..99808c5a58 100644 --- a/Bugzilla/App/Controller/CSPReport.pm +++ b/Bugzilla/App/Controller/CSPReport.pm @@ -59,7 +59,8 @@ use constant REPORT_FIELDS => sub setup_routes { my ($class, $r) = @_; - $r->post('/csp_report')->to('CSPReport#report'); + $r->post('/csp_report') + ->to('CSPReport#report', request_limit_format => 'empty'); } sub report { diff --git a/Bugzilla/App/Controller/Main.pm b/Bugzilla/App/Controller/Main.pm index db09ee6bba..15a8dc82bc 100644 --- a/Bugzilla/App/Controller/Main.pm +++ b/Bugzilla/App/Controller/Main.pm @@ -20,7 +20,8 @@ sub setup_routes { $r->get('/testagent.cgi')->to('Main#testagent'); $r->add_type('hex32' => qr/[[:xdigit:]]{32}/); - $r->post('/announcement/hide/')->to('Main#announcement_hide'); + $r->post('/announcement/hide/') + ->to('Main#announcement_hide', request_limit_format => 'json'); } sub root { diff --git a/Bugzilla/App/Plugin/RequestLimit.pm b/Bugzilla/App/Plugin/RequestLimit.pm new file mode 100644 index 0000000000..92e355b235 --- /dev/null +++ b/Bugzilla/App/Plugin/RequestLimit.pm @@ -0,0 +1,82 @@ +# 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::App::Plugin::RequestLimit; +use 5.10.1; +use Mojo::Base 'Mojolicious::Plugin'; + +use Bugzilla::Constants qw( + USAGE_MODE_MOJO + USAGE_MODE_MOJO_REST +); +use Bugzilla::Logging; + +use constant REQUEST_TOO_LARGE_ERROR => 'request_too_large'; +use constant REQUEST_TOO_LARGE_MESSAGE => 'The request is too large.'; + +my %BODY_LIMIT_REASONS = map { $_ => 1 } ( + 'Maximum message size exceeded', + 'Maximum buffer size exceeded', +); + +my %KNOWN_NON_BODY_LIMIT_REASONS = map { $_ => 1 } ( + 'Maximum start-line size exceeded', + 'Maximum header size exceeded', +); + +sub register { + my ($self, $app, $conf) = @_; + $app->hook(around_action => \&_around_action); +} + +sub _around_action { + my ($next, $c, $action, $is_endpoint) = @_; + return $next->() unless $is_endpoint; + return $next->() if $c->isa('Bugzilla::App::Controller::CGI'); + + my $reason = request_limit_reason($c->req); + return $next->() unless defined $reason; + + my $route = $c->match->endpoint->to_string; + WARN("Rejected oversized request for $route: $reason"); + + my $format = $c->stash->{request_limit_format} // 'html'; + if ($format eq 'rest') { + Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); + return $c->user_error(REQUEST_TOO_LARGE_ERROR); + } + elsif ($format eq 'json') { + return $c->render( + json => {error => REQUEST_TOO_LARGE_MESSAGE}, + status => 413 + ); + } + elsif ($format eq 'empty') { + return $c->render(data => '', status => 413); + } + + Bugzilla->usage_mode(USAGE_MODE_MOJO); + return $c->user_error( + REQUEST_TOO_LARGE_ERROR, + {}, + {status => 413, skip_exception_page => 1} + ); +} + +sub request_limit_reason { + my ($request) = @_; + return undef unless $request->is_limit_exceeded; + + my $error = $request->error; + return 'Unrecognized parser limit' unless ref $error eq 'HASH'; + + my $reason = $error->{message} // ''; + return undef if $KNOWN_NON_BODY_LIMIT_REASONS{$reason}; + return $BODY_LIMIT_REASONS{$reason} ? $reason : 'Unrecognized parser limit'; +} + +1; diff --git a/docs/en/rst/api/core/v1/general.rst b/docs/en/rst/api/core/v1/general.rst index 252ae63735..0de5552f43 100644 --- a/docs/en/rst/api/core/v1/general.rst +++ b/docs/en/rst/api/core/v1/general.rst @@ -52,6 +52,12 @@ The error contents look similar to: "code": 123 } +When BMO's request parser exceeds a message-size or body-buffer limit, the +REST API returns ``413 Request Entity Too Large`` with error code ``58`` and +the message ``The request is too large.`` rather than processing the truncated +body. These limits depend on the server configuration. Reduce the payload size +or split it into smaller requests where the API method supports batching. + .. _rest-query-string-limit: BMO's Varnish front end rejects request targets longer than 8 KiB, including diff --git a/t/app-cgi-request-limit.t b/t/app-cgi-request-limit.t index 8fe5fdd720..fa782a7fd7 100644 --- a/t/app-cgi-request-limit.t +++ b/t/app-cgi-request-limit.t @@ -24,6 +24,28 @@ use Bugzilla::Test::MockParams; use Test2::V0; use Test::Mojo; +{ + package TestRequest; + + sub new { + my ($class, $message, $is_limit_exceeded) = @_; + return bless { + message => $message, + is_limit_exceeded => $is_limit_exceeded // 1 + }, $class; + } + + sub error { + my ($self) = @_; + return {message => $self->{message}}; + } + + sub is_limit_exceeded { + my ($self) = @_; + return $self->{is_limit_exceeded}; + } +} + my $boundary = 'bugzilla-request-limit'; my $body = join( "\r\n", @@ -53,6 +75,40 @@ my $t = Test::Mojo->new('Bugzilla::App'); # The request-size environment limit also applies to the client's responses. $t->ua->max_response_size(0); +my $native_json_dispatched = 0; +my $native_json_route = $t->app->routes->post('/_test/native-json-limit'); +$native_json_route->to( + request_limit_format => 'json', + cb => sub { + my ($c) = @_; + $native_json_dispatched++; + return $c->render(json => {sentinel => 'controller ran'}); + } +); + +$t->post_ok( + '/_test/native-json-limit' => {'Content-Type' => 'application/json'} => '{}' +); +$t->status_is(200); +$t->json_is('/sentinel' => 'controller ran'); +is($native_json_dispatched, 1, 'under-limit native JSON request dispatches'); + +$native_json_dispatched = 0; +$t->post_ok( + '/_test/native-json-limit' => {'Content-Type' => 'application/json'} => + '{"data":"' . ('x' x 512) . '"}' +); +$t->status_is(413); +$t->header_like('Content-Type' => qr{^application/json\b}); +$t->json_is('/error' => 'The request is too large.'); +$t->content_unlike(qr{controller ran}); +is($native_json_dispatched, 0, 'oversized native JSON request does not dispatch'); + +$t->post_ok( + '/csp_report' => {'Content-Type' => 'application/csp-report'} => 'x' x 512 +)->status_is(413) + ->content_is(''); + $t->post_ok( '/post_bug.cgi' => { 'Content-Length' => length($small_body), @@ -134,6 +190,45 @@ $t->post_ok( ->json_is('/code' => 58) ->json_is('/message' => 'The request is too large.'); +for my $reason ( + 'Maximum message size exceeded', + 'Maximum buffer size exceeded' +) { + is( + Bugzilla::App::Plugin::RequestLimit::request_limit_reason( + TestRequest->new($reason) + ), + $reason, + "$reason is rejected before native action dispatch" + ); +} + +for my $reason ('Maximum header size exceeded', 'Maximum start-line size exceeded') { + is( + Bugzilla::App::Plugin::RequestLimit::request_limit_reason( + TestRequest->new($reason) + ), + undef, + "$reason preserves fallback behavior" + ); +} + +is( + Bugzilla::App::Plugin::RequestLimit::request_limit_reason( + TestRequest->new('A future Mojolicious limit error') + ), + 'Unrecognized parser limit', + 'unknown parser limits are rejected safely' +); + +is( + Bugzilla::App::Plugin::RequestLimit::request_limit_reason( + TestRequest->new('Unrelated parser error', 0) + ), + undef, + 'non-limit parser errors retain fallback behavior' +); + $t->app->routes->get('/_test/status-error')->to( cb => sub { my ($c) = @_; From 0324791be5bf1044721c19da0192fe1870692a79 Mon Sep 17 00:00:00 2001 From: Logan Rosen Date: Mon, 5 Oct 2026 21:06:00 -0400 Subject: [PATCH 2/2] Bug 2059957 - Preserve parser-limit errors on REST routes Keep rejecting every Mojolicious parser-limit failure on REST routes, matching the prior API guard. Add an HTTP regression test for a truncated REST header. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Bugzilla/App/Plugin/RequestLimit.pm | 9 ++++--- t/app-cgi-request-limit.t | 40 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/Bugzilla/App/Plugin/RequestLimit.pm b/Bugzilla/App/Plugin/RequestLimit.pm index 92e355b235..6d90356101 100644 --- a/Bugzilla/App/Plugin/RequestLimit.pm +++ b/Bugzilla/App/Plugin/RequestLimit.pm @@ -38,13 +38,13 @@ sub _around_action { return $next->() unless $is_endpoint; return $next->() if $c->isa('Bugzilla::App::Controller::CGI'); - my $reason = request_limit_reason($c->req); + my $format = $c->stash->{request_limit_format} // 'html'; + my $reason = request_limit_reason($c->req, $format eq 'rest'); return $next->() unless defined $reason; my $route = $c->match->endpoint->to_string; WARN("Rejected oversized request for $route: $reason"); - my $format = $c->stash->{request_limit_format} // 'html'; if ($format eq 'rest') { Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); return $c->user_error(REQUEST_TOO_LARGE_ERROR); @@ -68,14 +68,15 @@ sub _around_action { } sub request_limit_reason { - my ($request) = @_; + my ($request, $is_rest) = @_; return undef unless $request->is_limit_exceeded; my $error = $request->error; return 'Unrecognized parser limit' unless ref $error eq 'HASH'; my $reason = $error->{message} // ''; - return undef if $KNOWN_NON_BODY_LIMIT_REASONS{$reason}; + return undef if $KNOWN_NON_BODY_LIMIT_REASONS{$reason} && !$is_rest; + return $reason if $KNOWN_NON_BODY_LIMIT_REASONS{$reason}; return $BODY_LIMIT_REASONS{$reason} ? $reason : 'Unrecognized parser limit'; } diff --git a/t/app-cgi-request-limit.t b/t/app-cgi-request-limit.t index fa782a7fd7..0c55f864c3 100644 --- a/t/app-cgi-request-limit.t +++ b/t/app-cgi-request-limit.t @@ -46,6 +46,21 @@ use Test::Mojo; } } +{ + package TestRequestLimit; + + our $LIMIT_NEXT_HEADER = 0; + + sub lower_next_header_limit { + my ($tx) = @_; + return unless $LIMIT_NEXT_HEADER; + $LIMIT_NEXT_HEADER = 0; + my $request = $tx->req; + my $headers = $request->headers; + $headers->max_line_size(256); + } +} + my $boundary = 'bugzilla-request-limit'; my $body = join( "\r\n", @@ -190,6 +205,23 @@ $t->post_ok( ->json_is('/code' => 58) ->json_is('/message' => 'The request is too large.'); +$TestRequestLimit::LIMIT_NEXT_HEADER = 1; +my $app = $t->app; +$app->hook(after_build_tx => \&TestRequestLimit::lower_next_header_limit); +$t->post_ok( + '/rest/component/Test' => { + 'Content-Length' => 2, + 'Content-Type' => 'application/json', + 'X-Over-Limit-Header' => 'x' x 256, + } => '{}' +); +$t->status_is(413); +$t->header_like('Content-Type' => qr{^application/json\b}); +$t->header_is('Access-Control-Allow-Origin' => '*'); +$t->json_is('/error' => 1); +$t->json_is('/code' => 58); +$t->json_is('/message' => 'The request is too large.'); + for my $reason ( 'Maximum message size exceeded', 'Maximum buffer size exceeded' @@ -211,6 +243,14 @@ for my $reason ('Maximum header size exceeded', 'Maximum start-line size exceede undef, "$reason preserves fallback behavior" ); + is( + Bugzilla::App::Plugin::RequestLimit::request_limit_reason( + TestRequest->new($reason), + 1 + ), + $reason, + "$reason remains rejected for REST" + ); } is(