Commit b612670c authored by Alexis Reigel's avatar Alexis Reigel Committed by Douwe Maan

Fix: Serve favicon image always from the main GitLab domain to avoid issues with CORS

parent b349c01c
---
title: Serve favicon image always from the main GitLab domain to avoid issues with CORS
merge_request: 19810
author: Alexis Reigel
type: fixed
...@@ -2,10 +2,10 @@ module Gitlab ...@@ -2,10 +2,10 @@ module Gitlab
class Favicon class Favicon
class << self class << self
def main def main
return appearance_favicon.url if appearance_favicon.exists?
image_name = image_name =
if Gitlab::Utils.to_boolean(ENV['CANARY']) if appearance_favicon.exists?
appearance_favicon.url
elsif Gitlab::Utils.to_boolean(ENV['CANARY'])
'favicon-yellow.png' 'favicon-yellow.png'
elsif Rails.env.development? elsif Rails.env.development?
'favicon-blue.png' 'favicon-blue.png'
...@@ -13,7 +13,7 @@ module Gitlab ...@@ -13,7 +13,7 @@ module Gitlab
'favicon.png' 'favicon.png'
end end
ActionController::Base.helpers.image_path(image_name) ActionController::Base.helpers.image_path(image_name, host: host)
end end
def status_overlay(status_name) def status_overlay(status_name)
...@@ -22,7 +22,7 @@ module Gitlab ...@@ -22,7 +22,7 @@ module Gitlab
"#{status_name}.png" "#{status_name}.png"
) )
ActionController::Base.helpers.image_path(path) ActionController::Base.helpers.image_path(path, host: host)
end end
def available_status_names def available_status_names
...@@ -35,6 +35,16 @@ module Gitlab ...@@ -35,6 +35,16 @@ module Gitlab
private private
# we only want to create full urls when there's a different asset_host
# configured.
def host
if Gitlab::Application.config.asset_host.nil? || Gitlab::Application.config.asset_host == Gitlab.config.gitlab.base_url
nil
else
Gitlab.config.gitlab.base_url
end
end
def appearance def appearance
RequestStore.store[:appearance] ||= (Appearance.current || Appearance.new) RequestStore.store[:appearance] ||= (Appearance.current || Appearance.new)
end end
......
...@@ -21,6 +21,21 @@ RSpec.describe Gitlab::Favicon, :request_store do ...@@ -21,6 +21,21 @@ RSpec.describe Gitlab::Favicon, :request_store do
create :appearance, favicon: fixture_file_upload('spec/fixtures/dk.png') create :appearance, favicon: fixture_file_upload('spec/fixtures/dk.png')
expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/dk.png} expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/dk.png}
end end
context 'asset host' do
before do
allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
end
it 'returns a relative url when the asset host is not configured' do
expect(described_class.main).to match %r{^/assets/favicon-(?:\h+).png$}
end
it 'returns a full url when the asset host is configured' do
allow(Gitlab::Application.config).to receive(:asset_host).and_return('http://assets.local')
expect(described_class.main).to match %r{^http://localhost/assets/favicon-(?:\h+).png$}
end
end
end end
describe '.status_overlay' do describe '.status_overlay' 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