Commit dea0b7b4 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch 'revert-getaddrinfo-timeout' into 'master'

Revert "Add 15s timeout to GitLab UrlBlocker"

See merge request gitlab-org/gitlab!54425
parents bb13a881 5f505a62
...@@ -7,9 +7,6 @@ module Gitlab ...@@ -7,9 +7,6 @@ module Gitlab
class UrlBlocker class UrlBlocker
BlockedUrlError = Class.new(StandardError) BlockedUrlError = Class.new(StandardError)
GETADDRINFO_TIMEOUT_SECONDS = 15
private_constant :GETADDRINFO_TIMEOUT_SECONDS
class << self class << self
# Validates the given url according to the constraints specified by arguments. # Validates the given url according to the constraints specified by arguments.
# #
...@@ -113,7 +110,7 @@ module Gitlab ...@@ -113,7 +110,7 @@ module Gitlab
end end
def get_address_info(uri, dns_rebind_protection) def get_address_info(uri, dns_rebind_protection)
Addrinfo.getaddrinfo(uri.hostname, get_port(uri), nil, :STREAM, timeout: GETADDRINFO_TIMEOUT_SECONDS).map do |addr| Addrinfo.getaddrinfo(uri.hostname, get_port(uri), nil, :STREAM).map do |addr|
addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
end end
rescue SocketError rescue SocketError
......
...@@ -160,34 +160,6 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do ...@@ -160,34 +160,6 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
end end
end end
end end
context 'when resolving runs into a timeout' do
let(:import_url) { 'http://example.com' }
subject { described_class.validate!(import_url, dns_rebind_protection: dns_rebind_protection) }
before do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
allow(Addrinfo).to receive(:getaddrinfo).and_raise(SocketError)
end
context 'with dns rebinding enabled' do
let(:dns_rebind_protection) { true }
it 'raises an error due to DNS timeout' do
expect { subject }.to raise_error(described_class::BlockedUrlError)
end
end
context 'with dns rebinding disabled' do
let(:dns_rebind_protection) { false }
it_behaves_like 'validates URI and hostname' do
let(:expected_uri) { import_url }
let(:expected_hostname) { nil }
end
end
end
end end
describe '#blocked_url?' do describe '#blocked_url?' do
......
...@@ -12,38 +12,19 @@ module DnsHelpers ...@@ -12,38 +12,19 @@ module DnsHelpers
end end
def stub_all_dns! def stub_all_dns!
allow(Addrinfo).to receive(:getaddrinfo).and_return([]) allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM).and_return([])
allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM, anything, anything).and_return([])
end end
def stub_invalid_dns! def stub_invalid_dns!
invalid_addresses = %r{ allow(Addrinfo).to receive(:getaddrinfo).with(/\Afoobar\.\w|(\d{1,3}\.){4,}\d{1,3}\z/i, anything, nil, :STREAM) do
\A raise SocketError.new("getaddrinfo: Name or service not known")
(?: end
foobar\.\w |
(?:\d{1,3}\.){4,}\d{1,3}
)
\z
}ix
allow(Addrinfo).to receive(:getaddrinfo)
.with(invalid_addresses, any_args)
.and_raise(SocketError, 'getaddrinfo: Name or service not known')
end end
def permit_local_dns! def permit_local_dns!
local_addresses = %r{ local_addresses = /\A(127|10)\.0\.0\.\d{1,3}|(192\.168|172\.16)\.\d{1,3}\.\d{1,3}|0\.0\.0\.0|localhost\z/i
\A allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM).and_call_original
(?: allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM, anything, anything, any_args).and_call_original
(?:127|10)\.0\.0\.\d{1,3} |
(?:192\.168|172\.16)\.\d{1,3}\.\d{1,3} |
0\.0\.0\.0 |
localhost
)
\z
}ix
allow(Addrinfo).to receive(:getaddrinfo)
.with(local_addresses, any_args)
.and_call_original
end end
end end
...@@ -18,12 +18,13 @@ module StubRequests ...@@ -18,12 +18,13 @@ module StubRequests
end end
def stub_dns(url, ip_address:, port: 80) def stub_dns(url, ip_address:, port: 80)
url = URI(url) url = parse_url(url)
socket = Socket.sockaddr_in(port, ip_address) socket = Socket.sockaddr_in(port, ip_address)
addr = Addrinfo.new(socket) addr = Addrinfo.new(socket)
# See Gitlab::UrlBlocker
allow(Addrinfo).to receive(:getaddrinfo) allow(Addrinfo).to receive(:getaddrinfo)
.with(url.hostname, url.port, any_args) .with(url.hostname, url.port, nil, :STREAM)
.and_return([addr]) .and_return([addr])
end end
...@@ -33,14 +34,22 @@ module StubRequests ...@@ -33,14 +34,22 @@ module StubRequests
socket = Socket.sockaddr_in(port, ip_address) socket = Socket.sockaddr_in(port, ip_address)
addr = Addrinfo.new(socket) addr = Addrinfo.new(socket)
# See Gitlab::UrlBlocker
allow(Addrinfo).to receive(:getaddrinfo).and_call_original
allow(Addrinfo).to receive(:getaddrinfo) allow(Addrinfo).to receive(:getaddrinfo)
.with(url.hostname, any_args) .with(url.hostname, anything, nil, :STREAM)
.and_return([addr]) .and_return([addr])
end end
def stubbed_hostname(url, hostname: IP_ADDRESS_STUB) def stubbed_hostname(url, hostname: IP_ADDRESS_STUB)
url = URI(url) url = parse_url(url)
url.hostname = hostname url.hostname = hostname
url.to_s url.to_s
end end
private
def parse_url(url)
url.is_a?(URI) ? url : URI(url)
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment