diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a19fcc7a0a..deb60f69bc 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -9,6 +9,7 @@ class ApplicationController < ActionController::Base include UserTrackingConcern include SessionTrackingConcern include CacheConcern + include ErrorResponses include PreloadingConcern include DomainControlHelper include DatabaseHelper @@ -23,21 +24,6 @@ class ApplicationController < ActionController::Base helper_method :limited_federation_mode? helper_method :skip_csrf_meta_tags? - rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request - rescue_from Mastodon::NotPermittedError, with: :forbidden - rescue_from ActionController::RoutingError, ActiveRecord::RecordNotFound, with: :not_found - rescue_from ActionController::UnknownFormat, with: :not_acceptable - rescue_from ActionController::InvalidAuthenticityToken, with: :unprocessable_content - rescue_from Mastodon::RateLimitExceededError, with: :too_many_requests - - rescue_from(*Mastodon::HTTP_CONNECTION_ERRORS, with: :internal_server_error) - rescue_from Mastodon::RaceConditionError, Stoplight::Error::RedLight, ActiveRecord::SerializationFailure, with: :service_unavailable - - rescue_from Seahorse::Client::NetworkingError do |e| - Rails.logger.warn "Storage server error: #{e}" - service_unavailable - end - before_action :check_self_destruct! before_action :store_referrer, except: :raise_not_found, if: :devise_controller? @@ -118,42 +104,6 @@ class ApplicationController < ActionController::Base ActiveModel::Type::Boolean.new.cast(params[key]) end - def forbidden - respond_with_error(403) - end - - def not_found - respond_with_error(404) - end - - def gone - respond_with_error(410) - end - - def unprocessable_content - respond_with_error(422) - end - - def not_acceptable - respond_with_error(406) - end - - def bad_request - respond_with_error(400) - end - - def internal_server_error - respond_with_error(500) - end - - def service_unavailable - respond_with_error(503) - end - - def too_many_requests - respond_with_error(429) - end - def single_user_mode? @single_user_mode ||= Rails.configuration.x.single_user_mode && Account.without_internal.exists? end @@ -178,13 +128,6 @@ class ApplicationController < ActionController::Base @current_session = SessionActivation.find_by(session_id: cookies.signed['_session_id']) if cookies.signed['_session_id'].present? end - def respond_with_error(code) - respond_to do |format| - format.any { render "errors/#{code}", layout: 'error', status: code, formats: [:html] } - format.json { render json: { error: Rack::Utils::HTTP_STATUS_CODES[code] }, status: code } - end - end - def check_self_destruct! return unless self_destruct? diff --git a/app/controllers/concerns/error_responses.rb b/app/controllers/concerns/error_responses.rb new file mode 100644 index 0000000000..402ade0066 --- /dev/null +++ b/app/controllers/concerns/error_responses.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module ErrorResponses + extend ActiveSupport::Concern + + included do + rescue_from ActionController::InvalidAuthenticityToken, with: :unprocessable_content + rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request + rescue_from ActionController::RoutingError, ActiveRecord::RecordNotFound, with: :not_found + rescue_from ActionController::UnknownFormat, with: :not_acceptable + rescue_from Mastodon::NotPermittedError, with: :forbidden + rescue_from Mastodon::RaceConditionError, Stoplight::Error::RedLight, ActiveRecord::SerializationFailure, with: :service_unavailable + rescue_from Mastodon::RateLimitExceededError, with: :too_many_requests + rescue_from(*Mastodon::HTTP_CONNECTION_ERRORS, with: :internal_server_error) + + rescue_from Seahorse::Client::NetworkingError do |e| + Rails.logger.warn "Storage server error: #{e}" + service_unavailable + end + end + + protected + + def bad_request + respond_with_error(400) + end + + def forbidden + respond_with_error(403) + end + + def gone + respond_with_error(410) + end + + def internal_server_error + respond_with_error(500) + end + + def not_acceptable + respond_with_error(406) + end + + def not_found + respond_with_error(404) + end + + def service_unavailable + respond_with_error(503) + end + + def too_many_requests + respond_with_error(429) + end + + def unprocessable_content + respond_with_error(422) + end + + private + + def respond_with_error(code) + respond_to do |format| + format.any { render "errors/#{code}", layout: 'error', formats: [:html], status: code } + format.json { render json: { error: Rack::Utils::HTTP_STATUS_CODES[code] }, status: code } + end + end +end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 24b775a5ad..898edb3e23 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -6,52 +6,21 @@ RSpec.describe ApplicationController do render_views controller do - def success - head 200 - end - - def routing_error - raise ActionController::RoutingError, '' - end - - def record_not_found - raise ActiveRecord::RecordNotFound, '' - end - - def invalid_authenticity_token - raise ActionController::InvalidAuthenticityToken, '' - end - end - - shared_examples 'error response' do |code| - it "returns http #{code} for http and renders template" do - subject - - expect(response) - .to have_http_status(code) - expect(response.parsed_body) - .to have_css('body[class=error]') - expect(response.parsed_body.css('h1').to_s) - .to include(error_content(code)) - end - - def error_content(code) - if code == 422 - I18n.t('errors.422.content') - else - I18n.t("errors.#{code}") - end - end + def success = head(200) end context 'with a forgery' do - subject do + before do ActionController::Base.allow_forgery_protection = true routes.draw { post 'success' => 'anonymous#success' } - post 'success' end - it_behaves_like 'error response', 422 + it 'responds with 422 and error page' do + post 'success' + + expect(response) + .to have_http_status(422) + end end describe 'helper_method :current_account' do @@ -85,33 +54,6 @@ RSpec.describe ApplicationController do end end - context 'with ActionController::RoutingError' do - subject do - routes.draw { get 'routing_error' => 'anonymous#routing_error' } - get 'routing_error' - end - - it_behaves_like 'error response', 404 - end - - context 'with ActiveRecord::RecordNotFound' do - subject do - routes.draw { get 'record_not_found' => 'anonymous#record_not_found' } - get 'record_not_found' - end - - it_behaves_like 'error response', 404 - end - - context 'with ActionController::InvalidAuthenticityToken' do - subject do - routes.draw { get 'invalid_authenticity_token' => 'anonymous#invalid_authenticity_token' } - get 'invalid_authenticity_token' - end - - it_behaves_like 'error response', 422 - end - describe 'before_action :check_suspension' do before do routes.draw { get 'success' => 'anonymous#success' } @@ -141,64 +83,4 @@ RSpec.describe ApplicationController do expect { controller.raise_not_found }.to raise_error(ActionController::RoutingError, 'No route matches unmatched') end end - - describe 'forbidden' do - controller do - def route_forbidden - forbidden - end - end - - subject do - routes.draw { get 'route_forbidden' => 'anonymous#route_forbidden' } - get 'route_forbidden' - end - - it_behaves_like 'error response', 403 - end - - describe 'not_found' do - controller do - def route_not_found - not_found - end - end - - subject do - routes.draw { get 'route_not_found' => 'anonymous#route_not_found' } - get 'route_not_found' - end - - it_behaves_like 'error response', 404 - end - - describe 'gone' do - controller do - def route_gone - gone - end - end - - subject do - routes.draw { get 'route_gone' => 'anonymous#route_gone' } - get 'route_gone' - end - - it_behaves_like 'error response', 410 - end - - describe 'unprocessable_content' do - controller do - def route_unprocessable_content - unprocessable_content - end - end - - subject do - routes.draw { get 'route_unprocessable_content' => 'anonymous#route_unprocessable_content' } - get 'route_unprocessable_content' - end - - it_behaves_like 'error response', 422 - end end diff --git a/spec/controllers/concerns/error_responses_spec.rb b/spec/controllers/concerns/error_responses_spec.rb new file mode 100644 index 0000000000..678d876518 --- /dev/null +++ b/spec/controllers/concerns/error_responses_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ErrorResponses do + render_views + + shared_examples 'error response' do |code| + before { routes.draw { get 'show' => 'anonymous#show' } } + + it "returns http #{code} and renders error template" do + get 'show' + + expect(response) + .to have_http_status(code) + expect(response.parsed_body) + .to have_css('body[class=error]') + .and have_css('h1', text: error_content(code)) + end + + def error_content(code) + I18n.t("errors.#{code}") + .then { |value| I18n.t("errors.#{code}.content") if value.is_a?(Hash) } + end + end + + describe 'bad_request' do + controller(ApplicationController) do + def show = bad_request + end + + it_behaves_like 'error response', 400 + end + + describe 'forbidden' do + controller(ApplicationController) do + def show = forbidden + end + + it_behaves_like 'error response', 403 + end + + describe 'gone' do + controller(ApplicationController) do + def show = gone + end + + it_behaves_like 'error response', 410 + end + + describe 'internal_server_error' do + controller(ApplicationController) do + def show = internal_server_error + end + + it_behaves_like 'error response', 500 + end + + describe 'not_acceptable' do + controller(ApplicationController) do + def show = not_acceptable + end + + it_behaves_like 'error response', 406 + end + + describe 'not_found' do + controller(ApplicationController) do + def show = not_found + end + + it_behaves_like 'error response', 404 + end + + describe 'service_unavailable' do + controller(ApplicationController) do + def show = service_unavailable + end + + it_behaves_like 'error response', 503 + end + + describe 'too_many_requests' do + controller(ApplicationController) do + def show = too_many_requests + end + + it_behaves_like 'error response', 429 + end + + describe 'unprocessable_content' do + controller(ApplicationController) do + def show = unprocessable_content + end + + it_behaves_like 'error response', 422 + end + + context 'with ActionController::RoutingError' do + controller(ApplicationController) do + def show + raise ActionController::RoutingError, '' + end + end + + it_behaves_like 'error response', 404 + end + + context 'with ActiveRecord::RecordNotFound' do + controller(ApplicationController) do + def show + raise ActiveRecord::RecordNotFound, '' + end + end + + it_behaves_like 'error response', 404 + end + + context 'with ActionController::InvalidAuthenticityToken' do + controller(ApplicationController) do + def show + raise ActionController::InvalidAuthenticityToken, '' + end + end + + it_behaves_like 'error response', 422 + end +end