From 6cd8203686ac7c2584523394944fdbe905a5a21a Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 8 Aug 2017 11:44:27 +0200 Subject: [PATCH] FIX: allows onebox to force GET hosts returning wrong headers on HEAD --- lib/final_destination.rb | 4 +++- lib/oneboxer.rb | 6 ++++- spec/components/final_destination_spec.rb | 28 +++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/lib/final_destination.rb b/lib/final_destination.rb index ad02e6fbdf8..2e55f662e29 100644 --- a/lib/final_destination.rb +++ b/lib/final_destination.rb @@ -18,6 +18,7 @@ class FinalDestination end @opts = opts || {} + @force_get_hosts = @opts[:force_get_hosts] || [] @opts[:max_redirects] ||= 5 @opts[:lookup_ip] ||= lambda do |host| begin @@ -29,6 +30,7 @@ class FinalDestination @ignored = [Discourse.base_url_no_prefix] + (@opts[:ignore_redirects] || []) @limit = @opts[:max_redirects] @status = :ready + @http_verb = @force_get_hosts.any? { |host| hostname_matches?(host) } ? :get : :head @cookie = nil end @@ -79,7 +81,7 @@ class FinalDestination return nil unless validate_uri headers = request_headers - response = Excon.head( + response = Excon.public_send(@http_verb, @uri.to_s, read_timeout: FinalDestination.connection_timeout, headers: headers diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb index 42862abe325..54d239c91ee 100644 --- a/lib/oneboxer.rb +++ b/lib/oneboxer.rb @@ -21,6 +21,10 @@ module Oneboxer @ignore_redirects ||= ['http://www.dropbox.com', 'http://store.steampowered.com', Discourse.base_url] end + def self.force_get_hosts + @force_get_hosts ||= ['http://us.battle.net'] + end + def self.preview(url, options = nil) options ||= {} invalidate(url) if options[:invalidate_oneboxes] @@ -147,7 +151,7 @@ module Oneboxer def self.onebox_raw(url) Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do - fd = FinalDestination.new(url, ignore_redirects: ignore_redirects) + fd = FinalDestination.new(url, ignore_redirects: ignore_redirects, force_get_hosts: force_get_hosts) uri = fd.resolve return blank_onebox if uri.blank? || SiteSetting.onebox_domains_blacklist.include?(uri.hostname) options = { diff --git a/spec/components/final_destination_spec.rb b/spec/components/final_destination_spec.rb index baa544fdc03..8c58a6a2a6a 100644 --- a/spec/components/final_destination_spec.rb +++ b/spec/components/final_destination_spec.rb @@ -7,6 +7,8 @@ describe FinalDestination do { ignore_redirects: ['https://ignore-me.com'], + force_get_hosts: ['https://force.get.com'], + # avoid IP lookups in test lookup_ip: lambda do |host| case host @@ -17,6 +19,7 @@ describe FinalDestination do when 'private-host.com' then '192.168.10.1' when 'internal-ipv6.com' then '2001:abc:de:01:3:3d0:6a65:c2bf' when 'ignore-me.com' then '53.84.143.152' + when 'force.get.com' then '22.102.29.40' else as_ip = IPAddr.new(host) rescue nil raise "couldn't lookup #{host}" if as_ip.nil? @@ -132,6 +135,31 @@ describe FinalDestination do end end + context "GET can be forced" do + before do + stub_request(:head, 'https://force.get.com/posts?page=4') + stub_request(:get, 'https://force.get.com/posts?page=4') + stub_request(:head, 'https://eviltrout.com/posts?page=2') + stub_request(:get, 'https://eviltrout.com/posts?page=2') + end + + it "will do a GET when forced" do + final = FinalDestination.new('https://force.get.com/posts?page=4', opts) + expect(final.resolve.to_s).to eq('https://force.get.com/posts?page=4') + expect(final.status).to eq(:resolved) + expect(WebMock).to have_requested(:get, 'https://force.get.com/posts?page=4') + expect(WebMock).to_not have_requested(:head, 'https://force.get.com/posts?page=4') + end + + it "will do a HEAD if not forced" do + final = FinalDestination.new('https://eviltrout.com/posts?page=2', opts) + expect(final.resolve.to_s).to eq('https://eviltrout.com/posts?page=2') + expect(final.status).to eq(:resolved) + expect(WebMock).to_not have_requested(:get, 'https://eviltrout.com/posts?page=2') + expect(WebMock).to have_requested(:head, 'https://eviltrout.com/posts?page=2') + end + end + context "HEAD not supported" do before do stub_request(:get, 'https://eviltrout.com').to_return(