Commit 170f7906 authored by Jason Goodman's avatar Jason Goodman Committed by Kamil Trzciński

Use Array.join rather than URI.join

Add spec for cases where URI.join does not work as expected
parent 4f750ccf
...@@ -204,7 +204,7 @@ class Environment < ApplicationRecord ...@@ -204,7 +204,7 @@ class Environment < ApplicationRecord
public_path = project.public_path_for_source_path(path, commit_sha) public_path = project.public_path_for_source_path(path, commit_sha)
return unless public_path return unless public_path
[external_url, public_path].join('/') [external_url.delete_suffix('/'), public_path.delete_prefix('/')].join('/')
end end
def expire_etag_cache def expire_etag_cache
......
---
title: Prevent double slash in review apps path
merge_request: 31212
author:
type: fixed
...@@ -4,6 +4,7 @@ require 'spec_helper' ...@@ -4,6 +4,7 @@ require 'spec_helper'
describe Environment, :use_clean_rails_memory_store_caching do describe Environment, :use_clean_rails_memory_store_caching do
include ReactiveCachingHelpers include ReactiveCachingHelpers
using RSpec::Parameterized::TableSyntax
let(:project) { create(:project, :stubbed_repository) } let(:project) { create(:project, :stubbed_repository) }
subject(:environment) { create(:environment, project: project) } subject(:environment) { create(:environment, project: project) }
...@@ -782,12 +783,9 @@ describe Environment, :use_clean_rails_memory_store_caching do ...@@ -782,12 +783,9 @@ describe Environment, :use_clean_rails_memory_store_caching do
let(:source_path) { 'source/file.html' } let(:source_path) { 'source/file.html' }
let(:sha) { RepoHelpers.sample_commit.id } let(:sha) { RepoHelpers.sample_commit.id }
before do
environment.external_url = 'http://example.com'
end
context 'when the public path is not known' do context 'when the public path is not known' do
before do before do
environment.external_url = 'http://example.com'
allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(nil) allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(nil)
end end
...@@ -797,12 +795,23 @@ describe Environment, :use_clean_rails_memory_store_caching do ...@@ -797,12 +795,23 @@ describe Environment, :use_clean_rails_memory_store_caching do
end end
context 'when the public path is known' do context 'when the public path is known' do
before do where(:external_url, :public_path, :full_url) do
allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return('file.html') 'http://example.com' | 'file.html' | 'http://example.com/file.html'
end 'http://example.com/' | 'file.html' | 'http://example.com/file.html'
'http://example.com' | '/file.html' | 'http://example.com/file.html'
'http://example.com/' | '/file.html' | 'http://example.com/file.html'
'http://example.com/subpath' | 'public/file.html' | 'http://example.com/subpath/public/file.html'
'http://example.com/subpath/' | 'public/file.html' | 'http://example.com/subpath/public/file.html'
'http://example.com/subpath' | '/public/file.html' | 'http://example.com/subpath/public/file.html'
'http://example.com/subpath/' | '/public/file.html' | 'http://example.com/subpath/public/file.html'
end
with_them do
it 'returns the full external URL' do it 'returns the full external URL' do
expect(environment.external_url_for(source_path, sha)).to eq('http://example.com/file.html') environment.external_url = external_url
allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(public_path)
expect(environment.external_url_for(source_path, sha)).to eq(full_url)
end
end end
end end
end end
......
...@@ -2950,6 +2950,16 @@ describe Project do ...@@ -2950,6 +2950,16 @@ describe Project do
expect(project.public_path_for_source_path('file.html', sha)).to be_nil expect(project.public_path_for_source_path('file.html', sha)).to be_nil
end end
end end
it 'returns a public path with a leading slash unmodified' do
route_map = Gitlab::RouteMap.new(<<-MAP.strip_heredoc)
- source: 'source/file.html'
public: '/public/file'
MAP
allow(project).to receive(:route_map_for).with(sha).and_return(route_map)
expect(project.public_path_for_source_path('source/file.html', sha)).to eq('/public/file')
end
end end
context 'when there is no route map' do context 'when there is no route map' 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