Add 15s timeout to GitLab UrlBlocker

The URL blocker checks for resolvable addresses of the URLs it receives.

In some cases, bad/stale URLs do not resolve in a finite time, and lead
to a hang of the call for over the rack/puma timeout levels.

The `Addrinfo.getaddrinfo` call supports passing a timeout since
Ruby 2.7.0, and this change applies a 15 seconds
timeout to the operation.

Fix DNS mock helpers

The existing DNS mock helpers do not support receiving just the timeout
keyword option passed to the getaddrinfo call, leading to the fixture
failures in the pipeline tests.

Fix misunderstanding of DNS helpers

It looks like the merge request
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40490 had already
added coverage for sending keyword arguments to the getaddrinfo call,
but only in one stub situation

This change attempts to add the same type of mock to the others

Still unsure if this is entirely right for the tests, but will iterate
over if it fails again

Add a test forcing DNS timeout

Attempts adding a test to lower the timeout to unreasonable levels
and expect a DNS lookup failure to occur.

This test may not work as expected on platforms other than Linux due
to its reliance on the `getaddrinfo_a` call existing.

Also fixed are the DNS helpers (yet again) to only look for the
timeout params

Stub new parameter `timeout` also in StubRequests spec helper

Fix failing URL blocker timeout specs

Make DNS stubbing less brittle to changes

Skip timeout specs if it's not available

For example, on OSX getaddrinfo_a is not available.

lib/gitlab/url_blocker.rb

@@ -7,6 +7,9 @@ module Gitlab
   class UrlBlocker
     BlockedUrlError = Class.new(StandardError)
 
+    GETADDRINFO_TIMEOUT_SECONDS = 15
+    private_constant :GETADDRINFO_TIMEOUT_SECONDS
+
     class << self
       # Validates the given url according to the constraints specified by arguments.
       #
@@ -110,7 +113,7 @@ module Gitlab
       end
 
       def get_address_info(uri, dns_rebind_protection)
-        Addrinfo.getaddrinfo(uri.hostname, get_port(uri), nil, :STREAM).map do |addr|
+        Addrinfo.getaddrinfo(uri.hostname, get_port(uri), nil, :STREAM, timeout: GETADDRINFO_TIMEOUT_SECONDS).map do |addr|
           addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
         end
       rescue SocketError

spec/lib/gitlab/url_blocker_spec.rb

@@ -160,6 +160,47 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
         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
+        skip 'timeout is not available' unless timeout_available?
+
+        stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+        stub_const("#{described_class}::GETADDRINFO_TIMEOUT_SECONDS", 0)
+      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
+
+      # Detect whether the timeout option is available.
+      #
+      # See https://bugs.ruby-lang.org/issues/15553
+      def timeout_available?
+        Addrinfo.getaddrinfo('localhost', nil, timeout: 0)
+
+        false
+      rescue SocketError
+        true
+      end
+    end
   end
 
   describe '#blocked_url?' do

spec/support/helpers/dns_helpers.rb

@@ -12,19 +12,38 @@ module DnsHelpers
   end
 
   def stub_all_dns!
-    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([])
+    allow(Addrinfo).to receive(:getaddrinfo).and_return([])
   end
 
   def stub_invalid_dns!
-    allow(Addrinfo).to receive(:getaddrinfo).with(/\Afoobar\.\w|(\d{1,3}\.){4,}\d{1,3}\z/i, anything, nil, :STREAM) do
-      raise SocketError.new("getaddrinfo: Name or service not known")
-    end
+    invalid_addresses = %r{
+      \A
+        (?:
+          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
 
   def permit_local_dns!
-    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
-    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
+    local_addresses = %r{
+      \A
+        (?:
+          (?: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

spec/support/helpers/stub_requests.rb

@@ -18,14 +18,13 @@ module StubRequests
   end
 
   def stub_dns(url, ip_address:, port: 80)
-    url = parse_url(url)
+    url = URI(url)
     socket = Socket.sockaddr_in(port, ip_address)
     addr = Addrinfo.new(socket)
 
-    # See Gitlab::UrlBlocker
     allow(Addrinfo).to receive(:getaddrinfo)
-                         .with(url.hostname, url.port, nil, :STREAM)
-                         .and_return([addr])
+      .with(url.hostname, url.port, any_args)
+      .and_return([addr])
   end
 
   def stub_all_dns(url, ip_address:)
@@ -34,22 +33,14 @@ module StubRequests
     socket = Socket.sockaddr_in(port, ip_address)
     addr = Addrinfo.new(socket)
 
-    # See Gitlab::UrlBlocker
-    allow(Addrinfo).to receive(:getaddrinfo).and_call_original
     allow(Addrinfo).to receive(:getaddrinfo)
-      .with(url.hostname, anything, nil, :STREAM)
+      .with(url.hostname, any_args)
       .and_return([addr])
   end
 
   def stubbed_hostname(url, hostname: IP_ADDRESS_STUB)
-    url = parse_url(url)
+    url = URI(url)
     url.hostname = hostname
     url.to_s
   end
-
-  private
-
-  def parse_url(url)
-    url.is_a?(URI) ? url : URI(url)
-  end
 end