diff --git a/Bugzilla/App/Controller/API.pm b/Bugzilla/App/Controller/API.pm index d1b4c6cc53..767a66191d 100644 --- a/Bugzilla/App/Controller/API.pm +++ b/Bugzilla/App/Controller/API.pm @@ -17,8 +17,10 @@ use Try::Tiny; use Bugzilla::Constants; 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) = @_; @@ -30,38 +32,17 @@ sub setup_routes { $r->namespaces($namespaces); # Backwards compat with /api/user/profile which Phabricator requires - $r->under( - '/api' => sub { - my ($c) = @_; - _insert_rest_headers($c); - Bugzilla->usage_mode(USAGE_MODE_REST); - } - )->get('/user/profile')->to('V1::User#user_profile'); + $r->under('/api' => \&_prepare_rest_request) + ->get('/user/profile')->to('V1::User#user_profile'); # Other backwards compat routes - $r->under( - '/latest' => sub { - my ($c) = @_; - _insert_rest_headers($c); - Bugzilla->usage_mode(USAGE_MODE_REST); - } - )->get('/configuration')->to('V1::Configuration#configuration'); - $r->under( - '/bzapi' => sub { - my ($c) = @_; - _insert_rest_headers($c); - Bugzilla->usage_mode(USAGE_MODE_REST); - } - )->get('/configuration')->to('V1::Configuration#configuration'); + $r->under('/latest' => \&_prepare_rest_request) + ->get('/configuration')->to('V1::Configuration#configuration'); + $r->under('/bzapi' => \&_prepare_rest_request) + ->get('/configuration')->to('V1::Configuration#configuration'); # Set the usage mode for all routes under /rest - my $rest_routes = $r->under( - '/rest' => sub { - my ($c) = @_; - _insert_rest_headers($c); - Bugzilla->usage_mode(USAGE_MODE_REST); - } - ); + my $rest_routes = $r->under('/rest' => \&_prepare_rest_request); # Standard API support foreach my $version (SUPPORTED_VERSIONS) { @@ -89,6 +70,21 @@ 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); + } + + Bugzilla->usage_mode(USAGE_MODE_REST); + return 1; +} + sub _load_api_module { my ($routes, $module) = @_; try { @@ -105,15 +101,9 @@ sub _load_api_module { sub _insert_rest_headers { my ($c) = @_; - - # Access Control my @allowed_headers = qw(accept authorization content-type origin user-agent x-bugzilla-api-key x-requested-with); - $c->res->headers->header('Access-Control-Allow-Origin' => '*'); - $c->res->headers->header( - 'Access-Control-Allow-Headers' => join ', ', - @allowed_headers - ); + set_rest_cors_headers($c->res->headers, \@allowed_headers); } 1; diff --git a/Bugzilla/App/Controller/CGI.pm b/Bugzilla/App/Controller/CGI.pm index b8f133b007..81ca94f26c 100644 --- a/Bugzilla/App/Controller/CGI.pm +++ b/Bugzilla/App/Controller/CGI.pm @@ -17,9 +17,17 @@ use Socket qw(AF_INET inet_aton); use Mojo::File qw(path); use English qw(-no_match_vars); use Bugzilla::App::Stdout; -use Bugzilla::Constants qw(bz_locations USAGE_MODE_BROWSER); +use Bugzilla::Constants qw( + bz_locations + USAGE_MODE_BROWSER + USAGE_MODE_MOJO + USAGE_MODE_MOJO_REST +); +use Bugzilla::Logging; +use Bugzilla::WebService::Util qw(set_rest_cors_headers); my %SEEN; +use constant REQUEST_TOO_LARGE_ERROR => 'request_too_large'; sub setup_routes { my ($class, $r) = @_; @@ -60,6 +68,13 @@ sub load_one { my $inner = quote_sub $inner_name, $content, {}, \%options; my $wrapper = sub { my ($c) = @_; + + if ($c->req->is_limit_exceeded) { + my $reason = $c->req->error->{message}; + WARN("Rejected oversized request for $file: $reason"); + return _render_request_too_large($c, $file); + } + Bugzilla->request_cache->{mojo_controller} = $c; my $stdin = $c->_STDIN; local %ENV = $c->_ENV($file); @@ -94,6 +109,23 @@ sub load_one { return 1; } +sub _render_request_too_large { + my ($c, $file) = @_; + + if (path($file)->basename eq 'rest.cgi') { + set_rest_cors_headers($c->res->headers); + Bugzilla->usage_mode(USAGE_MODE_MOJO_REST); + return $c->user_error(REQUEST_TOO_LARGE_ERROR); + } + + Bugzilla->usage_mode(USAGE_MODE_MOJO); + return $c->user_error( + REQUEST_TOO_LARGE_ERROR, + {}, + {status => 413, skip_exception_page => 1} + ); +} + sub _ENV { my ($c, $script_name) = @_; my $tx = $c->tx; diff --git a/Bugzilla/App/Plugin/Error.pm b/Bugzilla/App/Plugin/Error.pm index 676a2e836e..f119a4dce1 100644 --- a/Bugzilla/App/Plugin/Error.pm +++ b/Bugzilla/App/Plugin/Error.pm @@ -28,7 +28,8 @@ sub register { } sub _render_error { - my ($type, $c, $error, $vars) = @_; + my ($type, $c, $error, $vars, $options) = @_; + $options ||= {}; # If values are defined in the stash, use those instead my $stash = $c->stash; @@ -49,7 +50,7 @@ sub _render_error { if (Bugzilla->usage_mode == USAGE_MODE_MOJO) { $logfunc->("webpage error: $error"); - if ($c->app->mode eq 'development') { + if ($c->app->mode eq 'development' && !$options->{skip_exception_page}) { use Bugzilla::Logging; my $class = $type ? 'Bugzilla::Error::' . ucfirst($type) : 'Mojo::Exception'; my $e = $class->new($error)->trace(2); @@ -63,8 +64,8 @@ sub _render_error { template => "global/$type-error", format => 'html', error => $error, - status => 200, - %{$vars} + %{$vars}, + status => $options->{status} // $vars->{status} // 200 ); return 0; } diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 443a7dbf7a..af8bd05342 100644 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -70,6 +70,7 @@ use constant WS_ERROR_CODE => { number_too_small => 55, illegal_date => 56, illegal_date_pronoun => 57, + request_too_large => 58, # Bug errors usually occupy the 100-200 range. improper_bug_id_field_value => 100, @@ -255,6 +256,7 @@ use constant STATUS_BAD_REQUEST => 400; use constant STATUS_NOT_AUTHORIZED => 401; use constant STATUS_NOT_FOUND => 404; use constant STATUS_GONE => 410; +use constant STATUS_REQUEST_TOO_LARGE => 413; # The integer value is the error code above returned by # the related webvservice call. We choose the appropriate @@ -263,6 +265,7 @@ use constant STATUS_GONE => 410; sub REST_STATUS_CODE_MAP { my $status_code_map = { 51 => STATUS_NOT_FOUND, + 58 => STATUS_REQUEST_TOO_LARGE, 101 => STATUS_NOT_FOUND, 102 => STATUS_NOT_AUTHORIZED, 106 => STATUS_NOT_AUTHORIZED, diff --git a/Bugzilla/WebService/Server/REST.pm b/Bugzilla/WebService/Server/REST.pm index aeb6fc66b7..e86f742636 100644 --- a/Bugzilla/WebService/Server/REST.pm +++ b/Bugzilla/WebService/Server/REST.pm @@ -19,7 +19,7 @@ use Bugzilla::Error; use Bugzilla::Hook; use Bugzilla::Util qw(html_quote disable_utf8 enable_utf8); use Bugzilla::WebService::Constants; -use Bugzilla::WebService::Util qw(taint_data fix_credentials); +use Bugzilla::WebService::Util qw(fix_credentials set_rest_cors_headers taint_data); # Load resource modules use Bugzilla::WebService::Server::REST::Resources::Bug; @@ -141,18 +141,7 @@ sub response { Bugzilla::Hook::process('webservice_rest_response', {rpc => $self, result => \$result, response => $response}); - # Access Control - my @allowed_headers - = qw(accept content-type origin user-agent x-requested-with); - foreach my $header (keys %{API_AUTH_HEADERS()}) { - - # We want to lowercase and replace _ with - - my $translated_header = $header; - $translated_header =~ tr/A-Z_/a-z\-/; - push(@allowed_headers, $translated_header); - } - $response->header("Access-Control-Allow-Origin", "*"); - $response->header("Access-Control-Allow-Headers", join(', ', @allowed_headers)); + set_rest_cors_headers($response); # ETag support my $etag = $self->bz_etag; diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 7e92dc40bf..32b34a4bb0 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -37,8 +37,26 @@ our @EXPORT_OK = qw( translate params_to_objects fix_credentials + set_rest_cors_headers ); +sub set_rest_cors_headers { + my ($headers, $allowed_headers) = @_; + if (!$allowed_headers) { + my @default_headers + = qw(accept authorization content-type origin user-agent x-requested-with); + foreach my $header (sort keys %{API_AUTH_HEADERS()}) { + $header =~ tr/A-Z_/a-z\-/; + push @default_headers, $header; + } + $allowed_headers = \@default_headers; + } + + $headers->header('Access-Control-Allow-Origin' => '*'); + $headers->header( + 'Access-Control-Allow-Headers' => join(', ', @$allowed_headers)); +} + sub extract_flags { my ($flags, $bug, $attachment) = @_; my (@new_flags, @old_flags); diff --git a/t/app-cgi-request-limit.t b/t/app-cgi-request-limit.t new file mode 100644 index 0000000000..8fe5fdd720 --- /dev/null +++ b/t/app-cgi-request-limit.t @@ -0,0 +1,148 @@ +#!/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 local/lib/perl5 ); + +BEGIN { + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{MOJO_MAX_BUFFER_SIZE} = 64; + $ENV{MOJO_MAX_MESSAGE_SIZE} = 512; +} + +use Bugzilla::Test::MockLocalconfig (urlbase => 'http://bmo.test'); +use Bugzilla::Test::MockDB; +use Bugzilla::Test::MockParams; + +use Test2::V0; +use Test::Mojo; + +my $boundary = 'bugzilla-request-limit'; +my $body = join( + "\r\n", + "--$boundary", + 'Content-Disposition: form-data; name="bug_type"', + '', + 'defect', + "--$boundary", + 'Content-Disposition: form-data; name="data"; filename="large.txt"', + 'Content-Type: text/plain', + '', + 'x' x 512, + "--$boundary--", + '' +); +my $small_body = join( + "\r\n", + "--$boundary", + 'Content-Disposition: form-data; name="bug_type"', + '', + 'defect', + "--$boundary--", + '' +); + +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); + +$t->post_ok( + '/post_bug.cgi' => { + 'Content-Length' => length($small_body), + 'Content-Type' => "multipart/form-data; boundary=$boundary", + } => $small_body +)->status_is(200); + +$t->post_ok( + '/index.cgi' => { + 'Content-Length' => 128, + 'Content-Type' => "multipart/form-data; boundary=$boundary", + } => 'x' x 128 +)->status_is(413) + ->content_like(qr{The request is too large\.}); + +$t->post_ok( + '/post_bug.cgi' => { + 'Content-Length' => length($body), + 'Content-Type' => "multipart/form-data; boundary=$boundary", + } => $body +)->status_is(413) + ->header_like('Content-Type' => qr{^text/html\b}) + ->content_like(qr{