forked from cybrespace/mastodon
Fix connect timeout not being enforced (#9329)
* Fix connect timeout not being enforced The loop was catching the timeout exception that should stop execution, so the next IP would no longer be within a timed block, which led to requests taking much longer than 10 seconds. * Use timeout on each IP attempt, but limit to 2 attempts * Fix code style issue * Do not break Request#perform if no block given * Update method stub in spec for Request * Move timeout inside the begin/rescue block * Use Resolv::DNS with timeout of 1 to get IP addresses * Update Request spec to stub Resolv::DNS instead of Addrinfo * Fix Resolve::DNS stubs in Request spec
This commit is contained in:
parent
824497fbce
commit
fd8145d232
|
@ -2,6 +2,7 @@
|
|||
|
||||
require 'ipaddr'
|
||||
require 'socket'
|
||||
require 'resolv'
|
||||
|
||||
class Request
|
||||
REQUEST_TARGET = '(request-target)'
|
||||
|
@ -45,7 +46,7 @@ class Request
|
|||
end
|
||||
|
||||
begin
|
||||
yield response.extend(ClientLimit)
|
||||
yield response.extend(ClientLimit) if block_given?
|
||||
ensure
|
||||
http_client.close
|
||||
end
|
||||
|
@ -94,7 +95,7 @@ class Request
|
|||
end
|
||||
|
||||
def timeout
|
||||
{ connect: 10, read: 10, write: 10 }
|
||||
{ connect: nil, read: 10, write: 10 }
|
||||
end
|
||||
|
||||
def http_client
|
||||
|
@ -139,16 +140,29 @@ class Request
|
|||
class Socket < TCPSocket
|
||||
class << self
|
||||
def open(host, *args)
|
||||
return super host, *args if thru_hidden_service? host
|
||||
return super(host, *args) if thru_hidden_service?(host)
|
||||
|
||||
outer_e = nil
|
||||
Addrinfo.foreach(host, nil, nil, :SOCK_STREAM) do |address|
|
||||
|
||||
Resolv::DNS.open do |dns|
|
||||
dns.timeouts = 1
|
||||
|
||||
addresses = dns.getaddresses(host).take(2)
|
||||
time_slot = 10.0 / addresses.size
|
||||
|
||||
addresses.each do |address|
|
||||
begin
|
||||
raise Mastodon::HostValidationError if PrivateAddressCheck.private_address? IPAddr.new(address.ip_address)
|
||||
return super address.ip_address, *args
|
||||
raise Mastodon::HostValidationError if PrivateAddressCheck.private_address?(IPAddr.new(address.to_s))
|
||||
|
||||
::Timeout.timeout(time_slot, HTTP::TimeoutError) do
|
||||
return super(address.to_s, *args)
|
||||
end
|
||||
rescue => e
|
||||
outer_e = e
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
raise outer_e if outer_e
|
||||
end
|
||||
|
||||
|
|
|
@ -48,9 +48,11 @@ describe Request do
|
|||
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))
|
||||
resolver = double
|
||||
|
||||
allow(resolver).to receive(:getaddresses).with('example.com').and_return(%w(0.0.0.0 2001:4860:4860::8844))
|
||||
allow(resolver).to receive(:timeouts=).and_return(nil)
|
||||
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
|
||||
|
||||
expect { |block| subject.perform &block }.to yield_control
|
||||
expect(a_request(:get, 'http://example.com')).to have_been_made.once
|
||||
|
@ -81,9 +83,12 @@ describe Request do
|
|||
end
|
||||
|
||||
it 'raises Mastodon::ValidationError' 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:db8::face"], :PF_INET6, :SOCK_STREAM))
|
||||
resolver = double
|
||||
|
||||
allow(resolver).to receive(:getaddresses).with('example.com').and_return(%w(0.0.0.0 2001:db8::face))
|
||||
allow(resolver).to receive(:timeouts=).and_return(nil)
|
||||
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
|
||||
|
||||
expect { subject.perform }.to raise_error Mastodon::ValidationError
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue