Merge branch 'pl-json-gon' into 'master'

Don't set gon variables in JSON requests

See merge request gitlab-org/gitlab-ce!21016

3 files changed, 62 insertions, 2 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 7228a2f1715..05ed3669a41 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -20,13 +20,13 @@ class ApplicationController < ActionController::Base
   before_action :ldap_security_check
   before_action :sentry_context
   before_action :default_headers
-  before_action :add_gon_variables, unless: :peek_request?
+  before_action :add_gon_variables, unless: [:peek_request?, :json_request?]
   before_action :configure_permitted_parameters, if: :devise_controller?
   before_action :require_email, unless: :devise_controller?
 
   around_action :set_locale
 
-  after_action :set_page_title_header, if: -> { request.format == :json }
+  after_action :set_page_title_header, if: :json_request?
 
   protect_from_forgery with: :exception, prepend: true
 
@@ -424,6 +424,10 @@ class ApplicationController < ActionController::Base
     request.path.start_with?('/-/peek')
   end
 
+  def json_request?
+    request.format.json?
+  end
+
   def should_enforce_terms?
     return false unless Gitlab::CurrentSettings.current_application_settings.enforce_terms
 
diff --git a/changelogs/unreleased/pl-json-gon.yml b/changelogs/unreleased/pl-json-gon.yml
new file mode 100644
index 00000000000..c0f93006c07
--- /dev/null
+++ b/changelogs/unreleased/pl-json-gon.yml
@@ -0,0 +1,5 @@
+---
+title: Don't set gon variables in JSON requests
+merge_request: 21016
+author: Peter Leitzen
+type: performance
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index bad7a28556c..421ab006792 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -56,6 +56,57 @@ describe ApplicationController do
     end
   end
 
+  describe '#add_gon_variables' do
+    before do
+      Gon.clear
+      sign_in user
+    end
+
+    let(:json_response) { JSON.parse(response.body) }
+
+    controller(described_class) do
+      def index
+        render json: Gon.all_variables
+      end
+    end
+
+    shared_examples 'setting gon variables' do
+      it 'sets gon variables' do
+        get :index, format: format
+
+        expect(json_response.size).not_to be_zero
+      end
+    end
+
+    shared_examples 'not setting gon variables' do
+      it 'does not set gon variables' do
+        get :index, format: format
+
+        expect(json_response.size).to be_zero
+      end
+    end
+
+    context 'with html format' do
+      let(:format) { :html }
+
+      it_behaves_like 'setting gon variables'
+
+      context 'for peek requests' do
+        before do
+          request.path = '/-/peek'
+        end
+
+        it_behaves_like 'not setting gon variables'
+      end
+    end
+
+    context 'with json format' do
+      let(:format) { :json }
+
+      it_behaves_like 'not setting gon variables'
+    end
+  end
+
   describe "#authenticate_user_from_personal_access_token!" do
     before do
       stub_authentication_activity_metrics(debug: false)