FIX: allows onebox to force GET hosts returning wrong headers on HEAD

This commit is contained in:
Joffrey JAFFEUX 2017-08-08 11:44:27 +02:00 committed by GitHub
parent aab0b06cbe
commit 6cd8203686
3 changed files with 36 additions and 2 deletions

View File

@ -18,6 +18,7 @@ class FinalDestination
end end
@opts = opts || {} @opts = opts || {}
@force_get_hosts = @opts[:force_get_hosts] || []
@opts[:max_redirects] ||= 5 @opts[:max_redirects] ||= 5
@opts[:lookup_ip] ||= lambda do |host| @opts[:lookup_ip] ||= lambda do |host|
begin begin
@ -29,6 +30,7 @@ class FinalDestination
@ignored = [Discourse.base_url_no_prefix] + (@opts[:ignore_redirects] || []) @ignored = [Discourse.base_url_no_prefix] + (@opts[:ignore_redirects] || [])
@limit = @opts[:max_redirects] @limit = @opts[:max_redirects]
@status = :ready @status = :ready
@http_verb = @force_get_hosts.any? { |host| hostname_matches?(host) } ? :get : :head
@cookie = nil @cookie = nil
end end
@ -79,7 +81,7 @@ class FinalDestination
return nil unless validate_uri return nil unless validate_uri
headers = request_headers headers = request_headers
response = Excon.head( response = Excon.public_send(@http_verb,
@uri.to_s, @uri.to_s,
read_timeout: FinalDestination.connection_timeout, read_timeout: FinalDestination.connection_timeout,
headers: headers headers: headers

View File

@ -21,6 +21,10 @@ module Oneboxer
@ignore_redirects ||= ['http://www.dropbox.com', 'http://store.steampowered.com', Discourse.base_url] @ignore_redirects ||= ['http://www.dropbox.com', 'http://store.steampowered.com', Discourse.base_url]
end end
def self.force_get_hosts
@force_get_hosts ||= ['http://us.battle.net']
end
def self.preview(url, options = nil) def self.preview(url, options = nil)
options ||= {} options ||= {}
invalidate(url) if options[:invalidate_oneboxes] invalidate(url) if options[:invalidate_oneboxes]
@ -147,7 +151,7 @@ module Oneboxer
def self.onebox_raw(url) def self.onebox_raw(url)
Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do 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 uri = fd.resolve
return blank_onebox if uri.blank? || SiteSetting.onebox_domains_blacklist.include?(uri.hostname) return blank_onebox if uri.blank? || SiteSetting.onebox_domains_blacklist.include?(uri.hostname)
options = { options = {

View File

@ -7,6 +7,8 @@ describe FinalDestination do
{ {
ignore_redirects: ['https://ignore-me.com'], ignore_redirects: ['https://ignore-me.com'],
force_get_hosts: ['https://force.get.com'],
# avoid IP lookups in test # avoid IP lookups in test
lookup_ip: lambda do |host| lookup_ip: lambda do |host|
case host case host
@ -17,6 +19,7 @@ describe FinalDestination do
when 'private-host.com' then '192.168.10.1' when 'private-host.com' then '192.168.10.1'
when 'internal-ipv6.com' then '2001:abc:de:01:3:3d0:6a65:c2bf' when 'internal-ipv6.com' then '2001:abc:de:01:3:3d0:6a65:c2bf'
when 'ignore-me.com' then '53.84.143.152' when 'ignore-me.com' then '53.84.143.152'
when 'force.get.com' then '22.102.29.40'
else else
as_ip = IPAddr.new(host) rescue nil as_ip = IPAddr.new(host) rescue nil
raise "couldn't lookup #{host}" if as_ip.nil? raise "couldn't lookup #{host}" if as_ip.nil?
@ -132,6 +135,31 @@ describe FinalDestination do
end end
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 context "HEAD not supported" do
before do before do
stub_request(:get, 'https://eviltrout.com').to_return( stub_request(:get, 'https://eviltrout.com').to_return(