Align UrlValidator to validate_url gem implementation.

Renamed UrlValidator to AddressableUrlValidator to avoid 'url:' naming collision with ActiveModel::Validations::UrlValidator in 'validates' statement.
Make use of the options attribute of the parent class ActiveModel::EachValidator.
Add more options: allow_nil, allow_blank, message.
Renamed 'protocols' option to 'schemes' to match the option naming from UrlValidator.

12 files changed, 135 insertions, 124 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index d28a12413bf..21a97c8d773 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -48,17 +48,17 @@ class ApplicationSetting < ApplicationRecord
 
   validates :home_page_url,
             allow_blank: true,
-            url: true,
+            addressable_url: true,
             if: :home_page_url_column_exists?
 
   validates :help_page_support_url,
             allow_blank: true,
-            url: true,
+            addressable_url: true,
             if: :help_page_support_url_column_exists?
 
   validates :after_sign_out_path,
             allow_blank: true,
-            url: true
+            addressable_url: true
 
   validates :admin_notification_email,
             devise_email: true,
@@ -218,7 +218,7 @@ class ApplicationSetting < ApplicationRecord
             if: :external_authorization_service_enabled
 
   validates :external_authorization_service_url,
-            url: true, allow_blank: true,
+            addressable_url: true, allow_blank: true,
             if: :external_authorization_service_enabled
 
   validates :external_authorization_service_timeout,
diff --git a/app/models/badge.rb b/app/models/badge.rb
index a244ed473de..50299cd6652 100644
--- a/app/models/badge.rb
+++ b/app/models/badge.rb
@@ -22,7 +22,7 @@ class Badge < ApplicationRecord
 
   scope :order_created_at_asc, -> { reorder(created_at: :asc) }
 
-  validates :link_url, :image_url, url: { protocols: %w(http https) }
+  validates :link_url, :image_url, addressable_url: true
   validates :type, presence: true
 
   def rendered_link_url(project = nil)
diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb
index 80dbb150085..997bf298025 100644
--- a/app/models/ci/build_runner_session.rb
+++ b/app/models/ci/build_runner_session.rb
@@ -13,7 +13,7 @@ module Ci
     belongs_to :build, class_name: 'Ci::Build', inverse_of: :runner_session
 
     validates :build, presence: true
-    validates :url, url: { protocols: %w(https) }
+    validates :url, addressable_url: { schemes: %w(https) }
 
     def terminal_specification
       wss_url = Gitlab::UrlHelpers.as_wss(self.url)
diff --git a/app/models/environment.rb b/app/models/environment.rb
index fa29a83e517..69224635e34 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -35,7 +35,7 @@ class Environment < ApplicationRecord
   validates :external_url,
             length: { maximum: 255 },
             allow_nil: true,
-            url: true
+            addressable_url: true
 
   delegate :stop_action, :manual_actions, to: :last_deployment, allow_nil: true
 
diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb
index 70954bf8b05..72270ee8b4f 100644
--- a/app/models/error_tracking/project_error_tracking_setting.rb
+++ b/app/models/error_tracking/project_error_tracking_setting.rb
@@ -22,7 +22,7 @@ module ErrorTracking
 
     belongs_to :project
 
-    validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true
+    validates :api_url, length: { maximum: 255 }, public_url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true
 
     validates :api_url, presence: { message: 'is a required field' }, if: :enabled
 
diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb
index 3028bf21301..8a768b3a2c0 100644
--- a/app/models/generic_commit_status.rb
+++ b/app/models/generic_commit_status.rb
@@ -3,7 +3,7 @@
 class GenericCommitStatus < CommitStatus
   before_validation :set_default_values
 
-  validates :target_url, url: true,
+  validates :target_url, addressable_url: true,
                          length: { maximum: 255 },
                          allow_nil: true
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 66fc83113ea..89ad90a964e 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -327,7 +327,7 @@ class Project < ApplicationRecord
 
   validates :namespace, presence: true
   validates :name, uniqueness: { scope: :namespace_id }
-  validates :import_url, public_url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
+  validates :import_url, public_url: { schemes: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
                                        ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
                                        enforce_user: true }, if: [:external_import?, :import_url_changed?]
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
diff --git a/app/models/releases/link.rb b/app/models/releases/link.rb
index 36ec33d3e3e..58c2b98e524 100644
--- a/app/models/releases/link.rb
+++ b/app/models/releases/link.rb
@@ -6,7 +6,7 @@ module Releases
 
     belongs_to :release
 
-    validates :url, presence: true, url: { protocols: %w(http https ftp) }, uniqueness: { scope: :release }
+    validates :url, presence: true, addressable_url: { schemes: %w(http https ftp) }, uniqueness: { scope: :release }
     validates :name, presence: true, uniqueness: { scope: :release }
 
     scope :sorted, -> { order(created_at: :desc) }
diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index 5610cfe0f24..b2fd5394a03 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -17,7 +17,7 @@ class RemoteMirror < ApplicationRecord
 
   belongs_to :project, inverse_of: :remote_mirrors
 
