Don't use bang method when there is no safe method56020-reduce-cyclomatic-complexity

https://github.com/rubocop-hq/ruby-style-guide#dangerous-method-bang

2 files changed, 35 insertions, 30 deletions

diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index 23a6bccf0fc..f6b2e2acf16 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -36,7 +36,7 @@ module Gitlab
         # Param url can be a string, URI or Addressable::URI
         uri = parse_url(url)
 
-        validate_uri!(
+        validate_uri(
           uri: uri,
           schemes: schemes,
           ports: ports,
@@ -48,16 +48,16 @@ module Gitlab
         hostname = uri.hostname
         port = get_port(uri)
 
-        addrs_info = get_address_info(hostname, port)
-        return [uri, nil] unless addrs_info
+        address_info = get_address_info(hostname, port)
+        return [uri, nil] unless address_info
 
-        protected_uri_with_hostname = enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
+        protected_uri_with_hostname = enforce_uri_hostname(address_info, uri, hostname, dns_rebind_protection)
 
         # Allow url from the GitLab instance itself but only for the configured hostname and ports
         return protected_uri_with_hostname if internal?(uri)
 
-        validate_local_request!(
-          address_info: addrs_info,
+        validate_local_request(
+          address_info: address_info,
           allow_localhost: allow_localhost,
           allow_local_network: allow_local_network
         )
@@ -94,16 +94,16 @@ module Gitlab
         [uri, hostname]
       end
 
-      def validate_uri!(uri:, schemes:, ports:, enforce_sanitization:, enforce_user:, ascii_only:)
-        validate_html_tags!(uri) if enforce_sanitization
+      def validate_uri(uri:, schemes:, ports:, enforce_sanitization:, enforce_user:, ascii_only:)
+        validate_html_tags(uri) if enforce_sanitization
 
         return if internal?(uri)
 
-        validate_scheme!(uri.scheme, schemes)
-        validate_port!(get_port(uri), ports) if ports.any?
-        validate_user!(uri.user) if enforce_user
-        validate_hostname!(uri.hostname)
-        validate_unicode_restriction!(uri) if ascii_only
+        validate_scheme(uri.scheme, schemes)
+        validate_port(get_port(uri), ports) if ports.any?
+        validate_user(uri.user) if enforce_user
+        validate_hostname(uri.hostname)
+        validate_unicode_restriction(uri) if ascii_only
       end
 
       def get_address_info(hostname, port)
@@ -113,20 +113,25 @@ module Gitlab
       rescue SocketError
       end
 
-      def validate_local_request!(address_info:, allow_localhost:, allow_local_network:)
+      def validate_local_request(address_info:, allow_localhost:, allow_local_network:)
         return if allow_local_network && allow_localhost
 
-        validate_localhost!(address_info) unless allow_localhost
-        validate_loopback!(address_info) unless allow_localhost
-        validate_local_network!(address_info) unless allow_local_network
-        validate_link_local!(address_info) unless allow_local_network
+        unless allow_localhost
+          validate_localhost(address_info)
+          validate_loopback(address_info)
+        end
+
+        unless allow_local_network
+          validate_local_network(address_info)
+          validate_link_local(address_info)
+        end
       end
 
       def get_port(uri)
         uri.port || uri.default_port
       end
 
-      def validate_html_tags!(uri)
+      def validate_html_tags(uri)
         uri_str = uri.to_s
         sanitized_uri = ActionController::Base.helpers.sanitize(uri_str, tags: [])
         if sanitized_uri != uri_str
@@ -146,7 +151,7 @@ module Gitlab
         CGI.unescape(url.to_s) =~ /\n|\r/
       end
 
-      def validate_port!(port, ports)
+      def validate_port(port, ports)
         return if port.blank?
         # Only ports under 1024 are restricted
         return if port >= 1024
@@ -155,20 +160,20 @@ module Gitlab
         raise BlockedUrlError, "Only allowed ports are #{ports.join(', ')}, and any over 1024"
       end
 
-      def validate_scheme!(scheme, schemes)
+      def validate_scheme(scheme, schemes)
         if scheme.blank? || (schemes.any? && !schemes.include?(scheme))
           raise BlockedUrlError, "Only allowed schemes are #{schemes.join(', ')}"
         end
       end
 
-      def validate_user!(value)
+      def validate_user(value)
         return if value.blank?
         return if value =~ /\A\p{Alnum}/
 
         raise BlockedUrlError, "Username needs to start with an alphanumeric character"
       end
 
-      def validate_hostname!(value)
+      def validate_hostname(value)
         return if value.blank?
         return if IPAddress.valid?(value)
         return if value =~ /\A\p{Alnum}/
@@ -176,13 +181,13 @@ module Gitlab
         raise BlockedUrlError, "Hostname or IP address invalid"
       end
 
-      def validate_unicode_restriction!(uri)
+      def validate_unicode_restriction(uri)
         return if uri.to_s.ascii_only?
 
         raise BlockedUrlError, "URI must be ascii only #{uri.to_s.dump}"
       end
 
-      def validate_localhost!(addrs_info)
+      def validate_localhost(addrs_info)
         local_ips = ["::", "0.0.0.0"]
         local_ips.concat(Socket.ip_address_list.map(&:ip_address))
 
@@ -191,19 +196,19 @@ module Gitlab
         raise BlockedUrlError, "Requests to localhost are not allowed"
       end
 
-      def validate_loopback!(addrs_info)
+      def validate_loopback(addrs_info)
         return unless addrs_info.any? { |addr| addr.ipv4_loopback? || addr.ipv6_loopback? }
 
         raise BlockedUrlError, "Requests to loopback addresses are not allowed"
       end
 
-      def validate_local_network!(addrs_info)
+      def validate_local_network(addrs_info)
         return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? || addr.ipv6_unique_local? }
 
         raise BlockedUrlError, "Requests to the local network are not allowed"
       end
 
-      def validate_link_local!(addrs_info)
+      def validate_link_local(addrs_info)
         netmask = IPAddr.new('169.254.0.0/16')
         return unless addrs_info.any? { |addr| addr.ipv6_linklocal? || netmask.include?(addr.ip_address) }
 
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 253366e0789..f8b0cbfb6f6 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -353,7 +353,7 @@ describe Gitlab::UrlBlocker do
     end
   end
 
-  describe '#validate_hostname!' do
+  describe '#validate_hostname' do
     let(:ip_addresses) do
       [
         '2001:db8:1f70::999:de8:7648:6e8',
@@ -378,7 +378,7 @@ describe Gitlab::UrlBlocker do
 
     it 'does not raise error for valid Ip addresses' do
       ip_addresses.each do |ip|
-        expect { described_class.send(:validate_hostname!, ip) }.not_to raise_error
+        expect { described_class.send(:validate_hostname, ip) }.not_to raise_error
       end
     end
   end