From 36b57037961383466b7f5c20b39ee68cd9f202a0 Mon Sep 17 00:00:00 2001 From: Rey Tucker Date: Tue, 20 Mar 2018 04:06:08 -0400 Subject: [PATCH] request: in the event of failure, try other IPs (#6761) (#6813) * request: in the event of failure, try other IPs (#6761) In the case where a name has multiple A/AAAA records, we should try subsequent records instead of immediately failing when we have a failure on the first IP address. This significantly improves delivery success when there are network connectivity problems affecting only IPv4 or IPv6. * fix method call style * request_spec: adjust test case to use Addrinfo * request: Request/open: move private addr check to within begin/rescue * request_spec: add case to test failover, fix exception check * Double Addrinfo.foreach so that it correctly yields instances --- app/lib/request.rb | 13 ++++++++++--- spec/lib/request_spec.rb | 11 ++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/lib/request.rb b/app/lib/request.rb index 5776b3d78..298fb9528 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -94,9 +94,16 @@ class Request class Socket < TCPSocket class << self def open(host, *args) - address = IPSocket.getaddress(host) - raise Mastodon::HostValidationError if PrivateAddressCheck.private_address? IPAddr.new(address) - super address, *args + outer_e = nil + Addrinfo.foreach(host, nil, nil, :SOCK_STREAM) do |address| + begin + raise Mastodon::HostValidationError if PrivateAddressCheck.private_address? IPAddr.new(address.ip_address) + return super address.ip_address, *args + rescue => e + outer_e = e + end + end + raise outer_e if outer_e end alias new open diff --git a/spec/lib/request_spec.rb b/spec/lib/request_spec.rb index dc7daa52c..5da357c55 100644 --- a/spec/lib/request_spec.rb +++ b/spec/lib/request_spec.rb @@ -48,6 +48,13 @@ describe Request do expect(a_request(:get, 'http://example.com')).to have_been_made.once end + it 'executes a HTTP request when the first address is private' do + allow(Addrinfo).to receive(:foreach).with('example.com', nil, nil, :SOCK_STREAM) + .and_yield(Addrinfo.new(["AF_INET", 0, "example.com", "0.0.0.0"], :PF_INET, :SOCK_STREAM)) + .and_yield(Addrinfo.new(["AF_INET6", 0, "example.com", "2001:4860:4860::8844"], :PF_INET6, :SOCK_STREAM)) + expect(a_request(:get, 'http://example.com')).to have_been_made.once + end + it 'sets headers' do expect(a_request(:get, 'http://example.com').with(headers: subject.headers)).to have_been_made end @@ -61,7 +68,9 @@ describe Request do end it 'raises Mastodon::ValidationError' do - allow(IPSocket).to receive(:getaddress).with('example.com').and_return('0.0.0.0') + allow(Addrinfo).to receive(:foreach).with('example.com', nil, nil, :SOCK_STREAM) + .and_yield(Addrinfo.new(["AF_INET", 0, "example.com", "0.0.0.0"], :PF_INET, :SOCK_STREAM)) + .and_yield(Addrinfo.new(["AF_INET6", 0, "example.com", "2001:db8::face"], :PF_INET6, :SOCK_STREAM)) expect{ subject.perform }.to raise_error Mastodon::ValidationError end end