-  validates :url, presence: true, public_url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }
+  validates :url, presence: true, public_url: { schemes: %w(ssh git http https), allow_blank: true, enforce_user: true }
 
   before_save :set_new_remote_name, if: :mirror_url_changed?
 
diff --git a/app/validators/addressable_url_validator.rb b/app/validators/addressable_url_validator.rb
new file mode 100644
index 00000000000..273e15ef925
--- /dev/null
+++ b/app/validators/addressable_url_validator.rb
@@ -0,0 +1,112 @@
+# frozen_string_literal: true
+
+# AddressableUrlValidator
+#
+# Custom validator for URLs. This is a stricter version of UrlValidator - it also checks
+# for using the right protocol, but it actually parses the URL checking for any syntax errors.
+# The regex is also different from `URI` as we use `Addressable::URI` here.
+#
+# By default, only URLs for the HTTP(S) schemes will be considered valid.
+# Provide a `:schemes` option to configure accepted schemes.
+#
+# Example:
+#
+#   class User < ActiveRecord::Base
+#     validates :personal_url, addressable_url: true
+#
+#     validates :ftp_url, addressable_url: { schemes: %w(ftp) }
+#
+#     validates :git_url, addressable_url: { schemes: %w(http https ssh git) }
+#   end
+#
+# This validator can also block urls pointing to localhost or the local network to
+# protect against Server-side Request Forgery (SSRF), or check for the right port.
+#
+# Configuration options:
+# * <tt>message</tt> - A custom error message (default is: "must be a valid URL").
+# * <tt>schemes</tt> - Array of URI schemes. Default: +['http', 'https']+
+# * <tt>allow_localhost</tt> - Allow urls pointing to +localhost+. Default: +true+
+# * <tt>allow_local_network</tt> - Allow urls pointing to private network addresses. Default: +true+
+# * <tt>allow_blank</tt> - Allow urls to be +blank+. Default: +false+
+# * <tt>allow_nil</tt> - Allow urls to be +nil+. Default: +false+
+# * <tt>ports</tt> - Allowed ports. Default: +all+.
+# * <tt>enforce_user</tt> - Validate user format. Default: +false+
+# * <tt>enforce_sanitization</tt> - Validate that there are no html/css/js tags. Default: +false+
+#
+# Example:
+#   class User < ActiveRecord::Base
+#     validates :personal_url, addressable_url: { allow_localhost: false, allow_local_network: false}
+#
+#     validates :web_url, addressable_url: { ports: [80, 443] }
+#   end
+class AddressableUrlValidator < ActiveModel::EachValidator
+  attr_reader :record
+
+  BLOCKER_VALIDATE_OPTIONS = {
+    schemes: %w(http https),
+    ports: [],
+    allow_localhost: true,
+    allow_local_network: true,
+    ascii_only: false,
+    enforce_user: false,
+    enforce_sanitization: false
+  }.freeze
+
+  DEFAULT_OPTIONS = BLOCKER_VALIDATE_OPTIONS.merge({
+    message: 'must be a valid URL'
+  }).freeze
+
+  def initialize(options)
+    options.reverse_merge!(DEFAULT_OPTIONS)
+
+    super(options)
+  end
+
+  def validate_each(record, attribute, value)
+    @record = record
+
+    unless value.present?
+      record.errors.add(attribute, options.fetch(:message))
+      return
+    end
+
+    value = strip_value!(record, attribute, value)
+
+    Gitlab::UrlBlocker.validate!(value, blocker_args)
+  rescue Gitlab::UrlBlocker::BlockedUrlError => e
+    record.errors.add(attribute, "is blocked: #{e.message}")
+  end
+
+  private
+
+  def strip_value!(record, attribute, value)
+    new_value = value.strip
+    return value if new_value == value
+
+    record.public_send("#{attribute}=", new_value) # rubocop:disable GitlabSecurity/PublicSend
+  end
+
+  def current_options
+    options.map do |option, value|
+      [option, value.is_a?(Proc) ? value.call(record) : value]
+    end.to_h
+  end
+
+  def blocker_args
+    current_options.slice(*BLOCKER_VALIDATE_OPTIONS.keys).tap do |args|
+      if self.class.allow_setting_local_requests?
+        args[:allow_localhost] = args[:allow_local_network] = true
+      end
+    end
+  end
+
+  def self.allow_setting_local_requests?
+    # We cannot use Gitlab::CurrentSettings as ApplicationSetting itself
+    # uses UrlValidator to validate urls. This ends up in a cycle
+    # when Gitlab::CurrentSettings creates an ApplicationSetting which then
+    # calls this validator.
+    #
+    # See https://gitlab.com/gitlab-org/gitlab-ee/issues/9833
+    ApplicationSetting.current&.allow_local_requests_from_hooks_and_services?
+  end
+end
diff --git a/app/validators/public_url_validator.rb b/app/validators/public_url_validator.rb
index 3ff880deedd..91847c5d866 100644
--- a/app/validators/public_url_validator.rb
+++ b/app/validators/public_url_validator.rb
@@ -2,7 +2,7 @@
 
 # PublicUrlValidator
 #
