From c6a4c9231e983f2bd5f0c2115a5c9c16fd18bfb2 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Sat, 13 Oct 2018 19:45:27 +0200 Subject: Use InvalidUTF8ErrorHandler only for rails 4 In Rails 5 catches invalid UTF8 characters in querystring in a params middleware, errors are handled by a params middleware and raises a BadRequest exception. This means that these UTF8 errors are not raised deeper in application stack and these can't also be handled on application level. If we would want to have custom handler for these errors, we would have to create a new middleware and insert it before actionpack's params middleware and rescue BadRequest exceptions there. But there is no need to do this currently (see discussion on https://gitlab.com/gitlab-org/gitlab-ce/issues/51908) --- app/controllers/application_controller.rb | 4 +++- spec/controllers/application_controller_spec.rb | 26 ++++++++++++++++++------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bbeaeb7694e..eeabcc0c9bb 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -12,7 +12,9 @@ class ApplicationController < ActionController::Base include WorkhorseHelper include EnforcesTwoFactorAuthentication include WithPerformanceBar - include InvalidUTF8ErrorHandler + # this can be removed after switching to rails 5 + # https://gitlab.com/gitlab-org/gitlab-ce/issues/51908 + include InvalidUTF8ErrorHandler unless Gitlab.rails5? before_action :authenticate_sessionless_user! before_action :authenticate_user! diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index a8556771edd..be3fc832008 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -685,22 +685,34 @@ describe ApplicationController do end context 'html' do + subject { get :index, text: "hi \255" } + it 'renders 412' do - get :index, text: "hi \255" + if Gitlab.rails5? + expect { subject }.to raise_error(ActionController::BadRequest) + else + subject - expect(response).to have_gitlab_http_status(412) - expect(response).to render_template :precondition_failed + expect(response).to have_gitlab_http_status(412) + expect(response).to render_template :precondition_failed + end end end context 'js' do + subject { get :index, text: "hi \255", format: :js } + it 'renders 412' do - get :index, text: "hi \255", format: :js + if Gitlab.rails5? + expect { subject }.to raise_error(ActionController::BadRequest) + else + subject - json_response = JSON.parse(response.body) + json_response = JSON.parse(response.body) - expect(response).to have_gitlab_http_status(412) - expect(json_response['error']).to eq('Invalid UTF-8') + expect(response).to have_gitlab_http_status(412) + expect(json_response['error']).to eq('Invalid UTF-8') + end end end end -- cgit v1.2.1