From 36e477750c015c3681c6ef3da35ad90ba9df9934 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Tue, 23 May 2017 13:31:20 -0400
Subject: [PATCH] FIX: Use same code path for downloading images
---
lib/file_helper.rb | 14 ++------------
lib/final_destination.rb | 7 ++++++-
spec/components/file_helper_spec.rb | 1 +
spec/components/final_destination_spec.rb | 17 +++++++++++++++++
spec/controllers/uploads_controller_spec.rb | 4 ++--
spec/jobs/pull_hotlinked_images_spec.rb | 1 +
6 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/lib/file_helper.rb b/lib/file_helper.rb
index e9e6ab54800..1cdddd1b425 100644
--- a/lib/file_helper.rb
+++ b/lib/file_helper.rb
@@ -1,4 +1,5 @@
require "open-uri"
+require "final_destination"
class FileHelper
@@ -10,7 +11,7 @@ class FileHelper
url = "https:" + url if url.start_with?("//")
raise Discourse::InvalidParameters.new(:url) unless url =~ /^https?:\/\//
- uri = parse_url(url)
+ uri = FinalDestination.new(url).resolve
extension = File.extname(uri.path)
tmp = Tempfile.new([tmp_file_name, extension])
@@ -36,15 +37,4 @@ class FileHelper
@@images_regexp ||= /\.(#{images.to_a.join("|")})$/i
end
- # HACK to support underscores in URLs
- # cf. http://stackoverflow.com/a/18938253/11983
- def self.parse_url(url)
- URI.parse(url)
- rescue URI::InvalidURIError
- host = url.match(".+\:\/\/([^\/]+)")[1]
- uri = URI.parse(url.sub(host, 'valid-host'))
- uri.instance_variable_set("@host", host)
- uri
- end
-
end
diff --git a/lib/final_destination.rb b/lib/final_destination.rb
index bf581c91ae4..4e0f4229d87 100644
--- a/lib/final_destination.rb
+++ b/lib/final_destination.rb
@@ -62,7 +62,7 @@ class FinalDestination
end
def validate_uri
- validate_uri_format && is_public?
+ validate_uri_format && is_dest_valid?
end
def validate_uri_format
@@ -75,6 +75,10 @@ class FinalDestination
(IPAddr.new(@uri.hostname) rescue nil).nil?
end
+ def is_dest_valid?
+ is_public?
+ end
+
def is_public?
return false unless @uri && @uri.host
@@ -116,4 +120,5 @@ class FinalDestination
end
header[1] if header
end
+
end
diff --git a/spec/components/file_helper_spec.rb b/spec/components/file_helper_spec.rb
index 49cdbdb5753..7b10d1a510f 100644
--- a/spec/components/file_helper_spec.rb
+++ b/spec/components/file_helper_spec.rb
@@ -7,6 +7,7 @@ describe FileHelper do
let(:png) { Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") }
before do
+ Excon.stub({ method: :head, hostname: 'eviltrout.com' }, {})
stub_request(:get, url).to_return(body: png)
end
diff --git a/spec/components/final_destination_spec.rb b/spec/components/final_destination_spec.rb
index 4b0b8b9fd2e..73611a5c45a 100644
--- a/spec/components/final_destination_spec.rb
+++ b/spec/components/final_destination_spec.rb
@@ -10,9 +10,12 @@ describe FinalDestination do
when 'eviltrout.com' then '52.84.143.152'
when 'codinghorror.com' then '91.146.108.148'
when 'discourse.org' then '104.25.152.10'
+ when 'some_thing.example.com' then '104.25.152.10'
when 'private-host.com' then '192.168.10.1'
when 'internal-ipv6.com' then '2001:abc:de:01:3:3d0:6a65:c2bf'
else
+ as_ip = IPAddr.new(host) rescue nil
+ raise "couldn't lookup #{host}" if as_ip.nil?
host
end
end
@@ -54,6 +57,20 @@ describe FinalDestination do
expect(final.redirected?).to eq(false)
expect(final.status).to eq(:resolved)
end
+
+ end
+
+ context "underscores in URLs" do
+ before do
+ Excon.stub({ method: :head, hostname: 'some_thing.example.com' }, doc_response)
+ end
+
+ it "doesn't raise errors with underscores in urls" do
+ final = FinalDestination.new('https://some_thing.example.com', opts)
+ expect(final.resolve.to_s).to eq('https://some_thing.example.com')
+ expect(final.redirected?).to eq(false)
+ expect(final.status).to eq(:resolved)
+ end
end
context "with a couple of redirects" do
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index f5fbec08d39..daf983ed46c 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -74,7 +74,7 @@ describe UploadsController do
controller.stubs(:is_api?).returns(true)
Jobs.expects(:enqueue).with(:create_avatar_thumbnails, anything)
-
+ Excon.stub({ method: :head, hostname: 'example.com' }, {})
stub_request(:get, "http://example.com/image.png").to_return(body: File.read('spec/fixtures/images/logo.png'))
xhr :post, :create, url: 'http://example.com/image.png', type: "avatar", synchronous: true
@@ -183,7 +183,7 @@ describe UploadsController do
it "handles file without extension" do
SiteSetting.authorized_extensions = "*"
- upload = Fabricate(:upload, original_filename: "image_file", sha1: sha)
+ Fabricate(:upload, original_filename: "image_file", sha1: sha)
controller.stubs(:render)
controller.expects(:send_file)
diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb
index bdae4fd3d4d..f0814526499 100644
--- a/spec/jobs/pull_hotlinked_images_spec.rb
+++ b/spec/jobs/pull_hotlinked_images_spec.rb
@@ -6,6 +6,7 @@ describe Jobs::PullHotlinkedImages do
before do
png = Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==")
stub_request(:get, "http://wiki.mozilla.org/images/2/2e/Longcat1.png").to_return(body: png)
+ Excon.stub({ method: :head, hostname: 'wiki.mozilla.org' }, {})
SiteSetting.download_remote_images_to_local = true
FastImage.expects(:size).returns([100, 100]).at_least_once
end