diff --git a/CHANGELOG b/CHANGELOG
index 31feb115d1e8599c2b49aab7178c173c365044d9..87baa6b66217813ae67a044c7d149ea885df64fb 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -53,6 +53,7 @@ v 8.1.0 (unreleased)
- Add "New Page" button to Wiki Pages tab (Stan Hu)
- Only render 404 page from /public
- Hide passwords from services API (Alex Lossent)
+ - Fix: Images cannot show when projects' path was changed
v 8.0.4
- Fix Message-ID header to be RFC 2111-compliant to prevent e-mails being dropped (Stan Hu)
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index bc8525df5a5097bdf4f793057251a0e15fc182c5..5782e649f8be15100eadec612886b6ea2f26e5ae 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -118,6 +118,8 @@ class Namespace < ActiveRecord::Base
gitlab_shell.add_namespace(path_was)
if gitlab_shell.mv_namespace(path_was, path)
+ Gitlab::UploadsTransfer.new.rename_namespace(path_was, path)
+
# If repositories moved successfully we need to
# send update instructions to users.
# However we cannot allow rollback since we moved namespace dir
diff --git a/app/models/project.rb b/app/models/project.rb
index 021920008ad8c9774e1f4bdd17e49897062258c1..cd30467fae33cc7b6e3096ae851439cc64d72c2f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -656,6 +656,8 @@ class Project < ActiveRecord::Base
# db changes in order to prevent out of sync between db and fs
raise Exception.new('repository cannot be renamed')
end
+
+ Gitlab::UploadsTransfer.new.rename_project(path_was, path, namespace.path)
end
def hook_attrs
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index c327c244f0d51688a2c1e31b8022ddab02d5ab4e..64ea6dd42eb8db8a88d90cd045fdd607c14c93d7 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -27,6 +27,7 @@ module Projects
def transfer(project, new_namespace)
Project.transaction do
old_path = project.path_with_namespace
+ old_namespace = project.namespace
new_path = File.join(new_namespace.try(:path) || '', project.path)
if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present?
@@ -51,6 +52,9 @@ module Projects
# clear project cached events
project.reset_events_cache
+ # Move uploads
+ Gitlab::UploadsTransfer.new.move_project(project.path, old_namespace.path, new_namespace.path)
+
true
end
end
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index f9673abbfe8a5d0f1ea0d4b6a176c2be373a4128..e82115858342f4af270fa2d474753a52ed2f2829 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -26,7 +26,7 @@ class FileUploader < CarrierWave::Uploader::Base
end
def secure_url
- File.join(Gitlab.config.gitlab.url, @project.path_with_namespace, "uploads", @secret, file.filename)
+ File.join("/uploads", @secret, file.filename)
end
def file_storage?
diff --git a/features/steps/admin/projects.rb b/features/steps/admin/projects.rb
index 17233f89f38adb2fc2332c1b11e801a4216eca67..5a1cc9aa15145c6f190efef66e92cd6d617f1112 100644
--- a/features/steps/admin/projects.rb
+++ b/features/steps/admin/projects.rb
@@ -41,6 +41,8 @@ class Spinach::Features::AdminProjects < Spinach::FeatureSteps
end
step 'I transfer project to group \'Web\'' do
+ allow_any_instance_of(Projects::TransferService).
+ to receive(:move_uploads_to_new_namespace).and_return(true)
find(:xpath, "//input[@id='new_namespace_id']").set group.id
click_button 'Transfer'
end
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index ae5f2544691bd7ba415cf2da80cfc6a623351535..32a368c2e2beda201bb63eb4b28251eda26bb390 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -48,6 +48,7 @@ module Gitlab
autoload :TableOfContentsFilter, 'gitlab/markdown/table_of_contents_filter'
autoload :TaskListFilter, 'gitlab/markdown/task_list_filter'
autoload :UserReferenceFilter, 'gitlab/markdown/user_reference_filter'
+ autoload :UploadLinkFilter, 'gitlab/markdown/upload_link_filter'
# Public: Parse the provided text with GitLab-Flavored Markdown
#
@@ -140,6 +141,7 @@ module Gitlab
Gitlab::Markdown::SyntaxHighlightFilter,
Gitlab::Markdown::SanitizationFilter,
+ Gitlab::Markdown::UploadLinkFilter,
Gitlab::Markdown::RelativeLinkFilter,
Gitlab::Markdown::EmojiFilter,
Gitlab::Markdown::TableOfContentsFilter,
diff --git a/lib/gitlab/markdown/upload_link_filter.rb b/lib/gitlab/markdown/upload_link_filter.rb
new file mode 100644
index 0000000000000000000000000000000000000000..fbada73ab865c9a677863164f8a1b814839fdf13
--- /dev/null
+++ b/lib/gitlab/markdown/upload_link_filter.rb
@@ -0,0 +1,47 @@
+require 'gitlab/markdown'
+require 'html/pipeline/filter'
+require 'uri'
+
+module Gitlab
+ module Markdown
+ # HTML filter that "fixes" relative upload links to files.
+ # Context options:
+ # :project (required) - Current project
+ #
+ class UploadLinkFilter < HTML::Pipeline::Filter
+ def call
+ doc.search('a').each do |el|
+ process_link_attr el.attribute('href')
+ end
+
+ doc.search('img').each do |el|
+ process_link_attr el.attribute('src')
+ end
+
+ doc
+ end
+
+ protected
+
+ def process_link_attr(html_attr)
+ return if html_attr.blank?
+
+ uri = html_attr.value
+ if uri.starts_with?("/uploads/")
+ html_attr.value = build_url(uri).to_s
+ end
+ end
+
+ def build_url(uri)
+ File.join(Gitlab.config.gitlab.url, context[:project].path_with_namespace, uri)
+ end
+
+ # Ensure that a :project key exists in context
+ #
+ # Note that while the key might exist, its value could be nil!
+ def validate
+ needs :project
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/uploads_transfer.rb b/lib/gitlab/uploads_transfer.rb
new file mode 100644
index 0000000000000000000000000000000000000000..be8fcc7b2d256db48600707493f343e198ea4370
--- /dev/null
+++ b/lib/gitlab/uploads_transfer.rb
@@ -0,0 +1,35 @@
+module Gitlab
+ class UploadsTransfer
+ def move_project(project_path, namespace_path_was, namespace_path)
+ new_namespace_folder = File.join(root_dir, namespace_path)
+ FileUtils.mkdir_p(new_namespace_folder) unless Dir.exist?(new_namespace_folder)
+ from = File.join(root_dir, namespace_path_was, project_path)
+ to = File.join(root_dir, namespace_path, project_path)
+ move(from, to, "")
+ end
+
+ def rename_project(path_was, path, namespace_path)
+ base_dir = File.join(root_dir, namespace_path)
+ move(path_was, path, base_dir)
+ end
+
+ def rename_namespace(path_was, path)
+ move(path_was, path)
+ end
+
+ private
+
+ def move(path_was, path, base_dir = nil)
+ base_dir = root_dir unless base_dir
+ from = File.join(base_dir, path_was)
+ to = File.join(base_dir, path)
+ FileUtils.mv(from, to)
+ rescue Errno::ENOENT
+ false
+ end
+
+ def root_dir
+ File.join(Rails.root, "public", "uploads")
+ end
+ end
+end
diff --git a/public/uploads/.gitkeep b/public/uploads/.gitkeep
deleted file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000
diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb
index f51abfedae529db22d8ec3705233e27e1d0dc61f..93c4494c6607ef084889c7b8faab31158d4bd433 100644
--- a/spec/controllers/projects/uploads_controller_spec.rb
+++ b/spec/controllers/projects/uploads_controller_spec.rb
@@ -33,7 +33,7 @@ describe Projects::UploadsController do
it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"rails_sample\"'
- expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads"
+ expect(response.body).to match "\"url\":\"/uploads"
expect(response.body).to match '\"is_image\":true'
end
end
@@ -49,7 +49,7 @@ describe Projects::UploadsController do
it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
- expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads"
+ expect(response.body).to match "\"url\":\"/uploads"
expect(response.body).to match '\"is_image\":false'
end
end
diff --git a/spec/lib/gitlab/email/attachment_uploader_spec.rb b/spec/lib/gitlab/email/attachment_uploader_spec.rb
index e8208e15e29ead60bb92b74ea82da81661dc40bf..8fb432367b63ddc603ca6b128f88537c7d6b9c7b 100644
--- a/spec/lib/gitlab/email/attachment_uploader_spec.rb
+++ b/spec/lib/gitlab/email/attachment_uploader_spec.rb
@@ -13,7 +13,6 @@ describe Gitlab::Email::AttachmentUploader do
expect(link).not_to be_nil
expect(link[:is_image]).to be_truthy
expect(link[:alt]).to eq("bricks")
- expect(link[:url]).to include("/#{project.path_with_namespace}")
expect(link[:url]).to include("bricks.png")
end
end
diff --git a/spec/lib/gitlab/markdown/upload_link_filter_spec.rb b/spec/lib/gitlab/markdown/upload_link_filter_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..9ae45a6f5597e035e11e393b5424f34fca221ce9
--- /dev/null
+++ b/spec/lib/gitlab/markdown/upload_link_filter_spec.rb
@@ -0,0 +1,75 @@
+# encoding: UTF-8
+
+require 'spec_helper'
+
+module Gitlab::Markdown
+ describe UploadLinkFilter do
+ def filter(doc, contexts = {})
+ contexts.reverse_merge!({
+ project: project
+ })
+
+ described_class.call(doc, contexts)
+ end
+
+ def image(path)
+ %(<img src="#{path}" />)
+ end
+
+ def link(path)
+ %(<a href="#{path}">#{path}</a>)
+ end
+
+ let(:project) { create(:project) }
+
+ shared_examples :preserve_unchanged do
+ it 'does not modify any relative URL in anchor' do
+ doc = filter(link('README.md'))
+ expect(doc.at_css('a')['href']).to eq 'README.md'
+ end
+
+ it 'does not modify any relative URL in image' do
+ doc = filter(image('files/images/logo-black.png'))
+ expect(doc.at_css('img')['src']).to eq 'files/images/logo-black.png'
+ end
+ end
+
+ it 'does not raise an exception on invalid URIs' do
+ act = link("://foo")
+ expect { filter(act) }.not_to raise_error
+ end
+
+ context 'with a valid repository' do
+ it 'rebuilds relative URL for a link' do
+ doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
+ expect(doc.at_css('a')['href']).
+ to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
+ end
+
+ it 'rebuilds relative URL for an image' do
+ doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
+ expect(doc.at_css('a')['href']).
+ to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
+ end
+
+ it 'does not modify absolute URL' do
+ doc = filter(link('http://example.com'))
+ expect(doc.at_css('a')['href']).to eq 'http://example.com'
+ end
+
+ it 'supports Unicode filenames' do
+ path = '/uploads/한글.png'
+ escaped = Addressable::URI.escape(path)
+
+ # Stub these methods so the file doesn't actually need to be in the repo
+ allow_any_instance_of(described_class).
+ to receive(:file_exists?).and_return(true)
+ allow_any_instance_of(described_class).
+ to receive(:image?).with(path).and_return(true)
+
+ doc = filter(image(escaped))
+ expect(doc.at_css('img')['src']).to match "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/%ED%95%9C%EA%B8%80.png"
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/uploads_transfer_spec.rb b/spec/lib/gitlab/uploads_transfer_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..260364a513e4d2646213999645cd09511ba0b85d
--- /dev/null
+++ b/spec/lib/gitlab/uploads_transfer_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+describe Gitlab::UploadsTransfer do
+ before do
+ @root_dir = File.join(Rails.root, "public", "uploads")
+ @upload_transfer = Gitlab::UploadsTransfer.new
+
+ @project_path_was = "test_project_was"
+ @project_path = "test_project"
+ @namespace_path_was = "test_namespace_was"
+ @namespace_path = "test_namespace"
+ end
+
+ after do
+ FileUtils.rm_rf([
+ File.join(@root_dir, @namespace_path),
+ File.join(@root_dir, @namespace_path_was)
+ ])
+ end
+
+ describe '#move_project' do
+ it "moves project upload to another namespace" do
+ FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path))
+ @upload_transfer.move_project(@project_path, @namespace_path_was, @namespace_path)
+
+ expected_path = File.join(@root_dir, @namespace_path, @project_path)
+ expect(Dir.exist?(expected_path)).to be_truthy
+ end
+ end
+
+ describe '#rename_project' do
+ it "renames project" do
+ FileUtils.mkdir_p(File.join(@root_dir, @namespace_path, @project_path_was))
+ @upload_transfer.rename_project(@project_path_was, @project_path, @namespace_path)
+
+ expected_path = File.join(@root_dir, @namespace_path, @project_path)
+ expect(Dir.exist?(expected_path)).to be_truthy
+ end
+ end
+
+ describe '#rename_namespace' do
+ it "renames namespace" do
+ FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path))
+ @upload_transfer.rename_namespace(@namespace_path_was, @namespace_path)
+
+ expected_path = File.join(@root_dir, @namespace_path, @project_path)
+ expect(Dir.exist?(expected_path)).to be_truthy
+ end
+ end
+end
diff --git a/spec/services/projects/download_service_spec.rb b/spec/services/projects/download_service_spec.rb
index f12e09c58c36fb0f53bc9e8c0fb4c3e047bf8a82..ddee2e62dfcfed6e9289b4ef6be404566406f74a 100644
--- a/spec/services/projects/download_service_spec.rb
+++ b/spec/services/projects/download_service_spec.rb
@@ -37,7 +37,6 @@ describe Projects::DownloadService do
it { expect(@link_to_file).to have_key('url') }
it { expect(@link_to_file).to have_key('is_image') }
it { expect(@link_to_file['is_image']).to be true }
- it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file['url']).to match('rails_sample.jpg') }
it { expect(@link_to_file['alt']).to eq('rails_sample') }
end
@@ -52,7 +51,6 @@ describe Projects::DownloadService do
it { expect(@link_to_file).to have_key('url') }
it { expect(@link_to_file).to have_key('is_image') }
it { expect(@link_to_file['is_image']).to be false }
- it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file['url']).to match('doc_sample.txt') }
it { expect(@link_to_file['alt']).to eq('doc_sample.txt') }
end
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index bb7da33b12e15f8eedc4bde0488268766bfacab1..47755bfc990311bdc420d1c0bf94d1fb75fab2d6 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -7,6 +7,8 @@ describe Projects::TransferService do
context 'namespace -> namespace' do
before do
+ allow_any_instance_of(Gitlab::UploadsTransfer).
+ to receive(:move_project).and_return(true)
group.add_owner(user)
@result = transfer_project(project, user, group)
end
diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/projects/upload_service_spec.rb
index fa4ff6b01ad9993cfcc6fa2d1ed112e2dc08f347..1b1a80d1fe731931de7fabe31826750a5fb6b8d3 100644
--- a/spec/services/projects/upload_service_spec.rb
+++ b/spec/services/projects/upload_service_spec.rb
@@ -18,7 +18,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file).to have_value('banana_sample') }
it { expect(@link_to_file[:is_image]).to equal(true) }
- it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('banana_sample.gif') }
end
@@ -34,7 +33,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_value('dk') }
it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file[:is_image]).to equal(true) }
- it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('dk.png') }
end
@@ -49,7 +47,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file).to have_value('rails_sample') }
it { expect(@link_to_file[:is_image]).to equal(true) }
- it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('rails_sample.jpg') }
end
@@ -64,7 +61,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file).to have_value('doc_sample.txt') }
it { expect(@link_to_file[:is_image]).to equal(false) }
- it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('doc_sample.txt') }
end