diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index ef4717f3f31..6dd62140c7f 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -89,7 +89,10 @@ module FileStore begin parsed_url = URI.parse(UrlHelper.encode(url)) - rescue URI::InvalidURIError, URI::InvalidComponentError + rescue + # There are many exceptions possible here including Addressable::URI:: excpetions + # and URI:: exceptions, catch all may seem wide, but it makes no sense to raise ever + # on an invalid url here return false end diff --git a/spec/multisite/s3_store_spec.rb b/spec/multisite/s3_store_spec.rb index c857e3e9ddf..f36a3a4bed4 100644 --- a/spec/multisite/s3_store_spec.rb +++ b/spec/multisite/s3_store_spec.rb @@ -283,9 +283,10 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do let(:s3_bucket) { resource.bucket(SiteSetting.s3_upload_bucket) } let(:s3_helper) { store.s3_helper } - it "returns false for blank urls" do - url = "" - expect(store.has_been_uploaded?(url)).to eq(false) + it "returns false for blank urls and bad urls" do + expect(store.has_been_uploaded?("")).to eq(false) + expect(store.has_been_uploaded?("http://test@test.com:test/test.git")).to eq(false) + expect(store.has_been_uploaded?("http:///+test@test.com/test.git")).to eq(false) end it "returns true if the base hostname is the same for both urls" do