diff --git a/app/models/upload.rb b/app/models/upload.rb index cb3b69e9e41..867d4de3304 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -81,8 +81,16 @@ class Upload < ActiveRecord::Base url = url.sub(Discourse.asset_host, "") if Discourse.asset_host.present? && Discourse.asset_host != SiteSetting.Upload.s3_cdn_url # when using s3 without CDN url = url.sub(/^https?\:/, "") if url.include?(Discourse.store.absolute_base_url) && Discourse.store.external? + # when using s3, we need to replace with the absolute base url - url = url.sub(SiteSetting.Upload.s3_cdn_url, Discourse.store.absolute_base_url) if SiteSetting.Upload.s3_cdn_url.present? + if SiteSetting.Upload.s3_cdn_url.present? + path = Discourse.store.s3_bucket.split("/", 2)[1] + + url = url.sub( + SiteSetting.Upload.s3_cdn_url, + "#{Discourse.store.absolute_base_url}#{path ? '/' + path : ''}" + ) + end # always try to get the path uri = begin diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 697cd266ee2..5145c7ae70b 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -93,7 +93,7 @@ describe Upload do describe "s3 store" do let(:path) { "/original/3X/1/0/10f73034616a796dfd70177dc54b6def44c4ba6f.png" } - let(:url) { "//#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com#{path}" } + let(:url) { "#{SiteSetting.Upload.absolute_base_url}#{path}" } before do SiteSetting.enable_s3_uploads = true @@ -102,23 +102,35 @@ describe Upload do SiteSetting.s3_secret_access_key = "some secret key" end - after do - SiteSetting.enable_s3_uploads = false - end - it "should return the right upload when using base url (not CDN) for s3" do upload - url = "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com#{path}" - expect(Upload.get_from_url(url)).to eq(upload) end - it "should return the right upload when using a CDN for s3" do - upload - s3_cdn_url = 'https://mycdn.slowly.net' - SiteSetting.s3_cdn_url = s3_cdn_url + describe 'when using a cdn' do + let(:s3_cdn_url) { 'https://mycdn.slowly.net' } - expect(Upload.get_from_url(URI.join(s3_cdn_url, path).to_s)).to eq(upload) + before do + SiteSetting.s3_cdn_url = s3_cdn_url + end + + it "should return the right upload" do + upload + expect(Upload.get_from_url(URI.join(s3_cdn_url, path).to_s)).to eq(upload) + end + + describe 'when upload bucket contains subfolder' do + let(:url) { "#{SiteSetting.Upload.absolute_base_url}/path/path2#{path}" } + + before do + SiteSetting.s3_upload_bucket = "s3-upload-bucket/path/path2" + end + + it "should return the right upload" do + upload + expect(Upload.get_from_url(URI.join(s3_cdn_url, path).to_s)).to eq(upload) + end + end end it "should return the right upload when using one CDN for both s3 and assets" do