Commit daa0cc1c authored by Robert Speicher's avatar Robert Speicher

Merge branch 'directory-names-containing-space' into 'master'

Do not escape URI when extracting path

## What does this MR do?

- remove escaping of URI parameters after they have already been unescaped in `ExtractsPath`
- remove spec that is relying on that behavior

## Why was this MR needed?

- spaces in directory names were escaped leading to 404 errors on repository files page

## What are the relevant issue numbers?

fixes gitlab-com/support-forum#952, #21106

See merge request !5878
parents eae636ed bb48a0fa
...@@ -30,6 +30,7 @@ v 8.11.0 (unreleased) ...@@ -30,6 +30,7 @@ v 8.11.0 (unreleased)
- Fix CI status icon link underline (ClemMakesApps) - Fix CI status icon link underline (ClemMakesApps)
- The Repository class is now instrumented - The Repository class is now instrumented
- Fix commit mention font inconsistency (ClemMakesApps) - Fix commit mention font inconsistency (ClemMakesApps)
- Do not escape URI when extracting path !5878 (winniehell)
- Fix filter label tooltip HTML rendering (ClemMakesApps) - Fix filter label tooltip HTML rendering (ClemMakesApps)
- Cache the commit author in RequestStore to avoid extra lookups in PostReceive - Cache the commit author in RequestStore to avoid extra lookups in PostReceive
- Expand commit message width in repo view (ClemMakesApps) - Expand commit message width in repo view (ClemMakesApps)
......
...@@ -94,7 +94,9 @@ module ExtractsPath ...@@ -94,7 +94,9 @@ module ExtractsPath
@options = params.select {|key, value| allowed_options.include?(key) && !value.blank? } @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? }
@options = HashWithIndifferentAccess.new(@options) @options = HashWithIndifferentAccess.new(@options)
@id = Addressable::URI.normalize_component(get_id) @id = params[:id] || params[:ref]
@id += "/" + params[:path] unless params[:path].blank?
@ref, @path = extract_ref(@id) @ref, @path = extract_ref(@id)
@repo = @project.repository @repo = @project.repository
if @options[:extended_sha1].blank? if @options[:extended_sha1].blank?
...@@ -116,12 +118,4 @@ module ExtractsPath ...@@ -116,12 +118,4 @@ module ExtractsPath
def tree def tree
@tree ||= @repo.tree(@commit.id, @path) @tree ||= @repo.tree(@commit.id, @path)
end end
private
def get_id
id = params[:id] || params[:ref]
id += "/" + params[:path] unless params[:path].blank?
id
end
end end
...@@ -30,17 +30,6 @@ describe ExtractsPath, lib: true do ...@@ -30,17 +30,6 @@ describe ExtractsPath, lib: true do
expect(@logs_path).to eq("/#{@project.path_with_namespace}/refs/#{ref}/logs_tree/files/ruby/popen.rb") expect(@logs_path).to eq("/#{@project.path_with_namespace}/refs/#{ref}/logs_tree/files/ruby/popen.rb")
end end
context 'escaped slash character in ref' do
let(:ref) { 'improve%2Fawesome' }
it 'has no escape sequences in @ref or @logs_path' do
assign_ref_vars
expect(@ref).to eq('improve/awesome')
expect(@logs_path).to eq("/#{@project.path_with_namespace}/refs/#{ref}/logs_tree/files/ruby/popen.rb")
end
end
context 'ref contains %20' do context 'ref contains %20' do
let(:ref) { 'foo%20bar' } let(:ref) { 'foo%20bar' }
...@@ -52,6 +41,16 @@ describe ExtractsPath, lib: true do ...@@ -52,6 +41,16 @@ describe ExtractsPath, lib: true do
expect(@id).to start_with('foo%20bar/') expect(@id).to start_with('foo%20bar/')
end end
end end
context 'path contains space' do
let(:params) { { path: 'with space', ref: '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' } }
it 'is not converted to %20 in @path' do
assign_ref_vars
expect(@path).to eq(params[:path])
end
end
end end
describe '#extract_ref' do describe '#extract_ref' do
......
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