-# Custom validator for URLs. This validator works like UrlValidator but
+# Custom validator for URLs. This validator works like AddressableUrlValidator but
 # it blocks by default urls pointing to localhost or the local network.
 #
 # This validator accepts the same params UrlValidator does.
@@ -12,17 +12,20 @@
 #   class User < ActiveRecord::Base
 #     validates :personal_url, public_url: true
 #
-#     validates :ftp_url, public_url: { protocols: %w(ftp) }
+#     validates :ftp_url, public_url: { schemes: %w(ftp) }
 #
 #     validates :git_url, public_url: { allow_localhost: true, allow_local_network: true}
 #   end
 #
-class PublicUrlValidator < UrlValidator
-  private
+class PublicUrlValidator < AddressableUrlValidator
+  DEFAULT_OPTIONS = {
+    allow_localhost: false,
+    allow_local_network: false
+  }.freeze
 
-  def default_options
-    # By default block all urls pointing to localhost or the local network
-    super.merge(allow_localhost: false,
-                allow_local_network: false)
+  def initialize(options)
+    options.reverse_merge!(DEFAULT_OPTIONS)
+
+    super(options)
   end
 end
diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb
deleted file mode 100644
index 3fd015c3cf5..00000000000
--- a/app/validators/url_validator.rb
+++ /dev/null
@@ -1,104 +0,0 @@
-# frozen_string_literal: true
-
-# UrlValidator
-#
-# Custom validator for URLs.
-#
-# By default, only URLs for the HTTP(S) protocols will be considered valid.
-# Provide a `:protocols` option to configure accepted protocols.
-#
-# Example:
-#
-#   class User < ActiveRecord::Base
-#     validates :personal_url, url: true
-#
-#     validates :ftp_url, url: { protocols: %w(ftp) }
-#
-#     validates :git_url, url: { protocols: %w(http https ssh git) }
-#   end
-#
-# This validator can also block urls pointing to localhost or the local network to
-# protect against Server-side Request Forgery (SSRF), or check for the right port.
-#
-# The available options are:
-# - protocols: Allowed protocols. Default: http and https
-# - allow_localhost: Allow urls pointing to localhost. Default: true
-# - allow_local_network: Allow urls pointing to private network addresses. Default: true
-# - ports: Allowed ports. Default: all.
-# - enforce_user: Validate user format. Default: false
-# - enforce_sanitization: Validate that there are no html/css/js tags. Default: false
-#
-# Example:
-#   class User < ActiveRecord::Base
-#     validates :personal_url, url: { allow_localhost: false, allow_local_network: false}
-#
-#     validates :web_url, url: { ports: [80, 443] }
-#   end
-class UrlValidator < ActiveModel::EachValidator
-  DEFAULT_PROTOCOLS = %w(http https).freeze
-
-  attr_reader :record
-
-  def validate_each(record, attribute, value)
-    @record = record
-
-    unless value.present?
-      record.errors.add(attribute, 'must be a valid URL')
-      return
-    end
-
-    value = strip_value!(record, attribute, value)
-
-    Gitlab::UrlBlocker.validate!(value, blocker_args)
-  rescue Gitlab::UrlBlocker::BlockedUrlError => e
-    record.errors.add(attribute, "is blocked: #{e.message}")
-  end
-
-  private
-
-  def strip_value!(record, attribute, value)
-    new_value = value.strip
-    return value if new_value == value
-
-    record.public_send("#{attribute}=", new_value) # rubocop:disable GitlabSecurity/PublicSend
-  end
-
-  def default_options
-    # By default the validator doesn't block any url based on the ip address
-    {
-      protocols: DEFAULT_PROTOCOLS,
-      ports: [],
-      allow_localhost: true,
-      allow_local_network: true,
-      ascii_only: false,
-      enforce_user: false,
-      enforce_sanitization: false
-    }
-  end
-
-  def current_options
-    options = self.options.map do |option, value|
-      [option, value.is_a?(Proc) ? value.call(record) : value]
-    end.to_h
-
-    default_options.merge(options)
-  end
-
-  def blocker_args
-    current_options.slice(*default_options.keys).tap do |args|
-      if allow_setting_local_requests?
-        args[:allow_localhost] = args[:allow_local_network] = true
-      end
-    end
-  end
-
-  def allow_setting_local_requests?
-    # We cannot use Gitlab::CurrentSettings as ApplicationSetting itself
-    # uses UrlValidator to validate urls. This ends up in a cycle
-    # when Gitlab::CurrentSettings creates an ApplicationSetting which then
-    # calls this validator.
-    #
-    # See https://gitlab.com/gitlab-org/gitlab-ee/issues/9833
-    ApplicationSetting.current&.allow_local_requests_from_hooks_and_services?
-  end
-end