Merge branch 'use_strong_params_for_extracts_ref' into 'master'

Use StrongParameters for ExtractsRef

See merge request gitlab-org/gitlab!79997

lib/extracts_ref.rb

@@ -126,8 +126,10 @@ module ExtractsRef
 
   # overridden in subclasses, do not remove
   def get_id
-    id = [params[:id] || params[:ref]]
-    id << "/" + params[:path] unless params[:path].blank?
+    allowed_params = params.permit(:id, :ref, :path)
+
+    id = [allowed_params[:id] || allowed_params[:ref]]
+    id << "/" + allowed_params[:path] unless allowed_params[:path].blank?
     id.join
   end
 

spec/lib/extracts_path_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe ExtractsPath do
   describe '#assign_ref_vars' do
     let(:ref) { sample_commit[:id] }
     let(:path) { sample_commit[:line_code_path] }
-    let(:params) { { path: path, ref: ref } }
+    let(:params) { ActionController::Parameters.new(path: path, ref: ref) }
 
     it_behaves_like 'assigns ref vars'
 
@@ -54,7 +54,8 @@ RSpec.describe ExtractsPath do
 
     context 'ref only exists without .atom suffix' do
       context 'with a path' do
-        let(:params) { { ref: 'v1.0.0.atom', path: 'README.md' } }
+        let(:ref) { 'v1.0.0.atom' }
+        let(:path) { 'README.md' }
 
         it 'renders a 404' do
           expect(self).to receive(:render_404)
@@ -64,7 +65,8 @@ RSpec.describe ExtractsPath do
       end
 
       context 'without a path' do
-        let(:params) { { ref: 'v1.0.0.atom' } }
+        let(:ref) { 'v1.0.0.atom' }
+        let(:path) { nil }
 
         before do
           assign_ref_vars
@@ -82,7 +84,8 @@ RSpec.describe ExtractsPath do
 
     context 'ref exists with .atom suffix' do
       context 'with a path' do
-        let(:params) { { ref: 'master.atom', path: 'README.md' } }
+        let(:ref) { 'master.atom' }
+        let(:path) { 'README.md' }
 
         before do
           repository = @project.repository
@@ -102,7 +105,8 @@ RSpec.describe ExtractsPath do
       end
 
       context 'without a path' do
-        let(:params) { { ref: 'master.atom' } }
+        let(:ref) { 'master.atom' }
+        let(:path) { nil }
 
         before do
           repository = @project.repository
@@ -125,7 +129,8 @@ RSpec.describe ExtractsPath do
     end
 
     context 'ref and path are nil' do
-      let(:params) { { path: nil, ref: nil } }
+      let(:path) { nil }
+      let(:ref) { nil }
 
       it 'does not set commit' do
         expect(container.repository).not_to receive(:commit).with('')

spec/lib/extracts_ref_spec.rb

@@ -10,7 +10,8 @@ RSpec.describe ExtractsRef do
   let_it_be(:container) { create(:snippet, :repository, author: owner) }
 
   let(:ref) { sample_commit[:id] }
-  let(:params) { { path: sample_commit[:line_code_path], ref: ref } }
+  let(:path) { sample_commit[:line_code_path] }
+  let(:params) { ActionController::Parameters.new(path: path, ref: ref) }
 
   before do
     ref_names = ['master', 'foo/bar/baz', 'v1.0.0', 'v2.0.0', 'release/app', 'release/app/v1.0.0']
@@ -23,7 +24,8 @@ RSpec.describe ExtractsRef do
     it_behaves_like 'assigns ref vars'
 
     context 'ref and path are nil' do
-      let(:params) { { path: nil, ref: nil } }
+      let(:ref) { nil }
+      let(:path) { nil }
 
       it 'does not set commit' do
         expect(container.repository).not_to receive(:commit).with('')
@@ -33,6 +35,15 @@ RSpec.describe ExtractsRef do
         expect(@commit).to be_nil
       end
     end
+
+    context 'when ref and path have incorrect format' do
+      let(:ref) { { wrong: :format } }
+      let(:path) { { also: :wrong } }
+
+      it 'does not raise an exception' do
+        expect { assign_ref_vars }.not_to raise_error
+      end
+    end
   end
 
   it_behaves_like 'extracts refs'

spec/support/shared_examples/path_extraction_shared_examples.rb

@@ -40,12 +40,13 @@ RSpec.shared_examples 'assigns ref vars' do
   end
 
   context 'path contains space' do
-    let(:params) { { path: 'with space', ref: '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' } }
+    let(:ref) { '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' }
+    let(:path) { 'with space' }
 
     it 'is not converted to %20 in @path' do
       assign_ref_vars
 
-      expect(@path).to eq(params[:path])
+      expect(@path).to eq(path)
     end
   end