From b612670cc0a9eabce5d0f8aa86e93823a4639c04 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 20 Jun 2018 14:00:13 +0000 Subject: Fix: Serve favicon image always from the main GitLab domain to avoid issues with CORS --- changelogs/unreleased/fix-favicon-cross-origin.yml | 5 +++++ lib/gitlab/favicon.rb | 20 +++++++++++++++----- spec/lib/gitlab/favicon_spec.rb | 15 +++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/fix-favicon-cross-origin.yml diff --git a/changelogs/unreleased/fix-favicon-cross-origin.yml b/changelogs/unreleased/fix-favicon-cross-origin.yml new file mode 100644 index 00000000000..3317781e222 --- /dev/null +++ b/changelogs/unreleased/fix-favicon-cross-origin.yml @@ -0,0 +1,5 @@ +--- +title: Serve favicon image always from the main GitLab domain to avoid issues with CORS +merge_request: 19810 +author: Alexis Reigel +type: fixed diff --git a/lib/gitlab/favicon.rb b/lib/gitlab/favicon.rb index faf7016d73a..d512fc58e46 100644 --- a/lib/gitlab/favicon.rb +++ b/lib/gitlab/favicon.rb @@ -2,10 +2,10 @@ module Gitlab class Favicon class << self def main - return appearance_favicon.url if appearance_favicon.exists? - 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' elsif Rails.env.development? 'favicon-blue.png' @@ -13,7 +13,7 @@ module Gitlab 'favicon.png' end - ActionController::Base.helpers.image_path(image_name) + ActionController::Base.helpers.image_path(image_name, host: host) end def status_overlay(status_name) @@ -22,7 +22,7 @@ module Gitlab "#{status_name}.png" ) - ActionController::Base.helpers.image_path(path) + ActionController::Base.helpers.image_path(path, host: host) end def available_status_names @@ -35,6 +35,16 @@ module Gitlab 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 RequestStore.store[:appearance] ||= (Appearance.current || Appearance.new) end diff --git a/spec/lib/gitlab/favicon_spec.rb b/spec/lib/gitlab/favicon_spec.rb index f36111a4946..122dcd9634c 100644 --- a/spec/lib/gitlab/favicon_spec.rb +++ b/spec/lib/gitlab/favicon_spec.rb @@ -21,6 +21,21 @@ RSpec.describe Gitlab::Favicon, :request_store do create :appearance, favicon: fixture_file_upload('spec/fixtures/dk.png') expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/dk.png} 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 describe '.status_overlay' do -- cgit v1.2.1