Commit 32683c2d authored by Vasilii Iakliushin's avatar Vasilii Iakliushin

Use StrongParameters for ExtractsRef

* Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/351520
* Sentry error:
https://sentry.gitlab.net/gitlab/gitlabcom/issues/3154433

Changelog: fixed
parent d088bff9
...@@ -126,8 +126,10 @@ module ExtractsRef ...@@ -126,8 +126,10 @@ module ExtractsRef
# overridden in subclasses, do not remove # overridden in subclasses, do not remove
def get_id def get_id
id = [params[:id] || params[:ref]] allowed_params = params.permit(:id, :ref, :path)
id << "/" + params[:path] unless params[:path].blank?
id = [allowed_params[:id] || allowed_params[:ref]]
id << "/" + allowed_params[:path] unless allowed_params[:path].blank?
id.join id.join
end end
......
...@@ -32,7 +32,7 @@ RSpec.describe ExtractsPath do ...@@ -32,7 +32,7 @@ RSpec.describe ExtractsPath do
describe '#assign_ref_vars' do describe '#assign_ref_vars' do
let(:ref) { sample_commit[:id] } let(:ref) { sample_commit[:id] }
let(:path) { sample_commit[:line_code_path] } 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' it_behaves_like 'assigns ref vars'
...@@ -54,7 +54,8 @@ RSpec.describe ExtractsPath do ...@@ -54,7 +54,8 @@ RSpec.describe ExtractsPath do
context 'ref only exists without .atom suffix' do context 'ref only exists without .atom suffix' do
context 'with a path' 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 it 'renders a 404' do
expect(self).to receive(:render_404) expect(self).to receive(:render_404)
...@@ -64,7 +65,8 @@ RSpec.describe ExtractsPath do ...@@ -64,7 +65,8 @@ RSpec.describe ExtractsPath do
end end
context 'without a path' do context 'without a path' do
let(:params) { { ref: 'v1.0.0.atom' } } let(:ref) { 'v1.0.0.atom' }
let(:path) { nil }
before do before do
assign_ref_vars assign_ref_vars
...@@ -82,7 +84,8 @@ RSpec.describe ExtractsPath do ...@@ -82,7 +84,8 @@ RSpec.describe ExtractsPath do
context 'ref exists with .atom suffix' do context 'ref exists with .atom suffix' do
context 'with a path' do context 'with a path' do
let(:params) { { ref: 'master.atom', path: 'README.md' } } let(:ref) { 'master.atom' }
let(:path) { 'README.md' }
before do before do
repository = @project.repository repository = @project.repository
...@@ -102,7 +105,8 @@ RSpec.describe ExtractsPath do ...@@ -102,7 +105,8 @@ RSpec.describe ExtractsPath do
end end
context 'without a path' do context 'without a path' do
let(:params) { { ref: 'master.atom' } } let(:ref) { 'master.atom' }
let(:path) { nil }
before do before do
repository = @project.repository repository = @project.repository
...@@ -125,7 +129,8 @@ RSpec.describe ExtractsPath do ...@@ -125,7 +129,8 @@ RSpec.describe ExtractsPath do
end end
context 'ref and path are nil' do 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 it 'does not set commit' do
expect(container.repository).not_to receive(:commit).with('') expect(container.repository).not_to receive(:commit).with('')
......
...@@ -10,7 +10,8 @@ RSpec.describe ExtractsRef do ...@@ -10,7 +10,8 @@ RSpec.describe ExtractsRef do
let_it_be(:container) { create(:snippet, :repository, author: owner) } let_it_be(:container) { create(:snippet, :repository, author: owner) }
let(:ref) { sample_commit[:id] } 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 before do
ref_names = ['master', 'foo/bar/baz', 'v1.0.0', 'v2.0.0', 'release/app', 'release/app/v1.0.0'] 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 ...@@ -23,7 +24,8 @@ RSpec.describe ExtractsRef do
it_behaves_like 'assigns ref vars' it_behaves_like 'assigns ref vars'
context 'ref and path are nil' do 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 it 'does not set commit' do
expect(container.repository).not_to receive(:commit).with('') expect(container.repository).not_to receive(:commit).with('')
...@@ -33,6 +35,15 @@ RSpec.describe ExtractsRef do ...@@ -33,6 +35,15 @@ RSpec.describe ExtractsRef do
expect(@commit).to be_nil expect(@commit).to be_nil
end end
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 end
it_behaves_like 'extracts refs' it_behaves_like 'extracts refs'
......
...@@ -40,12 +40,13 @@ RSpec.shared_examples 'assigns ref vars' do ...@@ -40,12 +40,13 @@ RSpec.shared_examples 'assigns ref vars' do
end end
context 'path contains space' do 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 it 'is not converted to %20 in @path' do
assign_ref_vars assign_ref_vars
expect(@path).to eq(params[:path]) expect(@path).to eq(path)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment